Skip to content

Commit 933f772

Browse files
hmottestadhmottestad
committed
bug fixes
1 parent 94d4189 commit 933f772

6 files changed

Lines changed: 144 additions & 16 deletions

File tree

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.eclipse.rdf4j.common.iteration.CloseableIteration;
2525
import org.eclipse.rdf4j.common.iteration.FilterIteration;
2626
import org.eclipse.rdf4j.common.iteration.IndexReportingIterator;
27+
import org.eclipse.rdf4j.common.transaction.QueryEvaluationMode;
2728
import org.eclipse.rdf4j.model.Value;
2829
import org.eclipse.rdf4j.query.BindingSet;
2930
import org.eclipse.rdf4j.query.MutableBindingSet;
@@ -132,7 +133,8 @@ private static QueryEvaluationStep supplyFilteredBindingSetAssignmentJoin(Filter
132133
}
133134
FilteredBindingSetAssignmentJoinIteration.DirectCondition directCondition = directCondition(
134135
filter.getCondition(),
135-
context);
136+
context,
137+
strategy.getQueryEvaluationMode() == QueryEvaluationMode.STRICT);
136138

137139
List<BindingSet> assignmentRows = bindingRows(assignment);
138140
QueryEvaluationStep leftPrepared = strategy.precompile(join.getLeftArg(), context);
@@ -155,7 +157,7 @@ private static QueryEvaluationStep supplyFilteredBindingSetAssignmentJoin(Filter
155157
}
156158

157159
private static FilteredBindingSetAssignmentJoinIteration.DirectCondition directCondition(ValueExpr condition,
158-
QueryEvaluationContext context) {
160+
QueryEvaluationContext context, boolean strict) {
159161
if (!(condition instanceof Compare)) {
160162
return null;
161163
}
@@ -172,7 +174,7 @@ private static FilteredBindingSetAssignmentJoinIteration.DirectCondition directC
172174
if (leftValue == null || rightValue == null) {
173175
return false;
174176
}
175-
return QueryEvaluationUtil.compare(leftValue, rightValue, operator);
177+
return QueryEvaluationUtil.compare(leftValue, rightValue, operator, strict);
176178
};
177179
}
178180

‎core/queryalgebra/evaluation/src/test/java/org/eclipse/rdf4j/query/algebra/evaluation/iterator/FilterIteratorTelemetryTest.java‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,24 +14,37 @@
1414
import static org.assertj.core.api.Assertions.assertThat;
1515
import static org.mockito.ArgumentMatchers.any;
1616
import static org.mockito.ArgumentMatchers.eq;
17+
import static org.mockito.Mockito.doReturn;
1718
import static org.mockito.Mockito.mock;
1819
import static org.mockito.Mockito.when;
1920

2021
import java.util.List;
22+
import java.util.Set;
2123
import java.util.concurrent.atomic.AtomicReference;
2224

25+
import org.eclipse.rdf4j.common.iteration.CloseableIteration;
2326
import org.eclipse.rdf4j.common.iteration.CloseableIteratorIteration;
27+
import org.eclipse.rdf4j.common.transaction.QueryEvaluationMode;
28+
import org.eclipse.rdf4j.model.Value;
29+
import org.eclipse.rdf4j.model.impl.BooleanLiteral;
2430
import org.eclipse.rdf4j.model.impl.SimpleValueFactory;
31+
import org.eclipse.rdf4j.model.vocabulary.XSD;
2532
import org.eclipse.rdf4j.query.BindingSet;
2633
import org.eclipse.rdf4j.query.algebra.BindingSetAssignment;
2734
import org.eclipse.rdf4j.query.algebra.Compare;
2835
import org.eclipse.rdf4j.query.algebra.Compare.CompareOp;
2936
import org.eclipse.rdf4j.query.algebra.Filter;
37+
import org.eclipse.rdf4j.query.algebra.Join;
3038
import org.eclipse.rdf4j.query.algebra.SingletonSet;
39+
import org.eclipse.rdf4j.query.algebra.TupleExpr;
3140
import org.eclipse.rdf4j.query.algebra.ValueConstant;
41+
import org.eclipse.rdf4j.query.algebra.ValueExpr;
3242
import org.eclipse.rdf4j.query.algebra.Var;
3343
import org.eclipse.rdf4j.query.algebra.evaluation.EvaluationStrategy;
44+
import org.eclipse.rdf4j.query.algebra.evaluation.QueryEvaluationStep;
3445
import org.eclipse.rdf4j.query.algebra.evaluation.QueryValueEvaluationStep;
46+
import org.eclipse.rdf4j.query.algebra.evaluation.impl.QueryEvaluationContext;
47+
import org.eclipse.rdf4j.query.impl.EmptyBindingSet;
3548
import org.eclipse.rdf4j.query.impl.MapBindingSet;
3649
import org.junit.jupiter.api.Test;
3750

