fixes

Author: hmottestad

core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbLftjPreparedPlanCache.java

@@ -44,24 +44,29 @@ synchronized void clear() {
 	}
 
 	static String normalizedKey(List<StatementPattern> patterns, Set<String> configuredIndexes) {
-		Map<String, String> aliases = new LinkedHashMap<>();
 		StringBuilder builder = new StringBuilder(configuredIndexes.size() * 6 + patterns.size() * 32);
 		builder.append("indexes=");
 		configuredIndexes.stream().sorted().forEach(indexName -> builder.append(indexName).append(','));
 		builder.append(";patterns=");
-		int[] nextAlias = { 0 };
-		for (StatementPattern pattern : patterns) {
-			builder.append('[').append(pattern.getScope().name()).append(';');
-			appendTerm(builder, pattern.getSubjectVar(), aliases, nextAlias);
-			appendTerm(builder, pattern.getPredicateVar(), aliases, nextAlias);
-			appendTerm(builder, pattern.getObjectVar(), aliases, nextAlias);
-			appendTerm(builder, pattern.getContextVar(), aliases, nextAlias);
-			builder.append(']');
-		}
+		patterns.stream()
+				.map(LmdbLftjPreparedPlanCache::patternKey)
+				.sorted()
+				.forEach(builder::append);
+		return builder.toString();
+	}
+
+	private static String patternKey(StatementPattern pattern) {
+		StringBuilder builder = new StringBuilder(48);
+		builder.append('[').append(pattern.getScope().name()).append(';');
+		appendTerm(builder, pattern.getSubjectVar());
+		appendTerm(builder, pattern.getPredicateVar());
+		appendTerm(builder, pattern.getObjectVar());
+		appendTerm(builder, pattern.getContextVar());
+		builder.append(']');
 		return builder.toString();
 	}
 
-	private static void appendTerm(StringBuilder builder, Var var, Map<String, String> aliases, int[] nextAlias) {
+	private static void appendTerm(StringBuilder builder, Var var) {
 		if (var == null) {
 			builder.append("null;");
 			return;
@@ -74,8 +79,7 @@ private static void appendTerm(StringBuilder builder, Var var, Map<String, Strin
 			builder.append("hidden;");
 			return;
 		}
-		String alias = aliases.computeIfAbsent(var.getName(), ignored -> "v" + nextAlias[0]++);
-		builder.append(alias).append(';');
+		builder.append("var=").append(var.getName()).append(';');
 	}
 
 	private static String valueKey(Value value) {

core/sail/lmdb/src/test/java/org/eclipse/rdf4j/sail/lmdb/LmdbLftjOptimizerTest.java

@@ -15,6 +15,7 @@
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -38,24 +39,45 @@
 
 class LmdbLftjOptimizerTest {
 
+	private static final SimpleValueFactory VF = SimpleValueFactory.getInstance();
+
 	@Test
-	void optimizeShouldReusePreparedPlanAcrossEquivalentVariableRenames() {
+	void optimizeShouldNotReusePreparedPlanAcrossEquivalentVariableRenames() {
 		TestQueryAccess queryAccess = new TestQueryAccess();
 		LmdbLftjOptimizer optimizer = new LmdbLftjOptimizer(
 				new LmdbLftjTripleSource(new EmptyTripleSource(), queryAccess));
 
 		TupleExpr first = new QueryRoot(cycle("a", "b", "c"));
 		TupleExpr second = new QueryRoot(cycle("x", "y", "z"));
+		Set<String> expectedBindingNames = second.getBindingNames();
+
+		optimizer.optimize(first, (Dataset) null, EmptyBindingSet.getInstance());
+		optimizer.optimize(second, (Dataset) null, EmptyBindingSet.getInstance());
+
+		assertEquals(2, queryAccess.cachedPlanPuts,
+				"renamed visible variables must produce a fresh prepared plan");
+		assertEquals(0, queryAccess.cachedPlanHits,
+				"prepared-plan reuse across renamed variables leaks stale binding names");
+		assertEquals(List.of("x", "y", "z"), lftjNode(second).plan().variableOrder());
+		assertEquals(expectedBindingNames, lftjNode(second).plan().bindingNames());
+	}
+
+	@Test
+	void optimizeShouldReusePreparedPlanAcrossEquivalentJoinReorders() {
+		TestQueryAccess queryAccess = new TestQueryAccess();
+		LmdbLftjOptimizer optimizer = new LmdbLftjOptimizer(
+				new LmdbLftjTripleSource(new EmptyTripleSource(), queryAccess));
+
+		TupleExpr first = new QueryRoot(distinctPredicateCycle("a", "b", "c"));
+		TupleExpr second = new QueryRoot(reorderedDistinctPredicateCycle("a", "b", "c"));
 
 		optimizer.optimize(first, (Dataset) null, EmptyBindingSet.getInstance());
 		optimizer.optimize(second, (Dataset) null, EmptyBindingSet.getInstance());
 
 		assertEquals(1, queryAccess.cachedPlanPuts,
-				"first cyclic plan should populate the prepared-plan cache once");
+				"equivalent cyclic shapes should share one prepared plan entry");
 		assertEquals(1, queryAccess.cachedPlanHits,
-				"equivalent cyclic shape should reuse the cached prepared plan on the second optimize");
-		assertInstanceOf(LmdbLftjTupleExpr.class, assertInstanceOf(QueryRoot.class, first).getArg());
-		assertInstanceOf(LmdbLftjTupleExpr.class, assertInstanceOf(QueryRoot.class, second).getArg());
+				"commuted join order should still hit the normalized prepared-plan cache");
 	}
 
 	private TupleExpr cycle(String a, String b, String c) {
@@ -69,6 +91,24 @@ private StatementPattern statementPattern(String subjectName, String objectName)
 		return new StatementPattern(new Var(subjectName), new Var("pred", FOAF.KNOWS), new Var(objectName));
 	}
 
+	private TupleExpr distinctPredicateCycle(String a, String b, String c) {
+		StatementPattern pattern1 = new StatementPattern(new Var(a), new Var("p1", VF.createIRI("urn:p1")), new Var(b));
+		StatementPattern pattern2 = new StatementPattern(new Var(b), new Var("p2", VF.createIRI("urn:p2")), new Var(c));
+		StatementPattern pattern3 = new StatementPattern(new Var(c), new Var("p3", VF.createIRI("urn:p3")), new Var(a));
+		return new Join(new Join(pattern1, pattern2), pattern3);
+	}
+
+	private TupleExpr reorderedDistinctPredicateCycle(String a, String b, String c) {
+		StatementPattern pattern1 = new StatementPattern(new Var(a), new Var("p1", VF.createIRI("urn:p1")), new Var(b));
+		StatementPattern pattern2 = new StatementPattern(new Var(b), new Var("p2", VF.createIRI("urn:p2")), new Var(c));
+		StatementPattern pattern3 = new StatementPattern(new Var(c), new Var("p3", VF.createIRI("urn:p3")), new Var(a));
+		return new Join(new Join(pattern2, pattern3), pattern1);
+	}
+
+	private LmdbLftjTupleExpr lftjNode(TupleExpr tupleExpr) {
+		return assertInstanceOf(LmdbLftjTupleExpr.class, assertInstanceOf(QueryRoot.class, tupleExpr).getArg());
+	}
+
 	private static final class EmptyTripleSource implements TripleSource {
 
 		@Override