Fix cypher projection query row estimation bug

Co-authored-by: Paul Horn <paul.horn@neotechnology.com>
Co-authored-by: Sören Reichardt <soren.reichardt@neo4j.com>

Author: 3 people

cypher-aggregation/build.gradle

@@ -30,6 +30,7 @@ dependencies {
     implementation project(':neo4j-values')
     implementation project(':progress-tracking')
     implementation project(':string-formatting')
+    implementation project(':transaction')
     implementation project(':triplet-graph-builder')
 
 

cypher-aggregation/src/integrationTest/java/org/neo4j/gds/projection/CypherAggregationTest.java

@@ -28,6 +28,7 @@
 import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
+import org.neo4j.configuration.GraphDatabaseSettings;
 import org.neo4j.exceptions.KernelException;
 import org.neo4j.gds.BaseProcTest;
 import org.neo4j.gds.BaseTest;
@@ -55,6 +56,8 @@
 import org.neo4j.kernel.api.CypherScope;
 import org.neo4j.kernel.api.procedure.CallableUserAggregationFunction;
 import org.neo4j.kernel.api.procedure.GlobalProcedures;
+import org.neo4j.test.TestDatabaseManagementServiceBuilder;
+import org.neo4j.test.extension.ExtensionCallback;
 
 import java.util.List;
 import java.util.Map;
@@ -1218,6 +1221,29 @@ void testLargerGraphSize() {
         }
     }
 
+    @Nested
+    class EstimateRowsTest extends BaseTest {
+
+        @Override
+        @ExtensionCallback
+        protected void configuration(TestDatabaseManagementServiceBuilder builder) {
+            super.configuration(builder.setConfig(GraphDatabaseSettings.log_queries_obfuscate_literals, true));
+        }
+
+        @Test
+        void explainingWithObfuscatedLiteralsDoesntRuinTheDay() {
+            assertThatCode(() -> runQuery("RETURN gds.graph.project('g', 0, 1)")).doesNotThrowAnyException();
+        }
+
+        @Test
+        void notLiterals() {
+            assertThatCode(() -> runQuery(
+                "RETURN gds.graph.project($g, $x, $x)",
+                Map.of("g", "g", "x", 0)
+            )).doesNotThrowAnyException();
+        }
+    }
+
     @Nested
     class InverseGraphTest extends BaseTest {
 

cypher-aggregation/src/main/java/org/neo4j/gds/projection/CypherAggregation.java

@@ -28,6 +28,7 @@
 import org.neo4j.gds.core.utils.progress.TaskStore;
 import org.neo4j.gds.logging.LogAdapter;
 import org.neo4j.gds.metrics.MetricsFacade;
+import org.neo4j.gds.transaction.DatabaseTransactionContext;
 import org.neo4j.internal.kernel.api.exceptions.ProcedureException;
 import org.neo4j.internal.kernel.api.procs.Neo4jTypes;
 import org.neo4j.internal.kernel.api.procs.QualifiedName;
@@ -112,7 +113,7 @@ public UserAggregationReducer createReducer(Context ctx) throws ProcedureExcepti
             ? WriteMode.NONE
             : WriteMode.LOCAL;
 
-        var queryEstimator = QueryEstimator.fromTransaction(transaction);
+        var queryEstimator = QueryEstimator.fromTransaction(DatabaseTransactionContext.of(databaseService, transaction));
 
         return new ProductGraphAggregator(
             DatabaseId.of(databaseService.databaseName()),

cypher-aggregation/src/main/java/org/neo4j/gds/projection/QueryEstimator.java

@@ -20,12 +20,12 @@
 package org.neo4j.gds.projection;
 
 import org.neo4j.gds.core.utils.progress.tasks.Task;
-import org.neo4j.graphdb.Transaction;
+import org.neo4j.gds.transaction.TransactionContext;
 
 public interface QueryEstimator {
     int estimateRows(String query);
 
-    static QueryEstimator fromTransaction(Transaction transaction) {
+    static QueryEstimator fromTransaction(TransactionContext transaction) {
         return new TxQueryEstimator(transaction);
     }
 
@@ -35,14 +35,14 @@ static QueryEstimator empty() {
 }
 
 final class TxQueryEstimator implements QueryEstimator {
-    private final Transaction transaction;
+    private final TransactionContext transactionContext;
 
-    TxQueryEstimator(Transaction transaction) {
-        this.transaction = transaction;
+    TxQueryEstimator(TransactionContext transactionContext) {
+        this.transactionContext = transactionContext;
     }
 
     @Override
     public int estimateRows(String query) {
-        return QueryRowEstimationUtil.estimatedRows(transaction, query);
+        return QueryRowEstimationUtil.estimatedRows(transactionContext, query);
     }
 }

cypher-aggregation/src/main/java/org/neo4j/gds/projection/QueryRowEstimationUtil.java

@@ -20,8 +20,8 @@
 package org.neo4j.gds.projection;
 
 import org.neo4j.gds.core.utils.progress.tasks.Task;
+import org.neo4j.gds.transaction.TransactionContext;
 import org.neo4j.graphdb.ExecutionPlanDescription;
-import org.neo4j.graphdb.Transaction;
 
 import java.util.Optional;
 
@@ -34,9 +34,11 @@ private QueryRowEstimationUtil() {}
      * of the cypher aggregation node. This child node contains the estimated number of rows that
      * will be returned by the query.
      */
-    static int estimatedRows(Transaction tx, String query) {
-        try {
+    static int estimatedRows(TransactionContext transactionContext, String query) {
+        //noinspection resource
+        try (var tx = transactionContext.fork().transaction()) {
             var executionPlan = tx.execute(explainQuery(query)).getExecutionPlanDescription();
+            tx.commit();
             var aggregationChild = findChildOfRemoteAggregation(executionPlan);
             return aggregationChild
                 .map(child -> ((Number) child.getArguments().get("EstimatedRows")).intValue())

transaction/src/main/java/org/neo4j/gds/transaction/TransactionContext.java

@@ -106,6 +106,13 @@ public KernelTransaction kernelTransaction() {
             return tx.kernelTransaction();
         }
 
+        /**
+         * @return The current {@link org.neo4j.graphdb.Transaction}.
+         */
+        public Transaction transaction() {
+            return tx;
+        }
+
         /**
          * Closes the underlying transaction.
          */