@@ -95,9 +108,46 @@ void simpleFilterConditionUsesInputBindingsWithoutScopeCopy() throws Exception {
95108
assertThat(conditionBindings.get()).isSameAs(row);
96109
}
97110

111+
@Test
112+
void fusedAssignmentCompareUsesStrategyQueryMode() throws Exception {
113+
Value year = SimpleValueFactory.getInstance().createLiteral("2007", XSD.GYEAR);
114+
Value dateTime = SimpleValueFactory.getInstance().createLiteral("2009-01-01T20:20:20Z", XSD.DATETIME);
115+
BindingSetAssignment left = assignment("left", year);
116+
BindingSetAssignment right = assignment("right", dateTime);
117+
Compare condition = new Compare(new Var("left"), new Var("right"), CompareOp.LT);
118+
Filter filter = new Filter(new Join(left, right), condition);
119+
QueryEvaluationContext context = new QueryEvaluationContext.Minimal(null);
120+
QueryEvaluationStep leftStep = ignored -> new CloseableIteratorIteration<>(
121+
List.of(singleBindingSet("left", year)).iterator());
122+
QueryValueEvaluationStep conditionStep = ignored -> BooleanLiteral.TRUE;
123+
EvaluationStrategy strategy = mock(EvaluationStrategy.class);
124+
when(strategy.getQueryEvaluationMode()).thenReturn(QueryEvaluationMode.STANDARD);
125+
doReturn(leftStep).when(strategy).precompile(eq((TupleExpr) left), eq(context));
126+
doReturn(conditionStep).when(strategy).precompile(eq((ValueExpr) condition), eq(context));
127+
128+
QueryEvaluationStep step = FilterIterator.supply(filter, strategy, context);
129+
130+
try (CloseableIteration<BindingSet> iteration = step.evaluate(EmptyBindingSet.getInstance())) {
131+
assertThat(iteration.hasNext()).isTrue();
132+
assertThat(iteration.next().getValue("right")).isEqualTo(dateTime);
133+
assertThat(iteration.hasNext()).isFalse();
134+
}
135+
}
136+
98137
private static BindingSet singleBindingSet(String name, String value) {
138+
return singleBindingSet(name, SimpleValueFactory.getInstance().createLiteral(value));
139+
}
140+
141+
private static BindingSet singleBindingSet(String name, Value value) {
99142
MapBindingSet bindingSet = new MapBindingSet();
100-
bindingSet.addBinding(name, SimpleValueFactory.getInstance().createLiteral(value));
143+
bindingSet.addBinding(name, value);
101144
return bindingSet;
102145
}
146+
147+
private static BindingSetAssignment assignment(String name, Value value) {
148+
BindingSetAssignment assignment = new BindingSetAssignment();
149+
assignment.setBindingNames(Set.of(name));
150+
assignment.setBindingSets(List.of(singleBindingSet(name, value)));
151+
return assignment;
152+
}
103153
}

