fixes

Author: hmottestad

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

@@ -779,7 +779,9 @@ private void recoverEstimatorAfterFailure(String action, RuntimeException e) {
 
 		@Override
 		public void close() {
-			// do nothing
+			if (nonIsolated && storeTxnStarted.get()) {
+				clearEstimatorUpdates();
+			}
 		}
 
 		@Override

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

@@ -484,21 +484,48 @@ private SketchBasedJoinEstimator getSketchBasedJoinEstimator() {
 	}
 
 	private DefaultEvaluationStrategyFactory getAutomaticDefaultEvaluationStrategyFactory() {
+		QueryOptimizerPipeline optimizerPipeline = getAutomaticOptimizerPipeline();
 		if (defaultEvalStratFactory == null) {
 			defaultEvalStratFactory = new DefaultEvaluationStrategyFactory(getFederatedServiceResolver());
-			defaultEvalStratFactory.setOptimizerPipeline(automaticOptimizerPipeline);
+		}
+		if (optimizerPipeline != null) {
+			defaultEvalStratFactory.setOptimizerPipeline(optimizerPipeline);
 		}
 		return defaultEvalStratFactory;
 	}
 
 	private LmdbEvaluationStrategyFactory getAutomaticLmdbEvaluationStrategyFactory() {
+		QueryOptimizerPipeline optimizerPipeline = getAutomaticOptimizerPipeline();
 		if (lmdbEvalStratFactory == null) {
 			lmdbEvalStratFactory = new LmdbEvaluationStrategyFactory(getFederatedServiceResolver());
-			lmdbEvalStratFactory.setOptimizerPipeline(automaticOptimizerPipeline);
+		}
+		if (optimizerPipeline != null) {
+			lmdbEvalStratFactory.setOptimizerPipeline(optimizerPipeline);
 		}
 		return lmdbEvalStratFactory;
 	}
 
+	private QueryOptimizerPipeline getAutomaticOptimizerPipeline() {
+		if (automaticOptimizerPipeline != null) {
+			return automaticOptimizerPipeline;
+		}
+		if (defaultEvalStratFactory != null) {
+			Optional<QueryOptimizerPipeline> optimizerPipeline = defaultEvalStratFactory.getOptimizerPipeline();
+			if (optimizerPipeline.isPresent()) {
+				automaticOptimizerPipeline = optimizerPipeline.get();
+				return automaticOptimizerPipeline;
+			}
+		}
+		if (lmdbEvalStratFactory != null) {
+			Optional<QueryOptimizerPipeline> optimizerPipeline = lmdbEvalStratFactory.getOptimizerPipeline();
+			if (optimizerPipeline.isPresent()) {
+				automaticOptimizerPipeline = optimizerPipeline.get();
+				return automaticOptimizerPipeline;
+			}
+		}
+		return null;
+	}
+
 	private void configureEvaluationStrategyFactory(EvaluationStrategyFactory factory) {
 		factory.setQuerySolutionCacheThreshold(getIterationCacheSyncThreshold());
 		factory.setTrackResultSize(isTrackResultSize());

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

@@ -1243,15 +1243,20 @@ public boolean storeTriple(long subj, long pred, long obj, long context, boolean
 				long[] quad = new long[] { subj, pred, obj, context };
 				boolean mainExplicitExists = mdb_get(writeTxn, mainIndex.getDB(true), keyVal, dataVal) == MDB_SUCCESS;
 				boolean mainInferredExists = mdb_get(writeTxn, mainIndex.getDB(false), keyVal, dataVal) == MDB_SUCCESS;
+				boolean statementAdded = !recordExistsInCacheOrMain(recordCache.getRecordState(quad, explicit),
+						explicit ? mainExplicitExists : mainInferredExists);
+				if (!statementAdded) {
+					return false;
+				}
 				if (explicit) {
 					TxnRecordCache.RecordState inferredCacheState = recordCache.getRecordState(quad, false);
-					if (inferredCacheState == TxnRecordCache.RecordState.ADD
-							|| inferredCacheState == TxnRecordCache.RecordState.ABSENT && mainInferredExists) {
+					if (recordExistsInCacheOrMain(inferredCacheState, mainInferredExists)) {
 						recordCache.removeRecord(quad, false, true);
 					}
 				}
 				// put record in cache and return immediately
-				return recordCache.storeRecord(quad, explicit, explicit ? !mainExplicitExists : !mainInferredExists);
+				recordCache.storeRecord(quad, explicit, explicit ? !mainExplicitExists : !mainInferredExists);
+				return true;
 			}
 
 			int rc = mdb_put(writeTxn, mainIndex.getDB(explicit), keyVal, dataVal, MDB_NOOVERWRITE);
@@ -1291,6 +1296,11 @@ public boolean storeTriple(long subj, long pred, long obj, long context, boolean
 		return stAdded;
 	}
 
+	private boolean recordExistsInCacheOrMain(TxnRecordCache.RecordState cacheState, boolean mainExists) {
+		return cacheState == TxnRecordCache.RecordState.ADD
+				|| cacheState == TxnRecordCache.RecordState.ABSENT && mainExists;
+	}
+
 	@Experimental
 	public void storeTriplesAligned(long[] subj, long[] pred, long[] obj, long[] context, int count, boolean explicit)
 			throws IOException {

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

@@ -14,6 +14,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.File;
@@ -47,6 +48,7 @@
 import org.eclipse.rdf4j.query.algebra.evaluation.EvaluationStrategy;
 import org.eclipse.rdf4j.query.algebra.evaluation.EvaluationStrategyFactory;
 import org.eclipse.rdf4j.query.algebra.evaluation.QueryOptimizer;
+import org.eclipse.rdf4j.query.algebra.evaluation.QueryOptimizerPipeline;
 import org.eclipse.rdf4j.query.algebra.evaluation.TripleSource;
 import org.eclipse.rdf4j.query.algebra.evaluation.impl.DefaultEvaluationStrategy;
 import org.eclipse.rdf4j.query.algebra.evaluation.impl.DefaultEvaluationStrategyFactory;
@@ -121,6 +123,25 @@ void longLivedConnectionsChooseCurrentAutomaticFactoryPerQueryCreation(@TempDir
 		}
 	}
 
+	@Test
+	void automaticFactoryPreservesConfiguredPipelineAfterSketchesBecomeReady(@TempDir File dataDir) throws Exception {
+		QueryOptimizerPipeline customPipeline = List::of;
+		LmdbStore store = new LmdbStore(dataDir, new LmdbStoreConfig("spoc"));
+		store.getEvaluationStrategyFactory().setOptimizerPipeline(customPipeline);
+		store.init();
+		try {
+			addSingleStatement(store, "urn:configured-pipeline");
+			SketchBasedJoinEstimator estimator = store.getBackingStore().getSketchBasedJoinEstimator();
+			estimator.stop();
+			estimator.rebuild();
+
+			assertTrue(estimator.isReadyNonBlocking());
+			assertSame(customPipeline, store.getEvaluationStrategyFactory().getOptimizerPipeline().orElse(null));
+		} finally {
+			store.shutDown();
+		}
+	}
+
 	@Test
 	void configuredEvaluationStrategyFactoryIsHonored() {
 		LmdbStoreConfig config = new LmdbStoreConfig();

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

@@ -439,6 +439,52 @@ void noneIsolationAppliesEstimatorUpdatesAcrossSinkFlushes(@TempDir File dataDir
 		}
 	}
 
+	@Test
+	void noneIsolationRollbackDiscardsEagerEstimatorUpdates(@TempDir File dataDir) throws Exception {
+		var vf = SimpleValueFactory.getInstance();
+		var s = vf.createIRI("urn:rollback:s");
+		var p = vf.createIRI("urn:rollback:p");
+		var o = vf.createIRI("urn:rollback:o");
+
+		LmdbStore store = new LmdbStore(dataDir, new LmdbStoreConfig("spoc"));
+		store.init();
+		try {
+			LmdbSailStore backingStore = store.getBackingStore();
+			backingStore.enableMultiThreading = false;
+			SketchBasedJoinEstimator estimator = backingStore.getSketchBasedJoinEstimator();
+			try (NotifyingSailConnection conn = store.getConnection()) {
+				conn.begin(IsolationLevels.NONE);
+				conn.addStatement(vf.createIRI("urn:rollback:seed"), p, vf.createIRI("urn:rollback:seed-o"));
+				conn.commit();
+			}
+			estimator.stop();
+			estimator.rebuild();
+			estimator.setRebuildAllowedSupplier(() -> false);
+			assertTrue(estimator.isReady());
+
+			try (NotifyingSailConnection conn = store.getConnection()) {
+				conn.begin(IsolationLevels.NONE);
+				conn.addStatement(s, p, o);
+				assertEquals(2.0d, estimator.cardinalitySingle(SketchBasedJoinEstimator.Component.P, p.stringValue()),
+						0.0d,
+						"NONE isolation applies estimator deltas eagerly before commit");
+				conn.rollback();
+			}
+
+			assertFalse(estimator.isReadyNonBlocking(),
+					"Rollback must discard eager non-isolated estimator state");
+			assertEquals(0.0d, estimator.cardinalitySingle(SketchBasedJoinEstimator.Component.P, p.stringValue()),
+					0.0d,
+					"Rolled-back statement must not remain visible to estimator sketches");
+			try (NotifyingSailConnection conn = store.getConnection()) {
+				assertFalse(conn.hasStatement(s, p, o, false),
+						"Rolled-back statement must not remain visible in the store");
+			}
+		} finally {
+			store.shutDown();
+		}
+	}
+
 	@Test
 	void commitsAndQueriesSucceedWhileEstimatorIsNotReady(@TempDir File dataDir) throws Exception {
 		var vf = SimpleValueFactory.getInstance();

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

@@ -369,6 +369,9 @@ private static LibraryJoinLoadResult loadLibraryThemeIntoDefaultContext(File dat
 				connection.commit();
 			}
 			SketchBasedJoinEstimator estimator = store.getBackingStore().getSketchBasedJoinEstimator();
+			if (!estimator.isReadyNonBlocking()) {
+				estimator.rebuild();
+			}
 			double leftRows = estimator.estimateCount(SketchBasedJoinEstimator.Component.O, null,
 					LIBRARY_LOCATED_AT.stringValue(), null, null);
 			double rightRows = estimator.estimateCount(SketchBasedJoinEstimator.Component.S, null,

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

@@ -203,7 +203,8 @@ public void testCachedDuplicateExplicitAddKeepsContextCountsBalanced() throws Ex
 
 			forceRecordCache(cachedStore);
 			cachedStore.startTransaction();
-			cachedStore.storeTriple(subject, predicate, object, context, true);
+			assertFalse(cachedStore.storeTriple(subject, predicate, object, context, true),
+					"Cached duplicate explicit add should not report a new statement");
 			cachedStore.commit();
 
 			cachedStore.startTransaction();