From acc8ea7effe9db6c5fc946857deb4062760a7c8b Mon Sep 17 00:00:00 2001 From: robfrank Date: Tue, 3 Mar 2026 11:55:33 +0100 Subject: [PATCH] fix: pass parameters when checking query idempotency in Cypher, Gremlin, and MongoDB engines The idempotency checks added in aac51f1c9 called analyze(query) without parameters, causing failures on parameterized queries ($p, params, etc.) because the engines tried to compile/execute the query without bindings. Also fixes a TOCTOU race in PageManager.flushPage() where the database could close between the isOpen() check and getSchema(), and recognizes the ArcadeDB MongoDB { collection: '...', query: {...} } format as a read operation. Co-Authored-By: Claude Opus 4.6 --- .../java/com/arcadedb/engine/PageManager.java | 17 ++++++++++++----- .../cypher/query/CypherQueryEngine.java | 14 ++++++++++---- .../gremlin/query/GremlinQueryEngine.java | 14 ++++++++++---- .../arcadedb/mongo/query/MongoQueryEngine.java | 3 ++- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/engine/src/main/java/com/arcadedb/engine/PageManager.java b/engine/src/main/java/com/arcadedb/engine/PageManager.java index eb74480a8b..12fb224aa1 100644 --- a/engine/src/main/java/com/arcadedb/engine/PageManager.java +++ b/engine/src/main/java/com/arcadedb/engine/PageManager.java @@ -24,6 +24,7 @@ import com.arcadedb.database.DatabaseInternal; import com.arcadedb.exception.ConcurrentModificationException; import com.arcadedb.exception.ConfigurationException; +import com.arcadedb.exception.DatabaseIsClosedException; import com.arcadedb.exception.DatabaseMetadataException; import com.arcadedb.log.LogManager; import com.arcadedb.utility.CallableNoReturn; @@ -372,13 +373,19 @@ protected void flushPage(final MutablePage page) throws IOException { totalPagesWrittenSize.addAndGet(written); }); - final PaginatedComponent component = (PaginatedComponent) database.getSchema().getFileByIdIfExists(fileId); - if (component != null) - component.updatePageCount(page.pageId.getPageNumber() + 1); + try { + final PaginatedComponent component = (PaginatedComponent) database.getSchema().getFileByIdIfExists(fileId); + if (component != null) + component.updatePageCount(page.pageId.getPageNumber() + 1); - totalPagesWritten.incrementAndGet(); + totalPagesWritten.incrementAndGet(); - database.getTransactionManager().notifyPageFlushed(page); + database.getTransactionManager().notifyPageFlushed(page); + } catch (final DatabaseIsClosedException e) { + // The database was closed concurrently after the isOpen() check above. + // The page data has already been written to disk, so we can safely skip + // the metadata updates. + } } else LogManager.instance() diff --git a/gremlin/src/main/java/com/arcadedb/cypher/query/CypherQueryEngine.java b/gremlin/src/main/java/com/arcadedb/cypher/query/CypherQueryEngine.java index 681d74230a..a7e0f93d52 100644 --- a/gremlin/src/main/java/com/arcadedb/cypher/query/CypherQueryEngine.java +++ b/gremlin/src/main/java/com/arcadedb/cypher/query/CypherQueryEngine.java @@ -57,16 +57,22 @@ public AnalyzedQuery analyze(final String query) { @Override public ResultSet query(final String query, final ContextConfiguration configuration, final Map parameters) { - if (!analyze(query).isIdempotent()) + final ArcadeCypher arcadeCypher = arcadeGraph.cypher(query); + arcadeCypher.setParameters(parameters); + if (!arcadeCypher.parse().isIdempotent()) throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent"); return command(query, configuration, parameters); } @Override public ResultSet query(final String query, final ContextConfiguration configuration, final Object... parameters) { - if (!analyze(query).isIdempotent()) - throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent"); - return command(query, null, parameters); + if (parameters.length % 2 != 0) + throw new IllegalArgumentException("Command parameters must be as pairs `, `"); + + final Map map = new LinkedHashMap<>(parameters.length / 2); + for (int i = 0; i < parameters.length; i += 2) + map.put((String) parameters[i], parameters[i + 1]); + return query(query, configuration, map); } @Override diff --git a/gremlin/src/main/java/com/arcadedb/gremlin/query/GremlinQueryEngine.java b/gremlin/src/main/java/com/arcadedb/gremlin/query/GremlinQueryEngine.java index 2d683347f9..f79e98de8a 100644 --- a/gremlin/src/main/java/com/arcadedb/gremlin/query/GremlinQueryEngine.java +++ b/gremlin/src/main/java/com/arcadedb/gremlin/query/GremlinQueryEngine.java @@ -43,16 +43,22 @@ public String getLanguage() { @Override public ResultSet query(final String query, ContextConfiguration configuration, final Map parameters) { - if (!analyze(query).isIdempotent()) + final ArcadeGremlin arcadeGremlin = arcadeGraph.gremlin(query); + arcadeGremlin.setParameters(parameters); + if (!arcadeGremlin.parse().isIdempotent()) throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent"); return command(query, null, parameters); } @Override public ResultSet query(final String query, ContextConfiguration configuration, final Object... parameters) { - if (!analyze(query).isIdempotent()) - throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent"); - return command(query, null, parameters); + if (parameters.length % 2 != 0) + throw new IllegalArgumentException("Command parameters must be as pairs `, `"); + + final Map map = new HashMap<>(parameters.length / 2); + for (int i = 0; i < parameters.length; i += 2) + map.put((String) parameters[i], parameters[i + 1]); + return query(query, configuration, map); } @Override diff --git a/mongodbw/src/main/java/com/arcadedb/mongo/query/MongoQueryEngine.java b/mongodbw/src/main/java/com/arcadedb/mongo/query/MongoQueryEngine.java index b43a52fd2b..c9c85cc413 100644 --- a/mongodbw/src/main/java/com/arcadedb/mongo/query/MongoQueryEngine.java +++ b/mongodbw/src/main/java/com/arcadedb/mongo/query/MongoQueryEngine.java @@ -79,7 +79,8 @@ private static Set detectMongoOperationTypes(final String query) || upper.contains("\"REMOVE\"")) return CollectionUtils.singletonSet(OperationType.DELETE); if (upper.contains("\"FIND\"") || upper.contains("\"AGGREGATE\"") || upper.contains("\"COUNT\"") - || upper.contains("\"DISTINCT\"")) + || upper.contains("\"DISTINCT\"") + || upper.contains("COLLECTION")) return CollectionUtils.singletonSet(OperationType.READ); if (upper.contains("\"CREATEINDEX\"") || upper.contains("\"CREATECOLLECTION\"") || upper.contains("\"DROP\"") || upper.contains("\"DROPCOLLECTION\"") || upper.contains("\"DROPINDEX\""))