Skip to content

Commit 72040f8

Browse files
jasperiqJervenBolleman
authored andcommitted
GH-5286 Improve right hand side evaluation of LeftJoin
Do not evaluate the right hand side when the condition can be evaluated with only left hand side bindings and evaluates to false.
1 parent 877b1c9 commit 72040f8

4 files changed

Lines changed: 271 additions & 14 deletions

File tree

core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/evaluationsteps/LeftJoinQueryEvaluationStep.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,13 @@ public CloseableIteration<BindingSet> evaluate(BindingSet bindings) {
103103
if (containsNone) {
104104
// left join is "well designed"
105105
leftJoin.setAlgorithm(LeftJoinIterator.class.getSimpleName());
106-
return LeftJoinIterator.getInstance(left, right, condition, bindings, leftJoin.getBindingNames());
106+
return LeftJoinIterator.getInstance(left, right, condition, bindings, leftJoin.getBindingNames(), leftJoin);
107107
} else {
108108
Set<String> problemVars = new HashSet<>(optionalVars);
109109
problemVars.retainAll(bindings.getBindingNames());
110110

111111
leftJoin.setAlgorithm(BadlyDesignedLeftJoinIterator.class.getSimpleName());
112-
return new BadlyDesignedLeftJoinIterator(left, right, condition, bindings, problemVars);
112+
return new BadlyDesignedLeftJoinIterator(left, right, condition, bindings, problemVars, leftJoin);
113113
}
114114
}
115115
}

