From 265d3d3d99ff2f0f8de183625837ed0a3339c949 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sat, 31 Oct 2026 20:11:49 +1000 Subject: [PATCH 1/2] Refuse an aggregate in a WHERE, and one beside a variable no item groups by in a body or an ORDER BY, as drivine4j does, at Cypher spec v0.13.0 Ports drivine4j's #489 and #482 (merged as liberation-data/drivine4j#490), with the spec's liberation-data/drivine-cypher-spec#38. - New check_aggregates (drivine/cypher/sql/aggregates.py), run before compiling beside check_variable_kinds: an aggregate in the WHERE of a MATCH, a WITH, a list or pattern comprehension, a quantifier, a quantified path pattern, EXISTS { } or COUNT { } is refused (Neo4j's 42I24). It reads every clause, UNION branches, UNWIND, SET, MERGE's ON CREATE and ON MATCH, CREATE, FOREACH and nested CALLs and subqueries among them, but not a subquery's own projection. - _require_explicit_grouping also reads an aggregating item's comprehension, quantifier, reduce and pattern bodies, and, after items that aggregate, an aggregating ORDER BY term (Neo4j's 42I18). It matches whole key expressions, so a body naming a grouping key or a key property compiles; a map projection of a renamed key reads the key, and a renamed key in an order term's pattern is refused. - drivine4j's CompilerRefusalTest and CypherQueryContract cases are ported. Fixes #306 Fixes #307 --- drivine/cypher/sql/aggregates.py | 217 ++++++++++++++++++ drivine/cypher/sql/sql_cypher_compiler.py | 134 ++++++++++- tests/cypher/VERSION | 2 +- tests/cypher/queries.yaml | 56 ++++- tests/cypher/refusals.yaml | 86 +++++++ tests/integration/test_cypher_compiled.py | 61 +++++ .../sync_integration/test_cypher_compiled.py | 61 +++++ tests/unit/test_cypher_refusals.py | 183 +++++++++++++++ 8 files changed, 787 insertions(+), 13 deletions(-) create mode 100644 drivine/cypher/sql/aggregates.py diff --git a/drivine/cypher/sql/aggregates.py b/drivine/cypher/sql/aggregates.py new file mode 100644 index 0000000..30b39c4 --- /dev/null +++ b/drivine/cypher/sql/aggregates.py @@ -0,0 +1,217 @@ +""" +Where a statement aggregates as Neo4j refuses it when it plans the statement: see `check_aggregates`. As drivine4j's +Aggregates. +""" + +from __future__ import annotations + +from collections.abc import Callable, Sequence + +from drivine.cypher.ast.clause import ( + Call, + Clause, + Create, + Delete, + Foreach, + Match, + Merge, + Return, + SetClause, + Unwind, + With, +) +from drivine.cypher.ast.expression import ( + CountStar, + Exists, + Expression, + Function, + ListComprehension, + PatternComprehension, + PatternPredicate, + Quantifier, + Reduce, + Subquery, +) +from drivine.cypher.ast.pattern import Pattern +from drivine.cypher.ast.projection import Projection +from drivine.cypher.ast.query import Query +from drivine.cypher.ast.set_item import SetAdd, SetItem, SetProperty, SetReplace +from drivine.drivine_error import DrivineError + + +def check_aggregates( + query: Query, + is_aggregate: Callable[[Expression], bool], + children: Callable[[Expression], list[Expression]], +) -> None: + """ + Refuses the statement where it aggregates as Cypher does not: an aggregate in a `WHERE` (Neo4j's 42I24, + InvalidAggregation), of a `MATCH`, a `WITH`, a comprehension, a quantifier, a pattern's or `EXISTS { … }`'s, in a + `CALL { … }` or a subquery too. Read from the statement alone, before it is compiled, so a statement that compiles + is compiled as it was. A subquery's own query, `COUNT { MATCH … RETURN count(*) }`, aggregates in its own + projections. + + `is_aggregate` and `children` are the compiler's, passed in so that neither module imports the other. + """ + _Aggregates(is_aggregate, children).query(query) + + +class _Aggregates: + def __init__( + self, + is_aggregate: Callable[[Expression], bool], + children: Callable[[Expression], list[Expression]], + ) -> None: + self._is_aggregate = is_aggregate + self._children = children + + def query(self, query: Query) -> None: + for clauses in (query.clauses, *(it.clauses for it in query.unions)): + self._clauses(clauses) + + def _clauses(self, clauses: Sequence[Clause]) -> None: + for clause in clauses: + if isinstance(clause, Match): + for pattern in clause.patterns: + self._pattern(pattern) + self._where(clause.where) + elif isinstance(clause, Unwind): + self._expression(clause.list) + elif isinstance(clause, With): + self._projection(clause.projection) + self._where(clause.where) + elif isinstance(clause, Create): + for pattern in clause.patterns: + self._pattern(pattern) + elif isinstance(clause, Merge): + self._pattern(clause.pattern) + for item in (*clause.on_create, *clause.on_match): + self._set_item(item) + elif isinstance(clause, SetClause): + for item in clause.items: + self._set_item(item) + elif isinstance(clause, Delete): + for expression in clause.expressions: + self._expression(expression) + elif isinstance(clause, Call): + self.query(clause.query) + elif isinstance(clause, Foreach): + self._expression(clause.list) + self._clauses(clause.clauses) + elif isinstance(clause, Return): + self._projection(clause.projection) + # A REMOVE holds no expression. + + def _set_item(self, item: SetItem) -> None: + if isinstance(item, (SetProperty, SetReplace, SetAdd)): + self._expression(item.value) + + def _projection(self, projection: Projection) -> None: + for item in projection.items: + self._expression(item.expression) + for sort in projection.order_by: + self._expression(sort.expression) + self._expression(projection.skip) + self._expression(projection.limit) + + def _pattern(self, pattern: Pattern) -> None: + for node in pattern.nodes: + self._expression(node.properties) + for relationship in pattern.relationships: + self._expression(relationship.properties) + if relationship.group is not None: + self._pattern(relationship.group.pattern) + self._where(relationship.group.where) + + def _where(self, where: Expression | None) -> None: + """Refuses a `WHERE` that aggregates outside a subquery's own query; then reads it as any expression.""" + if where is None: + return + aggregate = self._aggregate_in(where) + if aggregate is not None: + raise DrivineError( + f"{_named(aggregate)} aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH " + "first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + ) + self._expression(where) + + def _aggregate_in(self, expression: Expression) -> Expression | None: + """The first aggregate the expression holds, a comprehension's, a pattern's or `EXISTS { … }`'s among them, but + not one a subquery's own query holds.""" + if self._is_aggregate(expression): + return expression + for part in self._parts(expression): + found = self._aggregate_in(part) + if found is not None: + return found + return None + + def _expression(self, expression: Expression | None) -> None: + """Reads the expression for the `WHERE`s and subqueries it holds.""" + if expression is None: + return + if isinstance(expression, Subquery): + self.query(expression.query) + elif isinstance(expression, ListComprehension): + self._expression(expression.list) + self._where(expression.where) + self._expression(expression.projection) + elif isinstance(expression, Quantifier): + self._expression(expression.list) + self._where(expression.where) + elif isinstance(expression, PatternComprehension): + self._pattern(expression.pattern) + self._where(expression.where) + self._expression(expression.projection) + elif isinstance(expression, Exists): + for pattern in expression.patterns: + self._pattern(pattern) + self._where(expression.where) + elif isinstance(expression, PatternPredicate): + self._pattern(expression.pattern) + else: + for part in self._parts(expression): + self._expression(part) + + def _parts(self, expression: Expression) -> list[Expression]: + """Each expression the expression holds directly: a comprehension's, a quantifier's, a reduce's and a + pattern's parts too.""" + if isinstance(expression, ListComprehension): + return _present(expression.list, expression.where, expression.projection) + if isinstance(expression, Quantifier): + return _present(expression.list, expression.where) + if isinstance(expression, Reduce): + return [expression.initial, expression.list, expression.expression] + if isinstance(expression, PatternComprehension): + return [*_pattern_parts(expression.pattern), *_present(expression.where, expression.projection)] + if isinstance(expression, PatternPredicate): + return _pattern_parts(expression.pattern) + if isinstance(expression, Exists): + return [ + *(it for pattern in expression.patterns for it in _pattern_parts(pattern)), + *_present(expression.where), + ] + if isinstance(expression, Subquery): + return [] + return self._children(expression) + + +def _named(aggregate: Expression) -> str: + """An aggregate's name, as a message gives it.""" + if isinstance(aggregate, CountStar): + return "count(*)" + assert isinstance(aggregate, Function) + return f"{aggregate.name}()" + + +def _pattern_parts(pattern: Pattern) -> list[Expression]: + parts = [node.properties for node in pattern.nodes if node.properties is not None] + for relationship in pattern.relationships: + parts += _present(relationship.properties) + if relationship.group is not None: + parts += [*_pattern_parts(relationship.group.pattern), *_present(relationship.group.where)] + return parts + + +def _present(*items: Expression | None) -> list[Expression]: + return [it for it in items if it is not None] diff --git a/drivine/cypher/sql/sql_cypher_compiler.py b/drivine/cypher/sql/sql_cypher_compiler.py index 2bba7a3..ae33eed 100644 --- a/drivine/cypher/sql/sql_cypher_compiler.py +++ b/drivine/cypher/sql/sql_cypher_compiler.py @@ -7,6 +7,7 @@ import re from abc import ABC, abstractmethod from collections.abc import Callable, Iterable, Mapping, Sequence +from collections.abc import Set as AbstractSet from dataclasses import dataclass from decimal import Decimal from enum import Enum @@ -64,12 +65,13 @@ ) from drivine.cypher.ast.node_pattern import NodePattern from drivine.cypher.ast.pattern import Pattern, Shortest -from drivine.cypher.ast.projection import Projection, ProjectionItem +from drivine.cypher.ast.projection import Projection, ProjectionItem, SortItem from drivine.cypher.ast.query import Query from drivine.cypher.ast.relationship_pattern import Direction, RelationshipPattern from drivine.cypher.ast.remove_item import RemoveItem, RemoveProperty from drivine.cypher.ast.set_item import SetAdd, SetItem, SetProperty, SetReplace from drivine.cypher.cypher_reader import read_cypher +from drivine.cypher.sql.aggregates import check_aggregates from drivine.cypher.sql.compiled_cypher import CompiledCypher from drivine.cypher.sql.cypher_syntax import bracketed, imports, plain_with from drivine.cypher.sql.parameters import Parameters, cypher_json, jsonable @@ -500,6 +502,8 @@ def compile_query(self, query: Query, parameters: Mapping[str, Any]) -> Compiled # A variable used as two kinds, or made where it is bound already, is refused for that, before what is not # compiled yet. check_variable_kinds(query) + # An aggregate in a WHERE too. + check_aggregates(query, is_aggregate, children) if query.unions: return self._with_functions(self._union(query, parameters)) if not any(writes(it) for it in query.clauses): @@ -2955,7 +2959,7 @@ def _project(self, select: _Select, projection: Projection, final: bool) -> _Pro if not items: raise DrivineError("A projection has at least one item.") # Before _aggregates_first, which names each variable it groups by as an item of its own. - _require_explicit_grouping(items) + _require_explicit_grouping(items, projection.order_by, select.scope.keys()) made = self._anew_first(select, projection, items) if made is not None: return self._project(made[0], made[1], final) @@ -6682,20 +6686,30 @@ def _order_of(select: _Select) -> str | None: return ", ".join(f"{term} DESC" if descending else term for term, descending in select.order) or None -def _require_explicit_grouping(items: Sequence[ProjectionItem]) -> None: +def _require_explicit_grouping( + items: Sequence[ProjectionItem], order_by: Sequence[SortItem], scope: AbstractSet[str] +) -> None: """ - Refuses what Cypher refuses: an item that aggregates and names a variable outside its aggregates, where that - variable is not an item of its own, which a graph engine would have to group by without being told. + Refuses what Cypher refuses (Neo4j's 42I18): an item that aggregates and names a variable outside its aggregates + that is not an item of its own, which a graph engine would have to group by without being told; in a + comprehension's, a quantifier's, a reduce's or a pattern's body, a variable of `scope` outside an item that does + not aggregate. So too a term of `order_by` that aggregates, beside items that aggregate, and names a variable of + `scope` outside an item's name and an item that does not aggregate, as Neo4j takes them. As drivine4j's + requireExplicitGrouping. """ - keys = { - it.expression.name - for it in items - if not contains(it.expression, is_aggregate) and isinstance(it.expression, Variable) - } + grouping = [it for it in items if not contains(it.expression, is_aggregate)] + keys = {it.expression.name for it in grouping if isinstance(it.expression, Variable)} + grouped = [it.expression for it in grouping] for item in items: if not contains(item.expression, is_aggregate): continue - implicit = [it for it in variables_outside_aggregates(item.expression) if it not in keys] + implicit = [ + it + for it in _ordered( + variables_outside_aggregates(item.expression), _named_in_bodies(item.expression, scope, grouped) + ) + if it not in keys + ] if implicit: named = ", ".join(implicit) raise DrivineError( @@ -6703,6 +6717,104 @@ def _require_explicit_grouping(items: Sequence[ProjectionItem]) -> None: f"would group by it without being told. Aggregate in a WITH first, as in WITH {named}, collect(…) AS " "items RETURN { … }." ) + if len(grouping) == len(items): + return + names = {it.name for it in items} + for sort in order_by: + if not contains(sort.expression, is_aggregate): + continue + # Not less the variables an item is: one an item renames, named in an order term's pattern, is a new variable + # of the pattern's to Neo4j, which matches any node, so it is refused rather than read as the key. + implicit = _named_outside(sort.expression, names, scope, grouped) + if implicit: + raise DrivineError( + f"A term of an ORDER BY aggregates, and names {', '.join(implicit)} outside its aggregates and the " + "items: Cypher would group by it without being told. Order by an item's name, or by an expression of " + "the items and aggregates." + ) + + +def _named_in_bodies(expression: Expression, scope: AbstractSet[str], keys: Sequence[Expression]) -> list[str]: + """ + The variables of `scope` that the expression names outside its aggregates and each part of it `keys` holds, in a + comprehension's, a quantifier's, a reduce's or a pattern's body, which `variables_outside_aggregates` does not + read. + """ + if is_aggregate(expression) or expression in keys: + return [] + if isinstance(expression, ListComprehension): + return _ordered( + _named_in_bodies(expression.list, scope, keys), + *( + _named_outside(it, {expression.variable}, scope, keys) + for it in _present(expression.where, expression.projection) + ), + ) + if isinstance(expression, Quantifier): + return _ordered( + _named_in_bodies(expression.list, scope, keys), + *(_named_outside(it, {expression.variable}, scope, keys) for it in _present(expression.where)), + ) + if isinstance(expression, Reduce): + return _ordered( + _named_in_bodies(expression.initial, scope, keys), + _named_in_bodies(expression.list, scope, keys), + _named_outside(expression.expression, {expression.accumulator, expression.variable}, scope, keys), + ) + if isinstance(expression, (PatternComprehension, PatternPredicate, Exists)): + return _named_outside(expression, set(), scope, keys) + return _ordered(*(_named_in_bodies(it, scope, keys) for it in children(expression))) + + +def _named_outside( + expression: Expression, local: AbstractSet[str], scope: AbstractSet[str], keys: Sequence[Expression] +) -> list[str]: + """ + The variables of `scope` that the expression names outside its aggregates, and outside each part of it `keys` + holds, a comprehension's, a quantifier's, a reduce's or a pattern's body among them, but for those it binds or + `local` holds. A pattern binds each of its variables not of `scope`. A subquery's own query is read for none. + """ + + def outside(part: Expression, inner: AbstractSet[str] = local) -> list[str]: + return _named_outside(part, inner, scope, keys) + + def patterned(pattern: Pattern, parts: Sequence[Expression]) -> list[str]: + named = _ordered(variables_in_pattern(pattern), _present(pattern.variable)) + bound = {*local, *(it for it in named if it not in scope)} + return _ordered([it for it in named if it in scope and it not in local], *(outside(it, bound) for it in parts)) + + if is_aggregate(expression) or expression in keys: + return [] + if isinstance(expression, Variable): + return [] if expression.name in local or expression.name not in scope else [expression.name] + if isinstance(expression, MapProjection): + # A map projection of a key reads the key, as Neo4j reads it. + variable = expression.variable + read = variable not in local and variable in scope and Variable(variable) not in keys + return _ordered([variable] if read else [], *(outside(it) for it in children(expression))) + if isinstance(expression, ListComprehension): + return _ordered( + outside(expression.list), + *(outside(it, {*local, expression.variable}) for it in _present(expression.where, expression.projection)), + ) + if isinstance(expression, Quantifier): + return _ordered( + outside(expression.list), + *(outside(it, {*local, expression.variable}) for it in _present(expression.where)), + ) + if isinstance(expression, Reduce): + return _ordered( + outside(expression.initial), + outside(expression.list), + outside(expression.expression, {*local, expression.accumulator, expression.variable}), + ) + if isinstance(expression, PatternComprehension): + return patterned(expression.pattern, _present(expression.where, expression.projection)) + if isinstance(expression, PatternPredicate): + return patterned(expression.pattern, []) + if isinstance(expression, Exists): + return _ordered(*(patterned(it, _present(expression.where)) for it in expression.patterns)) + return _ordered(*(outside(it) for it in children(expression))) def _kind_of(values: Sequence[tuple[Expression, Value]]) -> Temporal | None: diff --git a/tests/cypher/VERSION b/tests/cypher/VERSION index 87a1cf5..6345c21 100644 --- a/tests/cypher/VERSION +++ b/tests/cypher/VERSION @@ -1 +1 @@ -v0.12.0 +v0.13.0 diff --git a/tests/cypher/queries.yaml b/tests/cypher/queries.yaml index 924b577..c3d72f9 100644 --- a/tests/cypher/queries.yaml +++ b/tests/cypher/queries.yaml @@ -1495,7 +1495,7 @@ cases: rows: [[9, 1], [8, 2]] - name: "Implicit grouping key" - description: "A RETURN item that mixes an aggregate with another variable is refused, as Neo4j refuses it." + description: "A RETURN item that mixes an aggregate with a variable no other item is is refused, as Neo4j refuses it; one that names only what the projection groups by, in a comprehension's body or an order term too, gives Neo4j's rows." steps: - note: Neo4j says "implicit grouping"; the compiler names the variable outside its aggregates. only: [graph] @@ -1507,6 +1507,60 @@ cases: - note: With the variable an item of its own, it is grouped by. cypher: "MATCH (p:Person {name: 'Laurence'})-[:ACTED_IN]->(m:Movie) RETURN p, collect(m.title)" rows: [[{name: "Laurence", born: 1961}, ["The Matrix"]]] + - note: "A comprehension's body that names a grouping key, or a variable of its own. Refused where it names another (see refusals.yaml, \"Implicit grouping key in a body or an ORDER BY\")." + cypher: "UNWIND [1, 2] AS x RETURN x, [y IN collect(x) | y + x] AS l ORDER BY x" + rows: [[1, [2]], [2, [4]]] + - cypher: "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, [y IN collect(z) | y + x] AS l ORDER BY x" + rows: [[1, [4]], [2, [5]]] + - cypher: "UNWIND [1, 2] AS x RETURN x, reduce(s = 0, y IN collect(x) | s + y + x) AS l ORDER BY x" + rows: [[1, 2], [2, 4]] + - note: "A property an item is is a grouping key too." + cypher: "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p.name, size([x IN collect(m.title) WHERE x <> p.name | x + p.name]) ORDER BY p.name" + rows: [["Carrie", 2], ["Keanu", 3], ["Laurence", 1]] + - cypher: "UNWIND [1, 2] AS x RETURN x, collect(x) AS c ORDER BY x" + rows: [[1, [1]], [2, [2]]] + - cypher: "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p.name, count(*) ORDER BY p.name" + rows: [["Carrie", 2], ["Keanu", 3], ["Laurence", 1]] + - note: "An order term after items that aggregate, that names an item's name, or what an item that does not aggregate is." + cypher: "UNWIND [{a: 1}, {a: 2}] AS x RETURN x.a AS a, count(*) AS c ORDER BY x.a + count(*) DESC" + rows: [[2, 1], [1, 1]] + - cypher: "UNWIND [1, 2] AS x RETURN x AS y, count(*) AS c ORDER BY x + count(*) DESC" + rows: [[2, 1], [1, 1]] + - cypher: "UNWIND [1, 2] AS x RETURN x AS y, count(*) AS c ORDER BY y + count(*) DESC" + rows: [[2, 1], [1, 1]] + - cypher: "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY [w IN collect(z) | w + x][0] DESC" + rows: [[2, 1], [1, 1]] + - note: "A map projection of a variable an item renames reads the key; so does a pattern, where no item renames the variable." + only: [neo4j, sql] + cypher: "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p AS q, count(*) AS c ORDER BY size(keys(p {.name})) + count(*) DESC" + rows: [[{name: "Keanu", born: 1964}, 3], [{name: "Carrie", born: 1967}, 2], [{name: "Laurence", born: 1961}, 1]] + - cypher: "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p AS q, count(*) AS c ORDER BY p {.born}.born + count(*) DESC" + only: [neo4j, sql] + rows: [[{name: "Carrie", born: 1967}, 2], [{name: "Keanu", born: 1964}, 3], [{name: "Laurence", born: 1961}, 1]] + - cypher: "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p, count(*) AS c ORDER BY size([(p)-[:KNOWS]->(k) | k]) * 10 - count(*) DESC" + only: [neo4j, sql] + rows: [[{name: "Laurence", born: 1961}, 1], [{name: "Keanu", born: 1964}, 3], [{name: "Carrie", born: 1967}, 2]] + - note: "Where an item renames it, Neo4j reads the variable in an order term's pattern as a new one that matches any node, not as the key; the compiler refuses it." + only: [neo4j] + cypher: "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p AS q, count(*) AS c ORDER BY size([(p)-[:KNOWS]->(k) | k]) * 10 - count(*) DESC" + rows: [[{name: "Laurence", born: 1961}, 1], [{name: "Carrie", born: 1967}, 2], [{name: "Keanu", born: 1964}, 3]] + - only: [sql] + cypher: "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p AS q, count(*) AS c ORDER BY size([(p)-[:KNOWS]->(k) | k]) * 10 - count(*) DESC" + error: "A term of an ORDER BY aggregates, and names p outside its aggregates and the items: Cypher would group by it without being told. Order by an item's name, or by an expression of the items and aggregates." + + - name: "Aggregate beside a WHERE" + description: "A WHERE over what an aggregate gave, and one in a subquery's or a CALL's own query, which aggregates in its projection, filters as Neo4j does. An aggregate in a WHERE is refused (see refusals.yaml, \"Aggregate in a WHERE\")." + steps: + - cypher: "UNWIND [1, 2] AS i WITH count(*) AS c WHERE c > 1 RETURN c" + rows: [2] + - cypher: "MATCH (p:Person)-[:ACTED_IN]->(m) WITH p, collect(m.title) AS ts WHERE size(ts) > 1 RETURN p.name ORDER BY p.name" + rows: ["Carrie", "Keanu"] + - cypher: "MATCH (p:Person) RETURN p.name, COUNT { MATCH (p)-[:ACTED_IN]->(m) WITH count(m) AS c WHERE c > 1 RETURN c } AS k ORDER BY p.name" + rows: [["Carrie", 1], ["Hugo", 0], ["Keanu", 1], ["Laurence", 0]] + - cypher: "MATCH (p:Person) WHERE EXISTS { MATCH (p)-[:ACTED_IN]->(m) WITH count(m) AS c WHERE c > 1 RETURN c } RETURN p.name ORDER BY p.name" + rows: ["Carrie", "Keanu"] + - cypher: "MATCH (p:Person) CALL (p) { MATCH (p)-[:ACTED_IN]->(m) WITH count(m) AS c WHERE c > 1 RETURN c } RETURN p.name, c ORDER BY p.name" + rows: [["Carrie", 2], ["Keanu", 3]] - name: "Variable-length ranges" description: "A variable-length relationship matches each length in its range." diff --git a/tests/cypher/refusals.yaml b/tests/cypher/refusals.yaml index 585e7ba..fb6fea7 100644 --- a/tests/cypher/refusals.yaml +++ b/tests/cypher/refusals.yaml @@ -606,3 +606,89 @@ cases: steps: - cypher: "MATCH p = (a)-[*]->(b) RETURN size(p)" error: "size() takes a list or a text, not a path, as in Neo4j: length() takes a path." + + - name: "Aggregate in a WHERE" + description: "An aggregate in a WHERE is refused, as Neo4j refuses it (its 42I24, \"invalid use of aggregate function\"): of a MATCH or a WITH, a pattern comprehension, a list comprehension, a quantifier, a quantified path pattern, EXISTS { … } and COUNT { … }, in a subquery's or a CALL { … }'s query too. A subquery or a CALL that aggregates in its own projection is filtered as any WITH is (see queries.yaml, \"Aggregate beside a WHERE\")." + steps: + - cypher: "UNWIND [1, 2] AS i WITH i WHERE sum(i) > 1 RETURN i" + error: "sum() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - note: "Within a condition of several, too." + cypher: "UNWIND [1, 2] AS i WITH i WHERE i > 1 AND count(*) > 0 RETURN i" + error: "count(*) aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - cypher: "MATCH (n) WHERE count(*) > 1 RETURN n" + error: "count(*) aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - cypher: "MATCH (n) WHERE sum(n.x) > 1 RETURN n" + error: "sum() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - cypher: "MATCH (a) WHERE a.x IN collect(a.y) RETURN a" + error: "collect() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - note: "A pattern comprehension's WHERE." + cypher: "MATCH (n) RETURN [(n)-->(m) WHERE count(m) > 1 | m] AS ms" + error: "count() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - note: "A quantifier's WHERE." + cypher: "UNWIND [[1, 2]] AS l RETURN all(x IN l WHERE sum(x) > 1) AS a" + error: "sum() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - note: "A list comprehension's WHERE." + cypher: "UNWIND [[1, 2]] AS l RETURN [x IN l WHERE count(x) > 1 | x] AS a" + error: "count() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - note: "A quantified path pattern's WHERE." + cypher: "MATCH (a) ((x)-[:R]->(y) WHERE count(x) > 1)+ (b) RETURN a" + error: "count() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - note: "The WHERE of EXISTS { … } and COUNT { … } of a pattern." + cypher: "MATCH (n) WHERE EXISTS { (n)-->(m) WHERE count(m) > 1 } RETURN n" + error: "count() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - cypher: "MATCH (n) WHERE COUNT { (n)-->(m) WHERE count(m) > 1 } > 0 RETURN n" + error: "count() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - note: "A WHERE in a subquery's own query." + cypher: "MATCH (n) WHERE EXISTS { MATCH (n)-->(m) WHERE count(m) > 1 RETURN m } RETURN n" + error: "count() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - cypher: "MATCH (n) RETURN COUNT { MATCH (n)-->(m) WITH m WHERE count(m) > 1 RETURN m } AS c" + error: "count() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - note: "A WHERE in a CALL { … }." + cypher: "UNWIND [1, 2] AS i CALL (i) { WITH i WHERE sum(i) > 1 RETURN i AS j } RETURN j" + error: "sum() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - cypher: "UNWIND [1, 2] AS i CALL (i) { MATCH (n) WHERE count(n) > i RETURN n } RETURN n" + error: "count() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + - cypher: "UNWIND [1, 2] AS i CALL { WITH i MATCH (n) WHERE max(n.x) > i RETURN n } RETURN n" + error: "max() aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + + - name: "Implicit grouping key in a body or an ORDER BY" + description: "An item that aggregates, and names in a comprehension's, a quantifier's, a reduce()'s or a pattern's body a variable outside each item that does not aggregate, is refused, as Neo4j refuses it (its 42I18, \"reference to non-grouping sub-expression\"); so is a term of an ORDER BY that aggregates, after items that aggregate, and names a variable outside the items. What Neo4j takes beside them is in queries.yaml, \"Implicit grouping key\"." + steps: + - note: "A list comprehension's body: z is no grouping key, though x is." + cypher: "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, [y IN collect(z) | y + z] AS l" + error: "l aggregates, and names z outside its aggregates, which no other item is: Cypher would group by it without being told. Aggregate in a WITH first, as in WITH z, collect(…) AS items RETURN { … }." + - note: "In a WITH too." + cypher: "UNWIND [1, 2] AS x UNWIND [3] AS z WITH x, [y IN collect(z) | y + z] AS l RETURN x, l" + error: "l aggregates, and names z outside its aggregates, which no other item is: Cypher would group by it without being told. Aggregate in a WITH first, as in WITH z, collect(…) AS items RETURN { … }." + - note: "A list comprehension's WHERE." + cypher: "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, [y IN collect(x) WHERE y > z | y] AS l" + error: "l aggregates, and names z outside its aggregates, which no other item is: Cypher would group by it without being told. Aggregate in a WITH first, as in WITH z, collect(…) AS items RETURN { … }." + - note: "A quantifier's WHERE." + cypher: "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, any(y IN collect(x) WHERE y > z) AS l" + error: "l aggregates, and names z outside its aggregates, which no other item is: Cypher would group by it without being told. Aggregate in a WITH first, as in WITH z, collect(…) AS items RETURN { … }." + - note: "A reduce()'s step." + cypher: "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, reduce(s = 0, y IN collect(x) | s + z) AS l" + error: "l aggregates, and names z outside its aggregates, which no other item is: Cypher would group by it without being told. Aggregate in a WITH first, as in WITH z, collect(…) AS items RETURN { … }." + - note: "x.b is no grouping key, though x.a is." + cypher: "UNWIND [{a: 1, b: 2}] AS x RETURN x.a, [y IN collect(x) | y.a + x.b] AS l" + error: "l aggregates, and names x outside its aggregates, which no other item is: Cypher would group by it without being told. Aggregate in a WITH first, as in WITH x, collect(…) AS items RETURN { … }." + - note: "A pattern comprehension's pattern names n, which no item is." + cypher: "MATCH (n) RETURN size([(n)-->(m) | m]) + count(*) AS c" + error: "c aggregates, and names n outside its aggregates, which no other item is: Cypher would group by it without being told. Aggregate in a WITH first, as in WITH n, collect(…) AS items RETURN { … }." + - cypher: "MATCH (n) RETURN n.x, size([(n)-->(m) | m.x]) + count(*) AS c" + error: "c aggregates, and names n outside its aggregates, which no other item is: Cypher would group by it without being told. Aggregate in a WITH first, as in WITH n, collect(…) AS items RETURN { … }." + - note: "So does EXISTS { … } of a pattern." + cypher: "MATCH (n) RETURN EXISTS { (n)-->() } AND count(*) > 0 AS b" + error: "b aggregates, and names n outside its aggregates, which no other item is: Cypher would group by it without being told. Aggregate in a WITH first, as in WITH n, collect(…) AS items RETURN { … }." + - note: "An ORDER BY term after items that aggregate: x.b is no grouping key, though x.a is." + cypher: "UNWIND [{a: 1}, {a: 2}] AS x RETURN x.a AS a, count(*) AS c ORDER BY x.b + count(*)" + error: "A term of an ORDER BY aggregates, and names x outside its aggregates and the items: Cypher would group by it without being told. Order by an item's name, or by an expression of the items and aggregates." + - cypher: "UNWIND [{a: 1, b: 2}] AS x WITH x.a AS a, count(*) AS c ORDER BY x.b + count(*) RETURN a" + error: "A term of an ORDER BY aggregates, and names x outside its aggregates and the items: Cypher would group by it without being told. Order by an item's name, or by an expression of the items and aggregates." + - cypher: "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY z + count(*)" + error: "A term of an ORDER BY aggregates, and names z outside its aggregates and the items: Cypher would group by it without being told. Order by an item's name, or by an expression of the items and aggregates." + - cypher: "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY [w IN collect(z) | w + z]" + error: "A term of an ORDER BY aggregates, and names z outside its aggregates and the items: Cypher would group by it without being told. Order by an item's name, or by an expression of the items and aggregates." + - note: "The openCypher TCK's ReturnOrderBy6 [4]." + cypher: "MATCH (me:Person)--(you:Person) RETURN count(you.age) AS agg ORDER BY me.age + count(you.age)" + error: "A term of an ORDER BY aggregates, and names me outside its aggregates and the items: Cypher would group by it without being told. Order by an item's name, or by an expression of the items and aggregates." diff --git a/tests/integration/test_cypher_compiled.py b/tests/integration/test_cypher_compiled.py index 361bfe0..6113346 100644 --- a/tests/integration/test_cypher_compiled.py +++ b/tests/integration/test_cypher_compiled.py @@ -991,6 +991,67 @@ async def rows_of(cypher: str, parameters: dict[str, Any] | None = None) -> list ) == [[2, [1, 1]]] +async def test_an_aggregate_beside_a_where_a_grouping_keys_body_or_an_order_term_that_neo4j_takes_gives_neo4js_rows( + cypher_pm: PersistenceManager, +): + # #306, #307: beside the aggregates now refused, those Neo4j takes. drivine4j's CypherQueryContract of + # liberation-data/drivine4j#489 and #482; the rows are Neo4j's (2026.10). + await seeded(cypher_pm) + + async def query(cypher: str) -> list[Any]: + return await cypher_pm.query(spec(cypher)) + + # A WHERE over what an aggregate gave, and one in a subquery's or a CALL's own query, which aggregates in its + # projection. + assert await query("UNWIND [1, 2] AS i WITH count(*) AS c WHERE c > 1 RETURN c") == [2] + assert await query( + "MATCH (p:Person)-[:ACTED_IN]->(m) WITH p, collect(m.title) AS ts WHERE size(ts) > 1 RETURN p.name " + "ORDER BY p.name" + ) == ["Carrie", "Keanu"] + assert await query( + "MATCH (p:Person) RETURN p.name, COUNT { MATCH (p)-[:ACTED_IN]->(m) WITH count(m) AS c WHERE c > 1 RETURN c } " + "AS k ORDER BY p.name" + ) == [["Carrie", 1], ["Hugo", 0], ["Keanu", 1], ["Laurence", 0]] + assert await query( + "MATCH (p:Person) WHERE EXISTS { MATCH (p)-[:ACTED_IN]->(m) WITH count(m) AS c WHERE c > 1 RETURN c } " + "RETURN p.name ORDER BY p.name" + ) == ["Carrie", "Keanu"] + assert await query( + "MATCH (p:Person) CALL (p) { MATCH (p)-[:ACTED_IN]->(m) WITH count(m) AS c WHERE c > 1 RETURN c } " + "RETURN p.name, c ORDER BY p.name" + ) == [["Carrie", 2], ["Keanu", 3]] + # An aggregate beside its grouping keys, and a body that names only them, or a variable of its own. + assert await query("UNWIND [1, 2] AS x RETURN x, collect(x) AS c ORDER BY x") == [[1, [1]], [2, [2]]] + assert await query("MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p.name, count(*) ORDER BY p.name") == [ + ["Carrie", 2], + ["Keanu", 3], + ["Laurence", 1], + ] + assert await query("UNWIND [1, 2] AS x RETURN x, [y IN collect(x) | y + x] AS l ORDER BY x") == [[1, [2]], [2, [4]]] + assert await query("UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, [y IN collect(z) | y + x] AS l ORDER BY x") == [ + [1, [4]], + [2, [5]], + ] + assert await query("UNWIND [1, 2] AS x RETURN x, reduce(s = 0, y IN collect(x) | s + y + x) AS l ORDER BY x") == [ + [1, 2], + [2, 4], + ] + assert await query( + "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p.name, size([x IN collect(m.title) WHERE x <> p.name | x + p.name]) " + "ORDER BY p.name" + ) == [["Carrie", 2], ["Keanu", 3], ["Laurence", 1]] + # An order term that names an item's name, or what an item that does not aggregate is. + assert await query("UNWIND [{a: 1}, {a: 2}] AS x RETURN x.a AS a, count(*) AS c ORDER BY x.a + count(*) DESC") == [ + [2, 1], + [1, 1], + ] + assert await query("UNWIND [1, 2] AS x RETURN x AS y, count(*) AS c ORDER BY x + count(*) DESC") == [[2, 1], [1, 1]] + assert await query("UNWIND [1, 2] AS x RETURN x AS y, count(*) AS c ORDER BY y + count(*) DESC") == [[2, 1], [1, 1]] + assert await query( + "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY [w IN collect(z) | w + x][0] DESC" + ) == [[2, 1], [1, 1]] + + async def test_an_optional_call_in_a_writes_subquery_counts_what_it_finds_and_the_row_it_finds_nothing_for( cypher_pm: PersistenceManager, ): diff --git a/tests/sync_integration/test_cypher_compiled.py b/tests/sync_integration/test_cypher_compiled.py index 92e4b12..ea7626a 100644 --- a/tests/sync_integration/test_cypher_compiled.py +++ b/tests/sync_integration/test_cypher_compiled.py @@ -984,6 +984,67 @@ def rows_of(cypher: str, parameters: dict[str, Any] | None = None) -> list[Any]: ) == [[2, [1, 1]]] +def test_an_aggregate_beside_a_where_a_grouping_keys_body_or_an_order_term_that_neo4j_takes_gives_neo4js_rows( + cypher_pm: PersistenceManager, +): + # #306, #307: beside the aggregates now refused, those Neo4j takes. drivine4j's CypherQueryContract of + # liberation-data/drivine4j#489 and #482; the rows are Neo4j's (2026.10). + seeded(cypher_pm) + + def query(cypher: str) -> list[Any]: + return cypher_pm.query(spec(cypher)) + + # A WHERE over what an aggregate gave, and one in a subquery's or a CALL's own query, which aggregates in its + # projection. + assert query("UNWIND [1, 2] AS i WITH count(*) AS c WHERE c > 1 RETURN c") == [2] + assert query( + "MATCH (p:Person)-[:ACTED_IN]->(m) WITH p, collect(m.title) AS ts WHERE size(ts) > 1 RETURN p.name " + "ORDER BY p.name" + ) == ["Carrie", "Keanu"] + assert query( + "MATCH (p:Person) RETURN p.name, COUNT { MATCH (p)-[:ACTED_IN]->(m) WITH count(m) AS c WHERE c > 1 RETURN c } " + "AS k ORDER BY p.name" + ) == [["Carrie", 1], ["Hugo", 0], ["Keanu", 1], ["Laurence", 0]] + assert query( + "MATCH (p:Person) WHERE EXISTS { MATCH (p)-[:ACTED_IN]->(m) WITH count(m) AS c WHERE c > 1 RETURN c } " + "RETURN p.name ORDER BY p.name" + ) == ["Carrie", "Keanu"] + assert query( + "MATCH (p:Person) CALL (p) { MATCH (p)-[:ACTED_IN]->(m) WITH count(m) AS c WHERE c > 1 RETURN c } " + "RETURN p.name, c ORDER BY p.name" + ) == [["Carrie", 2], ["Keanu", 3]] + # An aggregate beside its grouping keys, and a body that names only them, or a variable of its own. + assert query("UNWIND [1, 2] AS x RETURN x, collect(x) AS c ORDER BY x") == [[1, [1]], [2, [2]]] + assert query("MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p.name, count(*) ORDER BY p.name") == [ + ["Carrie", 2], + ["Keanu", 3], + ["Laurence", 1], + ] + assert query("UNWIND [1, 2] AS x RETURN x, [y IN collect(x) | y + x] AS l ORDER BY x") == [[1, [2]], [2, [4]]] + assert query("UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, [y IN collect(z) | y + x] AS l ORDER BY x") == [ + [1, [4]], + [2, [5]], + ] + assert query("UNWIND [1, 2] AS x RETURN x, reduce(s = 0, y IN collect(x) | s + y + x) AS l ORDER BY x") == [ + [1, 2], + [2, 4], + ] + assert query( + "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p.name, size([x IN collect(m.title) WHERE x <> p.name | x + p.name]) " + "ORDER BY p.name" + ) == [["Carrie", 2], ["Keanu", 3], ["Laurence", 1]] + # An order term that names an item's name, or what an item that does not aggregate is. + assert query("UNWIND [{a: 1}, {a: 2}] AS x RETURN x.a AS a, count(*) AS c ORDER BY x.a + count(*) DESC") == [ + [2, 1], + [1, 1], + ] + assert query("UNWIND [1, 2] AS x RETURN x AS y, count(*) AS c ORDER BY x + count(*) DESC") == [[2, 1], [1, 1]] + assert query("UNWIND [1, 2] AS x RETURN x AS y, count(*) AS c ORDER BY y + count(*) DESC") == [[2, 1], [1, 1]] + assert query( + "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY [w IN collect(z) | w + x][0] DESC" + ) == [[2, 1], [1, 1]] + + def test_an_optional_call_in_a_writes_subquery_counts_what_it_finds_and_the_row_it_finds_nothing_for( cypher_pm: PersistenceManager, ): diff --git a/tests/unit/test_cypher_refusals.py b/tests/unit/test_cypher_refusals.py index 31a3243..6dc76c7 100644 --- a/tests/unit/test_cypher_refusals.py +++ b/tests/unit/test_cypher_refusals.py @@ -607,3 +607,186 @@ def test_a_truncations_fields_given_as_an_expression_a_negated_number_utc_or_non "RETURN date.truncate('month', date(), {day: -2.0})", ): compiler("g").compile(cypher) + + +def _in_where(aggregate: str) -> str: + return ( + f"{aggregate} aggregates, and a WHERE holds no aggregate, as in Neo4j: aggregate in a WITH first, and filter " + "what it gives in that WITH's WHERE, as in WITH x, count(*) AS c WHERE c > 1." + ) + + +@pytest.mark.parametrize("compiler", COMPILERS.values(), ids=COMPILERS.keys()) +@pytest.mark.parametrize( + ("cypher", "message"), + [ + ("UNWIND [1, 2] AS i WITH i WHERE sum(i) > 1 RETURN i", _in_where("sum()")), + ("UNWIND [1, 2] AS i WITH i WHERE i > 1 AND count(*) > 0 RETURN i", _in_where("count(*)")), + ("MATCH (n) WHERE count(*) > 1 RETURN n", _in_where("count(*)")), + ("MATCH (a) WHERE a.x IN collect(a.y) RETURN a", _in_where("collect()")), + ("MATCH (n) RETURN [(n)-->(m) WHERE count(m) > 1 | m] AS ms", _in_where("count()")), + ("UNWIND [[1, 2]] AS l RETURN all(x IN l WHERE sum(x) > 1) AS a", _in_where("sum()")), + ("UNWIND [[1, 2]] AS l RETURN [x IN l WHERE count(x) > 1 | x] AS a", _in_where("count()")), + ("MATCH (a) ((x)-[:R]->(y) WHERE count(x) > 1)+ (b) RETURN a", _in_where("count()")), + ("MATCH (n) WHERE EXISTS { (n)-->(m) WHERE count(m) > 1 } RETURN n", _in_where("count()")), + ("MATCH (n) WHERE COUNT { (n)-->(m) WHERE count(m) > 1 } > 0 RETURN n", _in_where("count()")), + ("MATCH (n) RETURN COUNT { MATCH (n)-->(m) WHERE count(m) > 1 RETURN m } AS c", _in_where("count()")), + ("MATCH (n) RETURN COUNT { MATCH (n)-->(m) WITH m WHERE count(m) > 1 RETURN m } AS c", _in_where("count()")), + ("UNWIND [1, 2] AS i CALL (i) { WITH i WHERE sum(i) > 1 RETURN i AS j } RETURN j", _in_where("sum()")), + ("UNWIND [1, 2] AS i CALL { WITH i MATCH (n) WHERE max(n.x) > i RETURN n } RETURN n", _in_where("max()")), + ( + "UNWIND [1, 2] AS i CALL (i) { CALL (i) { UNWIND [1, 2] AS x WITH x WHERE sum(x) > i RETURN x } RETURN x } " + "RETURN x", + _in_where("sum()"), + ), + ( + "MATCH (p:Person) RETURN p.name AS n UNION MATCH (m:Movie) WHERE count(*) > 1 RETURN m.title AS n", + _in_where("count(*)"), + ), + ("UNWIND [x IN [1, 2] WHERE count(x) > 0 | x] AS y RETURN y", _in_where("count()")), + ("MATCH (n) SET n.v = [x IN [1, 2] WHERE sum(x) > 0 | x]", _in_where("sum()")), + ("MERGE (n:N) ON CREATE SET n.v = [x IN [1, 2] WHERE min(x) > 0 | x]", _in_where("min()")), + ("CREATE (n:N {v: [x IN [1, 2] WHERE avg(x) > 0 | x]})", _in_where("avg()")), + ("FOREACH (i IN [1] | CREATE (:N {v: [x IN [1, 2] WHERE max(x) > 0 | x]}))", _in_where("max()")), + ], +) +def test_an_aggregate_in_a_where_is_refused_as_neo4j_refuses_it( + compiler: type[SqlCypherCompiler], cypher: str, message: str +) -> None: + # drivine4j's #489: Neo4j's 42I24, "invalid use of aggregate function". Postgres failed with an error of its own, + # and Oracle gave rows. The spec's "Aggregate in a WHERE" holds most of these; the rest are drivine4j's. + with pytest.raises(DrivineError) as refused: + compiler("g").compile(cypher) + assert str(refused.value) == message + + +@pytest.mark.parametrize("compiler", COMPILERS.values(), ids=COMPILERS.keys()) +@pytest.mark.parametrize( + "cypher", + [ + "UNWIND [1, 2] AS i WITH count(*) AS c WHERE c > 1 RETURN c", + "UNWIND [1, 2] AS i WITH collect(i) AS l WHERE size(l) > 1 RETURN l", + "MATCH (n) RETURN COUNT { MATCH (n)-->(m) WITH count(m) AS c WHERE c > 0 RETURN c } AS k", + "MATCH (n) WHERE EXISTS { MATCH (n)-->(m) WITH count(m) AS c WHERE c > 0 RETURN c } RETURN n.x", + "MATCH (n) CALL (n) { MATCH (n)-->(m) WITH count(m) AS c WHERE c > 0 RETURN c } RETURN n.x, c", + "MATCH (n) WITH n, count { (n)-->() } AS c WHERE c > 0 RETURN n.x", + ], +) +def test_a_where_over_what_an_aggregate_gave_or_in_a_query_that_aggregates_in_its_own_projection_is_compiled( + compiler: type[SqlCypherCompiler], cypher: str +) -> None: + # Beside drivine4j's #489, as Neo4j runs them. + compiler("g").compile(cypher) + + +def _ungrouped_item(name: str, variable: str) -> str: + return ( + f"{name} aggregates, and names {variable} outside its aggregates, which no other item is: Cypher would group " + f"by it without being told. Aggregate in a WITH first, as in WITH {variable}, collect(…) AS items " + "RETURN { … }." + ) + + +def _ungrouped_term(variable: str) -> str: + return ( + f"A term of an ORDER BY aggregates, and names {variable} outside its aggregates and the items: Cypher would " + "group by it without being told. Order by an item's name, or by an expression of the items and aggregates." + ) + + +@pytest.mark.parametrize("compiler", COMPILERS.values(), ids=COMPILERS.keys()) +@pytest.mark.parametrize( + ("cypher", "message"), + [ + ("UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, [y IN collect(z) | y + z] AS l", _ungrouped_item("l", "z")), + ( + "UNWIND [1, 2] AS x UNWIND [3] AS z WITH x, [y IN collect(z) | y + z] AS l RETURN x, l", + _ungrouped_item("l", "z"), + ), + ( + "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, [y IN collect(x) WHERE y > z | y] AS l", + _ungrouped_item("l", "z"), + ), + ( + "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, any(y IN collect(x) WHERE y > z) AS l", + _ungrouped_item("l", "z"), + ), + ( + "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, reduce(s = 0, y IN collect(x) | s + z) AS l", + _ungrouped_item("l", "z"), + ), + ("UNWIND [{a: 1, b: 2}] AS x RETURN x.a, [y IN collect(x) | y.a + x.b] AS l", _ungrouped_item("l", "x")), + ("MATCH (n) RETURN size([(n)-->(m) | m]) + count(*) AS c", _ungrouped_item("c", "n")), + ("MATCH (n) RETURN n.x, size([(n)-->(m) | m.x]) + count(*) AS c", _ungrouped_item("c", "n")), + ("MATCH (n) RETURN EXISTS { (n)-->() } AND count(*) > 0 AS b", _ungrouped_item("b", "n")), + ("UNWIND [{a: 1}, {a: 2}] AS x RETURN x.a AS a, count(*) AS c ORDER BY x.b + count(*)", _ungrouped_term("x")), + ( + "UNWIND [{a: 1, b: 2}] AS x WITH x.a AS a, count(*) AS c ORDER BY x.b + count(*) RETURN a", + _ungrouped_term("x"), + ), + ("UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY z + count(*)", _ungrouped_term("z")), + ( + "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY [w IN collect(z) | w + z]", + _ungrouped_term("z"), + ), + ( + "MATCH (me:Person)--(you:Person) RETURN count(you.age) AS agg ORDER BY me.age + count(you.age)", + _ungrouped_term("me"), + ), + ( + "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p AS q, count(*) AS c ORDER BY size(keys(m {.title})) + count(*)", + _ungrouped_term("m"), + ), + # Neo4j runs these, but reads p in the pattern as a new variable that matches any node, not as the key q is. + ( + "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p AS q, count(*) AS c " + "ORDER BY size([(p)-[:KNOWS]->(k) | k]) + count(*)", + _ungrouped_term("p"), + ), + ( + "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p AS q, count(*) AS c " + "ORDER BY EXISTS { (p)-[:KNOWS]->() } AND count(*) > 1", + _ungrouped_term("p"), + ), + ], +) +def test_an_aggregating_item_or_order_term_that_names_a_variable_no_item_groups_by_in_a_body_too_is_refused( + compiler: type[SqlCypherCompiler], cypher: str, message: str +) -> None: + # drivine4j's #482: Neo4j's 42I18, "reference to non-grouping sub-expression". The body of a comprehension, a + # quantifier, a reduce() or a pattern was not read for the variables it names, nor an order term at all. The spec's + # "Implicit grouping key in a body or an ORDER BY" holds most of these; the rest are drivine4j's. + with pytest.raises(DrivineError) as refused: + compiler("g").compile(cypher) + assert str(refused.value) == message + + +@pytest.mark.parametrize("compiler", COMPILERS.values(), ids=COMPILERS.keys()) +@pytest.mark.parametrize( + "cypher", + [ + "UNWIND [1, 2] AS x RETURN x, [y IN collect(x) | y + x] AS l", + "UNWIND [1, 2] AS x RETURN x, collect(x) AS c", + "UNWIND [1, 2] AS x RETURN [x IN collect(x) | x + 1] AS l", + "UNWIND [1, 2] AS x RETURN x, reduce(s = 0, y IN collect(x) | s + y + x) AS l", + "UNWIND [{a: 1}, {a: 2}] AS x RETURN x.a, count(*) AS c", + "UNWIND [{a: 1}] AS x RETURN x.a, [y IN collect(x) | y.a + x.a] AS l", + "MATCH (n) RETURN n, size([(n)-->(m) | m]) + count(*) AS c", + "MATCH (n) RETURN n, EXISTS { (n)-->() } AND count(*) > 0 AS b", + "UNWIND [{a: 1}, {a: 2}] AS x RETURN x.a AS a, count(*) AS c ORDER BY x.a + count(*)", + "UNWIND [1, 2] AS x RETURN x AS y, count(*) AS c ORDER BY x + count(*)", + "UNWIND [1, 2] AS x RETURN x AS y, count(*) AS c ORDER BY y + count(*)", + "UNWIND [{a: 1, b: 2}] AS x RETURN x.a + x.b, count(*) AS cnt ORDER BY x.a + x.b + count(*)", + "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY [w IN collect(z) | w + x]", + # A key an item renames, named by an order term's map projection, or by a pattern where no item renames it. + "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p AS q, count(*) AS c ORDER BY size(keys(p {.name})) + count(*)", + "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p AS q, count(*) AS c ORDER BY p {.born}.born + count(*)", + "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p, count(*) AS c ORDER BY size([(p)-[:KNOWS]->(k) | k]) + count(*)", + ], +) +def test_a_body_or_order_term_that_names_only_what_the_projection_groups_by_is_compiled( + compiler: type[SqlCypherCompiler], cypher: str +) -> None: + # Beside drivine4j's #482: a body or an order term that names only a grouping key, an item's name, or a variable of + # its own, as Neo4j runs it. + compiler("g").compile(cypher) From 1550393780e4c7ec39e9ee450d5d2be87c69754f Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sat, 31 Oct 2026 21:46:59 +1000 Subject: [PATCH 2/2] Match a grouping key's function whatever its case, and test more of the clauses the aggregate check walks --- drivine/cypher/sql/sql_cypher_compiler.py | 40 +++++++++++++++---- tests/integration/test_cypher_compiled.py | 8 ++++ .../sync_integration/test_cypher_compiled.py | 8 ++++ tests/unit/test_cypher_refusals.py | 14 +++++++ 4 files changed, 63 insertions(+), 7 deletions(-) diff --git a/drivine/cypher/sql/sql_cypher_compiler.py b/drivine/cypher/sql/sql_cypher_compiler.py index ae33eed..0dce471 100644 --- a/drivine/cypher/sql/sql_cypher_compiler.py +++ b/drivine/cypher/sql/sql_cypher_compiler.py @@ -3530,9 +3530,13 @@ def named(expression: Expression) -> list[str]: limit=None, ) + # A key is read where it is written in another case too: see `_grouping_key`. + key_names = {_grouping_key(it): name for it, name in keys.items()} + def kept(term: Expression) -> Expression | None: - if term in keys: - return Variable(keys[term]) + key = key_names.get(_grouping_key(term)) if key_names else None + if key is not None: + return Variable(key) if term in aggregates: name = aggregates[term] return ( @@ -3542,12 +3546,13 @@ def kept(term: Expression) -> Expression | None: # An order term, or a part of one, that is an item is read by the item's name, as over a DISTINCT projection it # can only be; a variable an item is named after is that item, not the variable of that name before it. - item_names = {it.expression: it.name for it in items} + item_names = {_grouping_key(it.expression): it.name for it in items} def sorted_by(term: Expression) -> Expression | None: if isinstance(term, Variable) and term.name in by_name: return term - return Variable(item_names[term]) if term in item_names else kept(term) + item = item_names.get(_grouping_key(term)) + return Variable(item) if item is not None else kept(term) keeping = dataclasses.replace( projection, @@ -6699,7 +6704,7 @@ def _require_explicit_grouping( """ grouping = [it for it in items if not contains(it.expression, is_aggregate)] keys = {it.expression.name for it in grouping if isinstance(it.expression, Variable)} - grouped = [it.expression for it in grouping] + grouped = [_grouping_key(it.expression) for it in grouping] for item in items: if not contains(item.expression, is_aggregate): continue @@ -6734,13 +6739,34 @@ def _require_explicit_grouping( ) +def _grouping_key(expression: Expression) -> Expression: + """ + The expression as it is compared with a key that groups rows: each function's name in lower case, as Neo4j names a + function in any case, so `ORDER BY TOUPPER(x.a) + count(*)` reads the key `toUpper(x.a)`, in + `_require_explicit_grouping` and `_aggregates_first`. The AST keeps the name as written. + """ + + def lowered(part: Expression) -> Expression | None: + if not isinstance(part, Function): + return None + arguments = tuple(_grouping_key(it) for it in part.arguments) + return dataclasses.replace(part, name=part.name.lower(), arguments=arguments) + + return replaced(expression, lowered) + + +def _is_key(expression: Expression, keys: Sequence[Expression]) -> bool: + """Whether the expression is one of `keys`, each a `_grouping_key`.""" + return bool(keys) and _grouping_key(expression) in keys + + def _named_in_bodies(expression: Expression, scope: AbstractSet[str], keys: Sequence[Expression]) -> list[str]: """ The variables of `scope` that the expression names outside its aggregates and each part of it `keys` holds, in a comprehension's, a quantifier's, a reduce's or a pattern's body, which `variables_outside_aggregates` does not read. """ - if is_aggregate(expression) or expression in keys: + if is_aggregate(expression) or _is_key(expression, keys): return [] if isinstance(expression, ListComprehension): return _ordered( @@ -6783,7 +6809,7 @@ def patterned(pattern: Pattern, parts: Sequence[Expression]) -> list[str]: bound = {*local, *(it for it in named if it not in scope)} return _ordered([it for it in named if it in scope and it not in local], *(outside(it, bound) for it in parts)) - if is_aggregate(expression) or expression in keys: + if is_aggregate(expression) or _is_key(expression, keys): return [] if isinstance(expression, Variable): return [] if expression.name in local or expression.name not in scope else [expression.name] diff --git a/tests/integration/test_cypher_compiled.py b/tests/integration/test_cypher_compiled.py index 6113346..95f24c5 100644 --- a/tests/integration/test_cypher_compiled.py +++ b/tests/integration/test_cypher_compiled.py @@ -1050,6 +1050,14 @@ async def query(cypher: str) -> list[Any]: assert await query( "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY [w IN collect(z) | w + x][0] DESC" ) == [[2, 1], [1, 1]] + # A key named in another case: Neo4j names a function in any case. + assert await query( + "UNWIND [{a: 'b'}, {a: 'a'}] AS x RETURN toUpper(x.a) AS u, count(*) AS c ORDER BY TOUPPER(x.a) + count(*) DESC" + ) == [["B", 1], ["A", 1]] + assert await query( + "UNWIND [{a: 'b'}, {a: 'a'}] AS x RETURN toUpper(x.a) AS u, collect(DISTINCT x.a) AS c " + "ORDER BY TOUPPER(x.a) + size(collect(DISTINCT x.a)) DESC" + ) == [["B", ["b"]], ["A", ["a"]]] async def test_an_optional_call_in_a_writes_subquery_counts_what_it_finds_and_the_row_it_finds_nothing_for( diff --git a/tests/sync_integration/test_cypher_compiled.py b/tests/sync_integration/test_cypher_compiled.py index ea7626a..20f077b 100644 --- a/tests/sync_integration/test_cypher_compiled.py +++ b/tests/sync_integration/test_cypher_compiled.py @@ -1043,6 +1043,14 @@ def query(cypher: str) -> list[Any]: assert query( "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY [w IN collect(z) | w + x][0] DESC" ) == [[2, 1], [1, 1]] + # A key named in another case: Neo4j names a function in any case. + assert query( + "UNWIND [{a: 'b'}, {a: 'a'}] AS x RETURN toUpper(x.a) AS u, count(*) AS c ORDER BY TOUPPER(x.a) + count(*) DESC" + ) == [["B", 1], ["A", 1]] + assert query( + "UNWIND [{a: 'b'}, {a: 'a'}] AS x RETURN toUpper(x.a) AS u, collect(DISTINCT x.a) AS c " + "ORDER BY TOUPPER(x.a) + size(collect(DISTINCT x.a)) DESC" + ) == [["B", ["b"]], ["A", ["a"]]] def test_an_optional_call_in_a_writes_subquery_counts_what_it_finds_and_the_row_it_finds_nothing_for( diff --git a/tests/unit/test_cypher_refusals.py b/tests/unit/test_cypher_refusals.py index 6dc76c7..a89ff74 100644 --- a/tests/unit/test_cypher_refusals.py +++ b/tests/unit/test_cypher_refusals.py @@ -646,6 +646,9 @@ def _in_where(aggregate: str) -> str: ("UNWIND [x IN [1, 2] WHERE count(x) > 0 | x] AS y RETURN y", _in_where("count()")), ("MATCH (n) SET n.v = [x IN [1, 2] WHERE sum(x) > 0 | x]", _in_where("sum()")), ("MERGE (n:N) ON CREATE SET n.v = [x IN [1, 2] WHERE min(x) > 0 | x]", _in_where("min()")), + ("MERGE (n:N) ON MATCH SET n.v = [x IN [1, 2] WHERE min(x) > 0 | x]", _in_where("min()")), + ("MATCH (n) DELETE [x IN [n] WHERE count(x) > 0 | x][0]", _in_where("count()")), + ("UNWIND [1, 2] AS i RETURN i SKIP size([x IN [1] WHERE sum(x) > 0])", _in_where("sum()")), ("CREATE (n:N {v: [x IN [1, 2] WHERE avg(x) > 0 | x]})", _in_where("avg()")), ("FOREACH (i IN [1] | CREATE (:N {v: [x IN [1, 2] WHERE max(x) > 0 | x]}))", _in_where("max()")), ], @@ -729,6 +732,14 @@ def _ungrouped_term(variable: str) -> str: "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY [w IN collect(z) | w + z]", _ungrouped_term("z"), ), + ( + "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY reduce(s = 0, w IN collect(x) | z)", + _ungrouped_term("z"), + ), + ( + "UNWIND [1, 2] AS x UNWIND [3] AS z RETURN x, count(*) AS c ORDER BY any(w IN collect(x) WHERE w > z)", + _ungrouped_term("z"), + ), ( "MATCH (me:Person)--(you:Person) RETURN count(you.age) AS agg ORDER BY me.age + count(you.age)", _ungrouped_term("me"), @@ -782,6 +793,9 @@ def test_an_aggregating_item_or_order_term_that_names_a_variable_no_item_groups_ "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p AS q, count(*) AS c ORDER BY size(keys(p {.name})) + count(*)", "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p AS q, count(*) AS c ORDER BY p {.born}.born + count(*)", "MATCH (p:Person)-[:ACTED_IN]->(m) RETURN p, count(*) AS c ORDER BY size([(p)-[:KNOWS]->(k) | k]) + count(*)", + # A key named in another case, or spaced and bracketed otherwise: Neo4j names a function in any case. + "UNWIND [{a: 'x'}] AS x RETURN toUpper(x.a) AS u, count(*) AS c ORDER BY TOUPPER(x.a) + count(*)", + "UNWIND [{a: 'x'}] AS x RETURN toUpper( x.a ) AS u, count(*) AS c ORDER BY (toUpper(x.a)) + count(*)", ], ) def test_a_body_or_order_term_that_names_only_what_the_projection_groups_by_is_compiled(