Feat/4141 gql compliance (#4477)

* test: [#4141] add ISO GQL three-valued (trinary) logic regression matrix

Locks in the existing UNKNOWN/NULL three-valued logic behavior with a
13-case end-to-end regression test covering AND/OR/NOT/XOR null
propagation, comparisons against null, IN with null elements, WHERE
non-match coercion vs RETURN null preservation, and IS NULL concrete
booleans. Confirms ArcadeDB represents UNKNOWN as null-in-boolean-context
per ISO/IEC 39075, so no separate UNKNOWN value type is required.

Related to #4141

* feat: [#4141] reject deprecated Cypher syntax (PERIODIC COMMIT, {param}) with GQL hints

ISO/IEC 39075 (GQL) / Cypher 25 cleanup. Adds a token-stream pre-scan in
Cypher25AntlrParser that rejects removed legacy syntax with a clear,
actionable message instead of a cryptic ANTLR error:
  - 'PERIODIC COMMIT'  -> use 'CALL { ... } IN TRANSACTIONS [OF n ROWS]'
  - legacy '{param}'   -> use '$param'

The scan runs on the default token channel so matches inside string
literals/comments are never flagged, distinguishes a legacy '{name}' from
a map projection ('var {name}') and brace-block expressions
(EXISTS/COUNT/COLLECT/CALL { ... }), and excludes numeric '{n}'/'{n,m}'
quantified-path-pattern quantifiers. CypherStatementCache no longer
double-wraps CommandParsingException so the hint surfaces at top level.

Related to #4141

* test: [#4141] verify GQL grouping-key enforcement; surface validation errors

Adds regression coverage for ISO GQL explicit-grouping rules: explicit
single/multiple grouping keys work, an aggregation mixed with a grouping
key is allowed, and an aggregation mixed with a non-grouping reference is
rejected as 'AmbiguousAggregationExpression' (the enforcement already
lived in CypherSemanticValidator; this proves it). Also covers rejection
of aggregation in WHERE.

Cypher25AntlrParser now rethrows CommandParsingException as-is instead of
burying it under a generic 'Failed to parse Cypher query' wrapper, so
semantic-validation and explicit parse messages surface at top level.

Related to #4141

* refactor: [#4141] address PR review - hot-path pre-filter, idempotent tests

Code review follow-ups for the GQL deprecated-syntax check:
- Add an allocation-free pre-filter to checkDeprecatedSyntax so queries
  without '{' or 'PERIODIC' skip the second lex pass entirely (keeps GC
  pressure off the parse hot path).
- Move checkDeprecatedSyntax inside the parse try so any unexpected
  exception also gets the wrapping context; CommandParsingException is
  still re-thrown as-is so hints surface at top level (single control path).
- Document the intentional BRACE_BLOCK_KEYWORDS false-negative and the
  PARAM_NAME numeric-exclusion rationale.
- Make the three Issue4141 test setUp methods idempotent (drop a leftover
  db from an interrupted run) to avoid flaky re-runs on a dirty workspace.
- Add brace-block false-positive tests (EXISTS { }, COUNT { }) and a note
  on the sorting helper in the trinary-logic test.

Related to #4141

* refactor: [#4141] single-lex deprecated-syntax scan; close map-projection false positive

Second review round on the deprecated-syntax check:
- Reuse the main parse's token stream instead of re-lexing, and walk it with
  a 3-token sliding window (no intermediate ArrayList) - removes the
  double-lex and the extra list allocation on the parse hot path.
- Replace the name-token guard with a value-operator allowlist: a legacy
  '{name}' is only flagged when the '{' follows an operator/punctuation
  (=, comparison, arithmetic, ',', '(', '['), where a map projection is
  impossible. This closes the false positive on 'type{name}' where the
  projected-on variable is a keyword token (NAME/TYPE are legal variable
  names); the cost is only a harmless false negative after a keyword.
- Trim what-comments per CLAUDE.md, keeping the non-obvious WHYs.
- Align test import ordering with the project convention; add tests for the
  keyword-named-variable projection and for 'PERIODIC COMMIT' inside a
  string literal.

Related to #4141

Author: lvca

engine/src/main/java/com/arcadedb/query/opencypher/parser/Cypher25AntlrParser.java

@@ -26,6 +26,9 @@
 import org.antlr.v4.runtime.CommonTokenStream;
 import org.antlr.v4.runtime.Token;
 
+import java.util.Set;
+import java.util.regex.Pattern;
+
 /**
  * Cypher parser using ANTLR4 with the official Cypher 2.5 grammar.
  * This replaces the regex-based parser with a proper grammar-based implementation.
@@ -45,13 +48,14 @@ public CypherStatement parse(final String query) {
       throw new CommandParsingException("Query cannot be empty");
 
     try {
-      // Create lexer
       final Cypher25Lexer lexer = new Cypher25Lexer(CharStreams.fromString(query));
-
-      // Create token stream
       final CommonTokenStream tokens = new CommonTokenStream(lexer);
 
-      // Create parser
+      // Reject deprecated/legacy Cypher syntax with an actionable hint before the generic ANTLR parse,
+      // which would otherwise produce a cryptic "Unexpected input" error (issue #4141). Reuses this same
+      // token stream, so no second lex. The catch below re-throws CommandParsingException as-is.
+      checkDeprecatedSyntax(query, tokens);
+
       final Cypher25Parser parser = new Cypher25Parser(tokens);
 
       // Custom error handling
@@ -90,8 +94,77 @@ public CypherStatement parse(final String query) {
 
       return statement;
 
+    } catch (final CommandParsingException e) {
+      // semantic-validation and explicit parse errors already carry a clear, actionable message
+      // (e.g. AmbiguousAggregationExpression, UndefinedVariable, "Unexpected input ...") - surface it
+      throw e;
     } catch (final Exception e) {
       throw new CommandParsingException("Failed to parse Cypher query: " + query, e);
     }
   }
+
+  // A legacy named parameter name (the required leading letter/underscore also excludes purely numeric
+  // braces, so quantified-path-pattern quantifiers '{n}'/'{n,m}' are never mistaken for one).
+  private static final Pattern PARAM_NAME = Pattern.compile("[A-Za-z_][A-Za-z0-9_]*");
+
+  // Operator/punctuation tokens after which a '{...}' begins a value, never a map projection (which must
+  // attach to a variable). Restricting the legacy-'{name}' check to these positions guarantees we never
+  // flag a real map projection - including 'type{name}', where the projected-on variable is a keyword
+  // token (NAME, TYPE, ... are legal variable names) and so is indistinguishable from a clause keyword.
+  // The cost is only a false negative (a legacy param after a keyword like IN/RETURN yields the generic
+  // parser error instead of the hint), which is harmless.
+  private static final Set<Integer> VALUE_PREFIX_TOKENS = Set.of(
+      Cypher25Lexer.EQ, Cypher25Lexer.NEQ, Cypher25Lexer.LT, Cypher25Lexer.GT, Cypher25Lexer.LE, Cypher25Lexer.GE,
+      Cypher25Lexer.PLUS, Cypher25Lexer.MINUS, Cypher25Lexer.TIMES, Cypher25Lexer.DIVIDE, Cypher25Lexer.POW,
+      Cypher25Lexer.COMMA, Cypher25Lexer.LPAREN, Cypher25Lexer.LBRACKET);
+
+  /**
+   * Rejects deprecated syntax removed under ISO/IEC 39075 (GQL) / Cypher 25 with a hint at the supported
+   * replacement: {@code PERIODIC COMMIT} (use {@code CALL { ... } IN TRANSACTIONS}) and legacy
+   * {@code {param}} (use {@code $param}). Scans {@code tokens} on the default channel, so occurrences
+   * inside string literals or comments are never matched.
+   */
+  private static void checkDeprecatedSyntax(final String query, final CommonTokenStream tokens) {
+    // Allocation-free pre-filter: neither deprecated form is possible without a '{' or the word PERIODIC,
+    // so the token walk is skipped for the vast majority of queries.
+    if (query.indexOf('{') < 0 && !containsIgnoreCase(query, "PERIODIC"))
+      return;
+
+    tokens.fill(); // the parser reuses this same fully-buffered stream
+
+    // Sliding window of the three most recent default-channel tokens: t1 is the immediate predecessor of
+    // the current token, t2 the one before it, t3 the one before that. No intermediate list is allocated.
+    Token t3 = null, t2 = null, t1 = null;
+    for (final Token tok : tokens.getTokens()) {
+      if (tok.getChannel() != Token.DEFAULT_CHANNEL || tok.getType() == Token.EOF)
+        continue;
+
+      if (t1 != null && "PERIODIC".equalsIgnoreCase(t1.getText()) && "COMMIT".equalsIgnoreCase(tok.getText()))
+        throw new CommandParsingException(
+            "Deprecated syntax: 'PERIODIC COMMIT' is no longer supported. Use 'CALL { ... } IN TRANSACTIONS [OF n ROWS]' instead");
+
+      // legacy '{name}': t2='{', t1=name, tok='}', preceded (t3) by a value operator/punctuation
+      if (tok.getType() == Cypher25Lexer.RCURLY
+          && t2 != null && t2.getType() == Cypher25Lexer.LCURLY
+          && t1 != null && PARAM_NAME.matcher(t1.getText()).matches()
+          && t3 != null && VALUE_PREFIX_TOKENS.contains(t3.getType())) {
+        final String name = t1.getText();
+        throw new CommandParsingException(
+            "Deprecated syntax: legacy parameter '{" + name + "}' is no longer supported. Use '$" + name + "' instead");
+      }
+
+      t3 = t2;
+      t2 = t1;
+      t1 = tok;
+    }
+  }
+
+  // Allocation-free case-insensitive substring search (avoids String.toUpperCase() on the parse hot path).
+  private static boolean containsIgnoreCase(final String s, final String sub) {
+    final int max = s.length() - sub.length();
+    for (int i = 0; i <= max; i++)
+      if (s.regionMatches(true, i, sub, 0, sub.length()))
+        return true;
+    return false;
+  }
 }

engine/src/main/java/com/arcadedb/query/opencypher/query/CypherStatementCache.java

@@ -79,6 +79,9 @@ public CypherStatement get(final String query) {
   private CypherStatement parse(final String query) throws CommandParsingException {
     try {
       return parser.parse(query);
+    } catch (final CommandParsingException e) {
+      // already a clear, actionable parsing error (e.g. deprecated-syntax hints) - keep it at top level
+      throw e;
     } catch (final Exception e) {
       throw new CommandParsingException("Error parsing OpenCypher query: " + query, e);
     }

engine/src/test/java/com/arcadedb/query/opencypher/Issue4141DeprecatedSyntaxTest.java

@@ -0,0 +1,169 @@
+/*
+ * Copyright © 2021-present Arcade Data Ltd (info@arcadedata.com)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-FileCopyrightText: 2021-present Arcade Data Ltd (info@arcadedata.com)
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package com.arcadedb.query.opencypher;
+
+import com.arcadedb.database.Database;
+import com.arcadedb.database.DatabaseFactory;
+import com.arcadedb.exception.CommandParsingException;
+import com.arcadedb.query.sql.executor.ResultSet;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.Map;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/**
+ * Issue #4141 (ISO GQL / Cypher 25 cleanup): deprecated/legacy syntax must be rejected with a clear,
+ * actionable error pointing at the supported replacement.
+ * <ul>
+ *   <li>{@code PERIODIC COMMIT} -&gt; {@code CALL { ... } IN TRANSACTIONS}</li>
+ *   <li>legacy {@code {param}} -&gt; {@code $param}</li>
+ * </ul>
+ * Also guards against false positives: map projections ({@code n{name}}) and brace-block expressions
+ * ({@code COUNT { ... }}) must still parse, and the modern {@code $param} form must keep working.
+ */
+public class Issue4141DeprecatedSyntaxTest {
+  private Database database;
+
+  @BeforeEach
+  void setUp() {
+    final DatabaseFactory factory = new DatabaseFactory("./target/databases/testIssue4141DeprecatedSyntax");
+    if (factory.exists())
+      factory.open().drop(); // defend against a leftover db from a previously interrupted run
+    database = factory.create();
+    database.getSchema().createVertexType("Person");
+    database.transaction(() -> {
+      database.command("opencypher", "CREATE (p:Person {name: 'Alice', age: 30})");
+      database.command("opencypher", "CREATE (p:Person {name: 'Bob', age: 25})");
+    });
+  }
+
+  @AfterEach
+  void tearDown() {
+    if (database != null) {
+      database.drop();
+      database = null;
+    }
+  }
+
+  // ---- Deprecated syntax is rejected with an actionable message -------------------------------
+
+  @Test
+  void periodicCommitIsRejectedWithHint() {
+    assertThatThrownBy(() ->
+        database.command("opencypher", "USING PERIODIC COMMIT 500 MATCH (p:Person) RETURN p"))
+        .isInstanceOf(CommandParsingException.class)
+        .hasMessageContaining("PERIODIC COMMIT")
+        .hasMessageContaining("IN TRANSACTIONS");
+  }
+
+  @Test
+  void legacyParameterIsRejectedWithHint() {
+    assertThatThrownBy(() ->
+        database.command("opencypher", "MATCH (p:Person) WHERE p.name = {name} RETURN p"))
+        .isInstanceOf(CommandParsingException.class)
+        .hasMessageContaining("{name}")
+        .hasMessageContaining("$name");
+  }
+
+  @Test
+  void legacyParameterIsRejectedEvenWhenSuppliedAsArg() {
+    assertThatThrownBy(() ->
+        database.command("opencypher", "MATCH (p:Person) WHERE p.name = {name} RETURN p", Map.of("name", "Alice")))
+        .isInstanceOf(CommandParsingException.class)
+        .hasMessageContaining("$name");
+  }
+
+  // ---- Supported syntax keeps working (no false positives) ------------------------------------
+
+  @Test
+  void modernDollarParameterStillWorks() {
+    try (final ResultSet rs = database.query("opencypher", "MATCH (p:Person) WHERE p.name = $name RETURN p.age AS age",
+        Map.of("name", "Alice"))) {
+      assertThat(rs.hasNext()).isTrue();
+      assertThat((Integer) rs.next().getProperty("age")).isEqualTo(30);
+    }
+  }
+
+  @Test
+  void mapProjectionIsNotMistakenForLegacyParameter() {
+    // 'p{name}' has the same token shape as a legacy parameter ('{' name '}') but is a map projection
+    // (variable LCURLY variable RCURLY). It must parse, projecting the in-scope 'name' variable.
+    try (final ResultSet rs = database.query("opencypher",
+        "MATCH (p:Person) WHERE p.name = 'Alice' WITH p, p.name AS name RETURN p{name} AS proj")) {
+      assertThat(rs.hasNext()).isTrue();
+      final Map<String, Object> proj = rs.next().getProperty("proj");
+      assertThat(proj).containsEntry("name", "Alice");
+    }
+  }
+
+  @Test
+  void mapLiteralIsNotMistakenForLegacyParameter() {
+    try (final ResultSet rs = database.query("opencypher", "RETURN {name: 'Alice'} AS m")) {
+      assertThat(rs.hasNext()).isTrue();
+      final Map<String, Object> m = rs.next().getProperty("m");
+      assertThat(m).containsEntry("name", "Alice");
+    }
+  }
+
+  @Test
+  void existsBraceBlockIsNotMistakenForLegacyParameter() {
+    // 'EXISTS { ... }' is a brace-block expression; the following '{' must not trigger the legacy-param check.
+    try (final ResultSet rs = database.query("opencypher",
+        "MATCH (p:Person) WHERE EXISTS { (p) } RETURN p.name AS name ORDER BY name")) {
+      assertThat(rs.hasNext()).isTrue();
+    }
+  }
+
+  @Test
+  void countBraceBlockIsNotMistakenForLegacyParameter() {
+    // 'COUNT { ... }' is a brace-block expression; the following '{' must not trigger the legacy-param check.
+    try (final ResultSet rs = database.query("opencypher",
+        "MATCH (p:Person) RETURN p.name AS name, COUNT { (p) } AS c ORDER BY name")) {
+      assertThat(rs.hasNext()).isTrue();
+      assertThat(((Number) rs.next().getProperty("c")).longValue()).isEqualTo(1L);
+    }
+  }
+
+  @Test
+  void mapProjectionOnKeywordNamedVariableIsNotMistakenForLegacyParameter() {
+    // 'type' is a keyword token but a legal variable name, so 'type{name}' is a map projection whose '{'
+    // is preceded by the variable (not a value operator) and must not be flagged as a legacy parameter.
+    try (final ResultSet rs = database.query("opencypher",
+        "MATCH (type:Person) WHERE type.name = 'Alice' WITH type, type.name AS name RETURN type{name} AS proj")) {
+      assertThat(rs.hasNext()).isTrue();
+      final Map<String, Object> proj = rs.next().getProperty("proj");
+      assertThat(proj).containsEntry("name", "Alice");
+    }
+  }
+
+  @Test
+  void periodicCommitInsideStringLiteralIsNotFlagged() {
+    // The token scan runs on the default channel, so 'PERIODIC COMMIT' inside a string literal (a single
+    // string token) is not two keyword tokens and must not be flagged.
+    try (final ResultSet rs = database.query("opencypher", "RETURN 'PERIODIC COMMIT' AS s")) {
+      assertThat(rs.hasNext()).isTrue();
+      assertThat((String) rs.next().getProperty("s")).isEqualTo("PERIODIC COMMIT");
+    }
+  }
+}