‎core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/SketchBasedJoinEstimator.java‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2061,6 +2061,7 @@ public synchronized long rebuild() {
20612061
clearEstimateCacheIfPopulated();
20622062
clearPositiveReadinessCache();
20632063
long requestVersion = rebuildRequestVersion.get();
2064+
long sizeBeforeRebuild = approxStoreSize.get();
20642065

20652066
boolean rebuildIntoA = !usingA; // remember before toggling
20662067

@@ -2148,8 +2149,9 @@ public synchronized long rebuild() {
21482149
markRebuiltSketchesDirty(tgt);
21492150
}
21502151
current = tgt; // single volatile write → visible to all readers
2151-
seenTriples = seen;
2152-
approxStoreSize.set(seen);
2152+
long rebuiltRows = seen;
2153+
seenTriples = approxStoreSize.updateAndGet(
2154+
currentSize -> storeSizeAfterRebuild(rebuiltRows, sizeBeforeRebuild, currentSize));
21532155
sketchesLoaded = true;
21542156
clearRebuildRequiredIfUnchanged(requestVersion);
21552157
dirty.set(true);
@@ -2259,13 +2261,24 @@ private static long applyStoreSizeDelta(long current, long additions, long delet
22592261
return Math.max(0L, afterAdditions - Math.min(afterAdditions, deletions));
22602262
}
22612263

2264+
private static long storeSizeAfterRebuild(long rebuiltRows, long sizeBeforeRebuild, long currentSize) {
2265+
if (currentSize >= sizeBeforeRebuild) {
2266+
return applyStoreSizeDelta(rebuiltRows, currentSize - sizeBeforeRebuild, 0L);
2267+
}
2268+
return applyStoreSizeDelta(rebuiltRows, 0L, sizeBeforeRebuild - currentSize);
2269+
}
2270+
22622271
private boolean isReadyForIncrementalUpdates() {
2263-
if (rebuildRequired.get() || (rebuildEpoch.get() & 1L) != 0L) {
2272+
if (rebuildRequired.get()) {
22642273
return false;
22652274
}
22662275
if (sketchesLoaded) {
2276+
// applyIncrementalBatch mirrors exact deltas into both buffers while rebuildEpoch is odd.
22672277
return true;
22682278
}
2279+
if ((rebuildEpoch.get() & 1L) != 0L) {
2280+
return false;
2281+
}
22692282
State snap = current;
22702283
synchronized (snap) {
22712284
return hasRequiredActiveSketchGroups(snap);

‎core/sail/base/src/test/java/org/eclipse/rdf4j/sail/base/SketchBasedJoinEstimatorBudgetAndSliceRegressionTest.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static org.junit.jupiter.api.Assertions.assertTrue;
1919

2020
import java.lang.reflect.Field;
21+
import java.lang.reflect.Method;
2122
import java.util.ArrayList;
2223
import java.util.Map;
2324
import java.util.Set;
@@ -128,7 +129,7 @@ private static void clearActivePairs(SketchBasedJoinEstimator estimator) throws
128129
@SuppressWarnings("unchecked")
129130
Map<?, ?> pairs = (Map<?, ?>) field(activeState(estimator), "pairs");
130131
for (Object pairBuild : pairs.values()) {
131-
((Map<?, ?>) field(pairBuild, "rows")).clear();
132+
invokeNoArg(pairBuild, "clear");
132133
}
133134
}
134135

@@ -152,6 +153,12 @@ private static Object field(Object target, String name) throws Exception {
152153
return field.get(target);
153154
}
154155

156+
private static void invokeNoArg(Object target, String name) throws Exception {
157+
Method method = target.getClass().getDeclaredMethod(name);
158+
method.setAccessible(true);
159+
method.invoke(target);
160+
}
161+
155162
private static void clearSketchArray(AtomicReferenceArray<?> sketches) {
156163
for (int i = 0; i < sketches.length(); i++) {
157164
sketches.set(i, null);

‎core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbSketchJoinOptimizer.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ private List<Filter> dependentNestedCostingFilters(Filter filter, LeftJoin leftJ
200200
if (!LmdbJoinPlanSupport.containsExists(condition)) {
201201
continue;
202202
}
203-
Set<String> conditionVars = VarNameCollector.process(condition);
203+
Set<String> conditionVars = new HashSet<>(VarNameCollector.process(condition));
204204
conditionVars.retainAll(leftBindingNames);
205205
if (!conditionVars.isEmpty()) {
206206
costingFilters.add(new Filter(leftJoin.getLeftArg().clone(), condition.clone()));
@@ -488,14 +488,16 @@ private boolean isUniqueAlreadyBoundStatementProbe(TupleExpr tupleExpr, Set<Stri
488488
if (!(tupleExpr instanceof StatementPattern statementPattern)) {
489489
return false;
490490
}
491-
return isFixedOrBoundPatternVar(statementPattern.getSubjectVar(), boundNames)
491+
Var contextVar = statementPattern.getContextVar();
492+
return contextVar != null
493+
&& isFixedOrBoundPatternVar(statementPattern.getSubjectVar(), boundNames)
492494
&& isFixedOrBoundPatternVar(statementPattern.getPredicateVar(), boundNames)
493495
&& isFixedOrBoundPatternVar(statementPattern.getObjectVar(), boundNames)
494-
&& isFixedOrBoundPatternVar(statementPattern.getContextVar(), boundNames);
496+
&& isFixedOrBoundPatternVar(contextVar, boundNames);
495497
}
496498

497499
private boolean isFixedOrBoundPatternVar(Var var, Set<String> boundNames) {
498-
return var == null || var.hasValue() || boundNames.contains(var.getName());
500+
return var.hasValue() || boundNames.contains(var.getName());
499501
}
500502

501503
@Override

‎core/sail/lmdb/src/test/java/org/eclipse/rdf4j/sail/lmdb/LmdbSketchJoinOptimizerTest.java‎

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,23 +244,37 @@ void keepsOptionalCompareAgainstConstantWhenFilterUsesOptionalAlias() {
244244

245245
@Test
246246
void removesNoNewBindingOptionalDirectProbe() {
247+
BindingSetAssignment graph = values("graph", "ctx");
248+
StatementPattern type = statementPattern("node", "type", "nodeType");
249+
StatementPattern connects = statementPattern("node", "connectsTo", "neighbor");
250+
StatementPattern reverseProbe = statementPattern("neighbor", "connectsTo", "node", "graph");
251+
QueryRoot root = new QueryRoot(new LeftJoin(new Join(graph, new Join(type, connects)), reverseProbe));
252+
253+
new LmdbSketchJoinOptimizer(PlanningStatistics.rejected(), false).optimize(root, null, null);
254+
255+
assertTrue(!containsLeftJoin(root.getArg()));
256+
assertEquals(List.of(graph, type, connects), joinArgs(root.getArg()));
257+
}
258+
259+
@Test
260+
void keepsNoNewBindingOptionalProbeWithoutFixedContext() {
247261
StatementPattern type = statementPattern("node", "type", "nodeType");
248262
StatementPattern connects = statementPattern("node", "connectsTo", "neighbor");
249263
StatementPattern reverseProbe = statementPattern("neighbor", "connectsTo", "node");
250264
QueryRoot root = new QueryRoot(new LeftJoin(new Join(type, connects), reverseProbe));
251265

252266
new LmdbSketchJoinOptimizer(PlanningStatistics.rejected(), false).optimize(root, null, null);
253267

254-
assertTrue(!containsLeftJoin(root.getArg()));
255-
assertEquals(List.of(type, connects), joinArgs(root.getArg()));
268+
assertTrue(containsLeftJoin(root.getArg()));
256269
}
257270

258271
@Test
259272
void rewritesNoNewBindingExistsDirectProbeToJoinFactor() {
260273
StatementPattern hasTrack = statementPattern("section", "hasTrack", "track");
261274
StatementPattern trackType = new StatementPattern(new Var("track"),
262275
new Var("_const_type", VF.createIRI("urn:type")),
263-
new Var("_const_trackType", VF.createIRI("urn:TrackSection")));
276+
new Var("_const_trackType", VF.createIRI("urn:TrackSection")),
277+
new Var("_const_context", VF.createIRI("urn:ctx")));
264278
QueryRoot root = new QueryRoot(new Filter(hasTrack, new Exists(trackType)));
265279

266280
new LmdbSketchJoinOptimizer(PlanningStatistics.rejected(), false).optimize(root, null, null);
@@ -269,6 +283,19 @@ void rewritesNoNewBindingExistsDirectProbeToJoinFactor() {
269283
assertEquals(List.of(hasTrack, trackType), joinArgs(root.getArg()));
270284
}
271285

286+
@Test
287+
void keepsNoNewBindingExistsProbeWithoutFixedContext() {
288+
StatementPattern hasTrack = statementPattern("section", "hasTrack", "track");
289+
StatementPattern trackType = new StatementPattern(new Var("track"),
290+
new Var("_const_type", VF.createIRI("urn:type")),
291+
new Var("_const_trackType", VF.createIRI("urn:TrackSection")));
292+
QueryRoot root = new QueryRoot(new Filter(hasTrack, new Exists(trackType)));
293+
294+
new LmdbSketchJoinOptimizer(PlanningStatistics.rejected(), false).optimize(root, null, null);
295+
296+
assertTrue(containsExistsFilter(root.getArg()));
297+
}
298+
272299
@Test
273300
void rewritesNoNewBindingExistsProbeAboveOptionalFilterToJoinFactor() {
274301
StatementPattern sectionType = statementPattern("section", "type", "sectionType");
@@ -278,7 +305,8 @@ void rewritesNoNewBindingExistsProbeAboveOptionalFilterToJoinFactor() {
278305
Compare optionalFilter = new Compare(new Var("optOp"), new Var("section"), Compare.CompareOp.NE);
279306
StatementPattern trackType = new StatementPattern(new Var("track"),
280307
new Var("_const_type", VF.createIRI("urn:type")),
281-
new Var("_const_trackType", VF.createIRI("urn:TrackSection")));
308+
new Var("_const_trackType", VF.createIRI("urn:TrackSection")),
309+
new Var("_const_context", VF.createIRI("urn:ctx")));
282310
QueryRoot root = new QueryRoot(
283311
new Filter(new Filter(new LeftJoin(new Join(sectionType, hasTrack), optional), optionalFilter),
284312
new Exists(trackType)));
@@ -290,6 +318,26 @@ void rewritesNoNewBindingExistsProbeAboveOptionalFilterToJoinFactor() {
290318
assertTrue(joinArgs(root.getArg()).contains(trackType));
291319
}
292320

321+
@Test
322+
void keepsNoNewBindingExistsProbeAboveOptionalFilterWithoutFixedContext() {
323+
StatementPattern sectionType = statementPattern("section", "type", "sectionType");
324+
StatementPattern hasTrack = statementPattern("section", "hasTrack", "track");
325+
Extension optional = new Extension(statementPattern("section", "connectsOperationalPoint", "op"),
326+
new ExtensionElem(new Var("op"), "optOp"));
327+
Compare optionalFilter = new Compare(new Var("optOp"), new Var("section"), Compare.CompareOp.NE);
328+
StatementPattern trackType = new StatementPattern(new Var("track"),
329+
new Var("_const_type", VF.createIRI("urn:type")),
330+
new Var("_const_trackType", VF.createIRI("urn:TrackSection")));
331+
QueryRoot root = new QueryRoot(
332+
new Filter(new Filter(new LeftJoin(new Join(sectionType, hasTrack), optional), optionalFilter),
333+
new Exists(trackType)));
334+
335+
new LmdbSketchJoinOptimizer(PlanningStatistics.rejected(), false).optimize(root, null, null);
336+
337+
assertTrue(containsExistsFilter(root.getArg()));
338+
assertTrue(containsLeftJoin(root.getArg()));
339+
}
340+
293341
@Test
294342
void duplicatesValuesAndFilterIntoUnionBranches() {
295343
BindingSetAssignment values = values("target", "DX-200", "DX-201");
@@ -416,6 +464,12 @@ private static StatementPattern statementPattern(String subjectName, String pred
416464
new Var(objectName));
417465
}
418466

467+
private static StatementPattern statementPattern(String subjectName, String predicateName, String objectName,
468+
String contextName) {
469+
return new StatementPattern(new Var(subjectName), new Var(predicateName, VF.createIRI("urn:" + predicateName)),
470+
new Var(objectName), new Var(contextName));
471+
}
472+
419473
private static BindingSetAssignment values(String bindingName, String... values) {
420474
BindingSetAssignment assignment = new BindingSetAssignment();
421475
assignment.setBindingNames(Set.of(bindingName));

0 commit comments

Comments
 (0)