core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/iterator/BadlyDesignedLeftJoinIterator.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,14 @@ public BadlyDesignedLeftJoinIterator(EvaluationStrategy strategy, LeftJoin join,
5252
* Methods *
5353
*---------*/
5454

55-
public BadlyDesignedLeftJoinIterator(QueryEvaluationStep left, QueryEvaluationStep right,
56-
QueryValueEvaluationStep joinCondition, BindingSet inputBindings, Set<String> problemVars)
55+
public BadlyDesignedLeftJoinIterator(QueryEvaluationStep left,
56+
QueryEvaluationStep right,
57+
QueryValueEvaluationStep joinCondition,
58+
BindingSet inputBindings,
59+
Set<String> problemVars,
60+
LeftJoin leftJoin)
5761
throws QueryEvaluationException {
58-
super(left, right, joinCondition, getFilteredBindings(inputBindings, problemVars), problemVars);
62+
super(left, right, joinCondition, getFilteredBindings(inputBindings, problemVars), problemVars, leftJoin);
5963
this.inputBindings = inputBindings;
6064
this.problemVars = problemVars;
6165
}

core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/iterator/LeftJoinIterator.java

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.eclipse.rdf4j.query.algebra.evaluation.ValueExprEvaluationException;
2929
import org.eclipse.rdf4j.query.algebra.evaluation.impl.QueryEvaluationContext;
3030
import org.eclipse.rdf4j.query.algebra.evaluation.util.QueryEvaluationUtility;
31+
import org.eclipse.rdf4j.query.algebra.helpers.collectors.VarNameCollector;
3132

3233
public class LeftJoinIterator extends LookAheadIteration<BindingSet> {
3334
/*-----------*
@@ -41,6 +42,8 @@ public class LeftJoinIterator extends LookAheadIteration<BindingSet> {
4142
private final Set<String> scopeBindingNames;
4243

4344
private final CloseableIteration<BindingSet> leftIter;
45+
private final LeftJoin leftJoin;
46+
private final boolean canEvaluateConditionBasedOnLeftHandSide;
4447

4548
private CloseableIteration<BindingSet> rightIter;
4649

@@ -56,6 +59,7 @@ public LeftJoinIterator(EvaluationStrategy strategy, LeftJoin join, BindingSet b
5659
QueryEvaluationContext context)
5760
throws QueryEvaluationException {
5861
this.scopeBindingNames = join.getBindingNames();
62+
this.leftJoin = join;
5963

6064
leftIter = strategy.evaluate(join.getLeftArg(), bindings);
6165

@@ -69,12 +73,14 @@ public LeftJoinIterator(EvaluationStrategy strategy, LeftJoin join, BindingSet b
6973
} else {
7074
joinCondition = strategy.precompile(condition, context);
7175
}
76+
this.canEvaluateConditionBasedOnLeftHandSide = canEvaluateConditionBasedOnLeftHandSide(leftJoin);
7277
}
7378

7479
public LeftJoinIterator(QueryEvaluationStep left, QueryEvaluationStep right, QueryValueEvaluationStep joinCondition,
75-
BindingSet bindings, Set<String> scopeBindingNames)
80+
BindingSet bindings, Set<String> scopeBindingNames, LeftJoin leftJoin)
7681
throws QueryEvaluationException {
7782
this.scopeBindingNames = scopeBindingNames;
83+
this.leftJoin = leftJoin;
7884

7985
leftIter = left.evaluate(bindings);
8086

@@ -83,28 +89,37 @@ public LeftJoinIterator(QueryEvaluationStep left, QueryEvaluationStep right, Que
8389

8490
prepareRightArg = right;
8591
this.joinCondition = joinCondition;
92+
this.canEvaluateConditionBasedOnLeftHandSide = canEvaluateConditionBasedOnLeftHandSide(leftJoin);
8693

8794
}
8895

89-
public LeftJoinIterator(CloseableIteration<BindingSet> leftIter, QueryEvaluationStep prepareRightArg,
90-
QueryValueEvaluationStep joinCondition, Set<String> scopeBindingNames) {
96+
public LeftJoinIterator(CloseableIteration<BindingSet> leftIter,
97+
QueryEvaluationStep prepareRightArg,
98+
QueryValueEvaluationStep joinCondition,
99+
Set<String> scopeBindingNames,
100+
LeftJoin leftJoin) {
91101
this.scopeBindingNames = scopeBindingNames;
92102
this.leftIter = leftIter;
103+
this.leftJoin = leftJoin;
93104
this.rightIter = null;
94105
this.prepareRightArg = prepareRightArg;
95106
this.joinCondition = joinCondition;
107+
this.canEvaluateConditionBasedOnLeftHandSide = canEvaluateConditionBasedOnLeftHandSide(leftJoin);
96108
}
97109

98110
public static CloseableIteration<BindingSet> getInstance(QueryEvaluationStep left,
99-
QueryEvaluationStep prepareRightArg, QueryValueEvaluationStep joinCondition, BindingSet bindings,
100-
Set<String> scopeBindingNames) {
111+
QueryEvaluationStep prepareRightArg,
112+
QueryValueEvaluationStep joinCondition,
113+
BindingSet bindings,
114+
Set<String> scopeBindingNames,
115+
LeftJoin leftJoin) {
101116

102117
CloseableIteration<BindingSet> leftIter = left.evaluate(bindings);
103118

104119
if (leftIter == QueryEvaluationStep.EMPTY_ITERATION) {
105120
return leftIter;
106121
} else {
107-
return new LeftJoinIterator(leftIter, prepareRightArg, joinCondition, scopeBindingNames);
122+
return new LeftJoinIterator(leftIter, prepareRightArg, joinCondition, scopeBindingNames, leftJoin);
108123
}
109124

110125
}
@@ -125,7 +140,11 @@ protected BindingSet getNextElement() throws QueryEvaluationException {
125140
if (leftIter.hasNext()) {
126141
// Use left arg's bindings in case join fails
127142
leftBindings = leftIter.next();
128-
nextRightIter = rightIter = prepareRightArg.evaluate(leftBindings);
143+
if (shouldEvaluateRightHandSide(leftBindings)) {
144+
nextRightIter = rightIter = prepareRightArg.evaluate(leftBindings);
145+
} else {
146+
return leftBindings;
147+
}
129148
} else {
130149
return null;
131150
}
@@ -135,7 +154,11 @@ protected BindingSet getNextElement() throws QueryEvaluationException {
135154
leftBindings = leftIter.next();
136155

137156
nextRightIter.close();
138-
nextRightIter = rightIter = prepareRightArg.evaluate(leftBindings);
157+
if (shouldEvaluateRightHandSide(leftBindings)) {
158+
nextRightIter = rightIter = prepareRightArg.evaluate(leftBindings);
159+
} else {
160+
return leftBindings;
161+
}
139162
}
140163

141164
if (nextRightIter == QueryEvaluationStep.EMPTY_ITERATION) {
@@ -147,7 +170,7 @@ protected BindingSet getNextElement() throws QueryEvaluationException {
147170
BindingSet rightBindings = nextRightIter.next();
148171

149172
try {
150-
if (joinCondition == null) {
173+
if (joinCondition == null || canEvaluateConditionBasedOnLeftHandSide) {
151174
return rightBindings;
152175
} else {
153176
// Limit the bindings to the ones that are in scope for
@@ -184,6 +207,34 @@ protected BindingSet getNextElement() throws QueryEvaluationException {
184207
return null;
185208
}
186209

210+
private static boolean canEvaluateConditionBasedOnLeftHandSide(LeftJoin leftJoin) {
211+
if (leftJoin.hasCondition()) {
212+
var collector = new VarNameCollector();
213+
leftJoin.getCondition().visit(collector);
214+
215+
Set<String> assuredBindingNames = leftJoin.getAssuredBindingNames();
216+
return assuredBindingNames.containsAll(collector.getVarNames());
217+
}
218+
219+
return false;
220+
}
221+
222+
private boolean shouldEvaluateRightHandSide(BindingSet leftBindings) {
223+
if (!canEvaluateConditionBasedOnLeftHandSide) {
224+
return true;
225+
}
226+
227+
QueryBindingSet scopeBindings = new QueryBindingSet(leftJoin.getAssuredBindingNames().size());
228+
for (String scopeBindingName : leftJoin.getAssuredBindingNames()) {
229+
Binding binding = leftBindings.getBinding(scopeBindingName);
230+
if (binding != null) {
231+
scopeBindings.addBinding(binding);
232+
}
233+
}
234+
235+
return isTrue(joinCondition, scopeBindings);
236+
}
237+
187238
private boolean isTrue(QueryValueEvaluationStep expr, QueryBindingSet bindings) {
188239
Value value = expr.evaluate(bindings);
189240
return QueryEvaluationUtility.getEffectiveBooleanValue(value).orElse(false);
Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Eclipse RDF4J contributors.
3+
*
4+
* All rights reserved. This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Distribution License v1.0
6+
* which accompanies this distribution, and is available at
7+
* http://www.eclipse.org/org/documents/edl-v10.php.
8+
*
9+
* SPDX-License-Identifier: BSD-3-Clause
10+
*******************************************************************************/
11+
package org.eclipse.rdf4j.query.algebra.evaluation.iterator;
12+
13+
import static org.junit.jupiter.api.Assertions.*;
14+
15+
import java.util.List;
16+
import java.util.Set;
17+
import java.util.concurrent.atomic.AtomicInteger;
18+
19+
import org.eclipse.rdf4j.common.iteration.CloseableIteration;
20+
import org.eclipse.rdf4j.model.*;
21+
import org.eclipse.rdf4j.model.impl.BooleanLiteral;
22+
import org.eclipse.rdf4j.model.impl.SimpleValueFactory;
23+
import org.eclipse.rdf4j.query.BindingSet;
24+
import org.eclipse.rdf4j.query.QueryEvaluationException;
25+
import org.eclipse.rdf4j.query.algebra.*;
26+
import org.eclipse.rdf4j.query.algebra.evaluation.*;
27+
import org.eclipse.rdf4j.query.algebra.evaluation.impl.DefaultEvaluationStrategy;
28+
import org.junit.jupiter.api.BeforeAll;
29+
import org.junit.jupiter.api.BeforeEach;
30+
import org.junit.jupiter.api.DisplayName;
31+
import org.junit.jupiter.api.Test;
32+
33+
class LeftJoinIteratorTest {
34+
35+
private static final QueryBindingSet RIGHT_BINDINGS = new QueryBindingSet(1);
36+
private static final ValueFactory VALUE_FACTORY = SimpleValueFactory.getInstance();
37+
private static final EvaluationStrategy EVALUATOR = new DefaultEvaluationStrategy(new TripleSource() {
38+
39+
@Override
40+
public ValueFactory getValueFactory() {
41+
return SimpleValueFactory.getInstance();
42+
}
43+
44+
@Override
45+
public CloseableIteration<? extends Statement> getStatements(Resource subj, IRI pred,
46+
Value obj, Resource... contexts) throws QueryEvaluationException {
47+
return null;
48+
}
49+
}, null);
50+
51+
private QueryBindingSet bindingSet;
52+
private BindingSetAssignment left;
53+
private QueryEvaluationStep leftHandSide;
54+
private RightHandSideQueryEvaluationStep rightHandSide;
55+
56+
@BeforeAll
57+
static void beforeAll() {
58+
RIGHT_BINDINGS.addBinding("right", VALUE_FACTORY.createLiteral(42));
59+
}
60+
61+
@BeforeEach
62+
void setUp() {
63+
bindingSet = new QueryBindingSet(1);
64+
bindingSet.addBinding("left", VALUE_FACTORY.createLiteral(42));
65+
66+
left = new BindingSetAssignment();
67+
left.setBindingSets(List.of(bindingSet));
68+
69+
leftHandSide = EVALUATOR.precompile(left);
70+
rightHandSide = new RightHandSideQueryEvaluationStep();
71+
}
72+
73+
@Test
74+
@DisplayName("when condition can be evaluated only with left hand side and condition evaluates to false, then don't evaluate right hand side")
75+
void skipsRightHandSideEvaluation() {
76+
QueryValueEvaluationStep condition = bindings -> BooleanLiteral.valueOf(false);
77+
Compare compare = new Compare(new Var("left"), new ValueConstant(VALUE_FACTORY.createLiteral(1337)));
78+
79+
runLeftOnce(condition, leftJoin(compare));
80+
81+
assertFalse(rightHandSide.isEvaluated);
82+
}
83+
84+
@Test
85+
@DisplayName("when condition can be evaluated only with left hand side and condition evaluates to false, then return left bindings")
86+
void onlyReturnLeftHandSideBindings() {
87+
QueryValueEvaluationStep condition = bindings -> BooleanLiteral.valueOf(false);
88+
Compare compare = new Compare(new Var("left"), new ValueConstant(VALUE_FACTORY.createLiteral(1337)));
89+
90+
var result = runLeftOnce(condition, leftJoin(compare));
91+
92+
assertIterableEquals(left.getBindingSets(), Set.of(result));
93+
}
94+
95+
@Test
96+
@DisplayName("when condition can be evaluated only with left hand side and condition evaluates to true, then evaluate right hand side")
97+
void evaluatesRightHandSideWithTrueCondition() {
98+
QueryValueEvaluationStep condition = bindings -> BooleanLiteral.valueOf(true);
99+
Compare compare = new Compare(new Var("left"), new ValueConstant(VALUE_FACTORY.createLiteral(42)));
100+
101+
runLeftOnce(condition, leftJoin(compare));
102+
103+
assertTrue(rightHandSide.isEvaluated);
104+
}
105+
106+
@Test
107+
@DisplayName("when condition can be evaluated only with left hand side and condition evaluates to true, then return joined bindings")
108+
void returnsRightBindings() {
109+
QueryValueEvaluationStep condition = bindings -> BooleanLiteral.valueOf(true);
110+
Compare compare = new Compare(new Var("left"), new ValueConstant(VALUE_FACTORY.createLiteral(42)));
111+
112+
var result = runLeftOnce(condition, leftJoin(compare));
113+
114+
var expected = new QueryBindingSet(2);
115+
bindingSet.forEach(expected::addBinding);
116+
RIGHT_BINDINGS.forEach(expected::addBinding);
117+
assertIterableEquals(expected, result);
118+
}
119+
120+
@Test
121+
@DisplayName("when condition can be evaluated only with left hand side and condition evaluates to true, then only evaluate condition once")
122+
void onlyEvaluatesConditionOnce() {
123+
var evaluations = new AtomicInteger(0);
124+
QueryValueEvaluationStep condition = bindings -> {
125+
evaluations.incrementAndGet();
126+
return BooleanLiteral.valueOf(true);
127+
};
128+
Compare compare = new Compare(new Var("left"), new ValueConstant(VALUE_FACTORY.createLiteral(42)));
129+
130+
runLeftOnce(condition, leftJoin(compare));
131+
132+
assertEquals(1, evaluations.get());
133+
}
134+
135+
@Test
136+
@DisplayName("when condition cannot be evaluated only with left hand side, then evaluate right hand side")
137+
void evaluatesRightHandSideEvaluation() {
138+
QueryValueEvaluationStep condition = bindings -> BooleanLiteral.valueOf(true);
139+
Compare compare = new Compare(new Var("right"), new ValueConstant(VALUE_FACTORY.createLiteral(42)));
140+
141+
runLeftOnce(condition, leftJoin(compare));
142+
143+
assertTrue(rightHandSide.isEvaluated);
144+
}
145+
146+
@Test
147+
@DisplayName("when no condition, then evaluate right hand side")
148+
void noCondition() {
149+
Compare compare = new Compare(new Var("right"), new ValueConstant(VALUE_FACTORY.createLiteral(42)));
150+
151+
runLeftOnce(null, leftJoin(compare));
152+
153+
assertTrue(rightHandSide.isEvaluated);
154+
}
155+
156+
private LeftJoin leftJoin(Compare compare) {
157+
return new LeftJoin(left, new EmptySet(), compare);
158+
}
159+
160+
private BindingSet runLeftOnce(QueryValueEvaluationStep condition, LeftJoin leftJoin) {
161+
try (LeftJoinIterator iterator = new LeftJoinIterator(
162+
leftHandSide,
163+
rightHandSide,
164+
condition,
165+
bindingSet,
166+
Set.of("left", "right"),
167+
leftJoin)) {
168+
return iterator.getNextElement();
169+
}
170+
}
171+
172+
private static class RightHandSideQueryEvaluationStep implements QueryEvaluationStep {
173+
174+
private boolean isEvaluated = false;
175+
176+
@Override
177+
public CloseableIteration<BindingSet> evaluate(BindingSet bindings) {
178+
isEvaluated = true;
179+
var bothBindings = new QueryBindingSet(2);
180+
bindings.forEach(bothBindings::addBinding);
181+
RIGHT_BINDINGS.forEach(bothBindings::addBinding);
182+
var rightIterator = List.of(bothBindings).iterator();
183+
184+
return new CloseableIteration<>() {
185+
@Override
186+
public void close() {
187+
// Nothing to close
188+
}
189+
190+
@Override
191+
public boolean hasNext() {
192+
return rightIterator.hasNext();
193+
}
194+
195+
@Override
196+
public BindingSet next() {
197+
return rightIterator.next();
198+
}
199+
};
200+
}
201+
}
202+
}

0 commit comments

Comments
 (0)