diff --git a/drivine/cypher/sql/sql_cypher_compiler.py b/drivine/cypher/sql/sql_cypher_compiler.py index 17a2c72..d4ddfb9 100644 --- a/drivine/cypher/sql/sql_cypher_compiler.py +++ b/drivine/cypher/sql/sql_cypher_compiler.py @@ -2782,6 +2782,7 @@ def carried(column: str, value: Callable[[], Value], aggregates: Expression) -> column = f"s{i + 1}" # An order term that is an item, as written, is that item: so it may be ordered by where rows are compared. item = next((j for j, it in enumerate(items) if it.expression == sort.expression), -1) + over_items = _over_items(sort.expression, items, item_columns) if aggregating else None if contains(sort.expression, lambda it: isinstance(it, Variable) and it.name in anew) and all( it in item_columns for it in variables_in(sort.expression) ): @@ -2791,17 +2792,27 @@ def carried(column: str, value: Callable[[], Value], aggregates: Expression) -> carried(column, lambda item=item: compiled[item][1], sort.expression) elif compared and not contains(sort.expression, _aggregates): after[column] = self._sortable(self._expression(sort.expression, item_columns)) + elif over_items is not None: + # Where rows are grouped, so is one whose every aggregate and key is an item, each such part read by its + # item's name, as _aggregates_first reads it: worked out in the query, each key is read again as the + # GROUP BY reads it (_keys_read), on Oracle through its text (item_property), at each level of an + # operation over it, and `me.age + count(you.age)` compiled to twice what it did before + # (liberation-data/drivine4j#413). + after[column] = self._sortable(self._expression(over_items, item_columns)) else: # One that aggregates reads the rows it aggregates, as an item does; so does one that reads an aggregate # _aggregates_first worked out first, which no item is. Beside a variable-length walk it binds no value # in a subquery: carried out of a WITH's query, one so bound closes Oracle's (23.26) session # (DPY-4011), as `WITH b ORDER BY toString(coalesce(b.name, b.title))` did, its argument long enough to # be bound. Elsewhere it does, so operations nested in it compile to a statement that grows with them, - # not exponentially. - scope = {**select.scope, **dict(compiled)} - term = sort.expression + # not exponentially. Where rows are grouped, a part of it that is an item keying the groups reads that + # item's column value, as the GROUP BY does: Oracle refuses another read of the key (ORA-00979), as + # `me.age` read from the properties beside `me.age AS age` read through its text (item_property) was + # (liberation-data/drivine4j#413). + read, keys = _keys_read(sort.expression, items, columns) if aggregating else (sort.expression, {}) + scope = {**select.scope, **dict(compiled), **keys} carry = functools.partial( - carried, column, lambda term=term, scope=scope: self._expression(term, scope), term + carried, column, lambda read=read, scope=scope: self._expression(read, scope), sort.expression ) if select.walks: self._unshared(carry) @@ -5820,6 +5831,53 @@ def _aggregates(expression: Expression) -> bool: return is_aggregate(expression) or (isinstance(expression, Variable) and expression.name.startswith("#aggregate")) +def _over_items(term: Expression, items: list[ProjectionItem], item_columns: dict[str, Value]) -> Expression | None: + """ + The term, an order term of a grouping projection, with each part of it that is one of the items read by that + item's name, where so it reads no aggregate, no subquery and nothing but the items, `item_columns`; None where it + does. A variable an item is named after is that item, not the variable of that name before it. + """ + + def by_name(part: Expression) -> Expression | None: + if isinstance(part, Variable) and part.name in item_columns: + return part + return next((Variable(it.name) for it in items if it.expression == part), None) + + read = replaced(term, by_name) + if holds_subquery(read) or contains(read, is_aggregate) or not all(it in item_columns for it in variables_in(read)): + return None + return read + + +def _keys_read( + term: Expression, items: list[ProjectionItem], columns: list[tuple[str, Value]] +) -> tuple[Expression, dict[str, Value]]: + """ + The term, each part of it that is an item of a grouping projection keying the groups, one of the items, read by a + name no variable has, and what each such name is bound to: the item's value among the columns, as it is grouped + by. `replaced` reads no comprehension's, quantifier's or reduce's own expressions, so a part it reads is never one + whose variables such a one binds again: a key is read beside `[me IN [1] | me]` as anywhere else. + """ + # The last of two items alike, as drivine4j's `associate` keeps it. + keys = [ + (i, item.expression) + for i, item in reversed(list(enumerate(items))) + if not contains(item.expression, is_aggregate) and variables_in(item.expression) + ] + bound: dict[str, Value] = {} + + def key(part: Expression) -> Expression | None: + i = next((i for i, expression in keys if expression == part), None) + if i is None: + return None + # Named as no variable is, unless quoted so. + name = f"#key {i + 1}" + bound[name] = columns[i][1] + return Variable(name) + + return replaced(term, key), bound + + def children(expression: Expression) -> list[Expression]: if isinstance(expression, (Property, HasLabels)): return [expression.subject] diff --git a/tests/integration/test_cypher_compiled.py b/tests/integration/test_cypher_compiled.py index 1ae1d2b..dc4b0d9 100644 --- a/tests/integration/test_cypher_compiled.py +++ b/tests/integration/test_cypher_compiled.py @@ -4038,3 +4038,104 @@ async def rows(cypher: str) -> list[Any]: for cypher, expected in writes: assert await rows(cypher) == expected, cypher await clear(cypher_pm) + + +async def test_an_order_term_over_an_aggregate_that_reads_a_grouping_key_is_sorted_by_the_key_as_it_is_grouped_by( + cypher_pm: PersistenceManager, +): + # On Oracle `me.age` in the term read the properties, where the key is grouped by as read through its text, and + # Oracle refused it (ORA-00979): #256, liberation-data/drivine4j#413. drivine4j's CypherQueryContract. + async def rows(cypher: str) -> list[Any]: + return await cypher_pm.query(spec(cypher)) + + await clear(cypher_pm) + # Sorted by the key alone, A, B, C, D; by the key and the count, B, A, C, D. + await rows( + "CREATE (a:CyKey {name: 'A', age: 10, born: date('2000-01-01')}), " + "(b:CyKey {name: 'B', age: 11, born: date('2000-01-05')}), " + "(c:CyKey {name: 'C', age: 20, born: date('2000-01-20')}), " + "(d:CyKey {name: 'D', age: 21, born: date('2000-01-25')}), " + "(a)-[:K]->(b), (a)-[:K]->(c), (a)-[:K]->(d)" + ) + match = "MATCH (me:CyKey)--(you:CyKey)" + by_key = [[10, 3], [11, 1], [20, 1], [21, 1]] + by_both = [[11, 1], [10, 3], [20, 1], [21, 1]] + cases: list[tuple[str, list[Any]]] = [ + (f"{match} RETURN me.age AS age, count(you.age) AS cnt ORDER BY me.age + count(you.age)", by_both), + ( + f"{match} WITH me.age AS age, count(you.age) AS cnt ORDER BY me.age + count(you.age) RETURN age, cnt", + by_both, + ), + (f"{match} RETURN DISTINCT me.age AS age, count(you.age) AS cnt ORDER BY me.age + count(you.age)", by_both), + (f"{match} RETURN me.age AS age, count(you.age) AS cnt ORDER BY age + count(you.age)", by_both), + # Sorted, skipped and limited before the WITH's WHERE, as Cypher reads it; a node keying the groups, read by + # property. + ( + f"{match} WITH me.age AS age, count(you.age) AS cnt ORDER BY me.age + count(you.age) SKIP 1 LIMIT 2 " + "WHERE cnt = 1 RETURN age", + [20], + ), + (f"{match} WITH me, count(you) AS cnt ORDER BY me.age * count(you) RETURN me.name", ["B", "C", "D", "A"]), + # A subquery in a term made of items, which _over_items leaves to be worked out in the query that groups. + ( + f"{match} WITH me, count(you) AS cnt ORDER BY COUNT {{ (me)--() }} * count(you) * 100 - me.age " + "RETURN me.name", + ["D", "C", "B", "A"], + ), + # An aggregate that is no item; a comprehension that names its own variable as the key's; the key in a function. + # On Oracle the comprehension's term compiles to 4.7 MB, which Oracle is still parsing minutes later: + over an + # aggregate's result is written out at every level until #235 is fixed. + (f"{match} RETURN me.age AS age, count(*) AS c ORDER BY me.age + count(you.age)", by_both), + *( + [] + if cypher_pm.type is DatabaseType.ORACLE + else [ + ( + f"{match} RETURN me.age AS age, count(you.age) AS cnt " + "ORDER BY me.age + size([me IN [1] | me]) + count(you.age)", + by_both, + ) + ] + ), + (f"{match} RETURN me.age AS age, count(*) AS cnt ORDER BY toString(me.age) + count(*)", by_key), + # Two keys, one a part of the other. + ( + f"{match} RETURN me.age AS a, me.age + 1 AS b, count(*) AS cnt " + "ORDER BY me.age + 1 + count(*), me.age * 2 - count(*)", + [[11, 12, 1], [10, 11, 3], [20, 21, 1], [21, 22, 1]], + ), + # Beside a variable-length walk, where the key is read as elsewhere. + ( + "MATCH (me:CyKey)-[:K*1..1]-(you:CyKey) RETURN me.age AS age, count(you) AS cnt " + "ORDER BY me.age * count(you)", + [[11, 1], [20, 1], [21, 1], [10, 3]], + ), + (f"{match} RETURN me.age AS age, count(you.age) AS cnt ORDER BY me.age", by_key), + # A key that is an expression, and several keys. + ( + f"{match} RETURN me.age + 1 AS k, count(*) AS cnt ORDER BY me.age + 1 + count(*)", + [[12, 1], [11, 3], [21, 1], [22, 1]], + ), + ( + f"{match} RETURN me.name AS name, me.age AS age, count(*) AS cnt ORDER BY me.age * count(*) DESC, me.name", + [["A", 10, 3], ["D", 21, 1], ["C", 20, 1], ["B", 11, 1]], + ), + ] + # Each that differs, so a store's every difference shows at once. + differing: list[str] = [] + for cypher, expected in cases: + try: + given = await rows(cypher) + except Exception as e: # noqa: BLE001 + differing.append(f"{cypher}: {str(e).splitlines()[0]}") + continue + if given != expected: + differing.append(f"{cypher}: {given}") + assert differing == [] + # A temporal key: A's 30 days are past C's 10. + temporal = ( + f"{match} RETURN me.name AS name, me.born AS born, count(*) AS cnt " + "ORDER BY me.born + duration({days: 10 * count(*)})" + ) + assert [it[0] for it in await rows(temporal)] == ["B", "C", "A", "D"], temporal + await clear(cypher_pm) diff --git a/tests/sync_integration/test_cypher_compiled.py b/tests/sync_integration/test_cypher_compiled.py index 9e00b7b..e2149b1 100644 --- a/tests/sync_integration/test_cypher_compiled.py +++ b/tests/sync_integration/test_cypher_compiled.py @@ -4005,3 +4005,104 @@ def rows(cypher: str) -> list[Any]: for cypher, expected in writes: assert rows(cypher) == expected, cypher clear(cypher_pm) + + +def test_an_order_term_over_an_aggregate_that_reads_a_grouping_key_is_sorted_by_the_key_as_it_is_grouped_by( + cypher_pm: PersistenceManager, +): + # On Oracle `me.age` in the term read the properties, where the key is grouped by as read through its text, and + # Oracle refused it (ORA-00979): #256, liberation-data/drivine4j#413. drivine4j's CypherQueryContract. + def rows(cypher: str) -> list[Any]: + return cypher_pm.query(spec(cypher)) + + clear(cypher_pm) + # Sorted by the key alone, A, B, C, D; by the key and the count, B, A, C, D. + rows( + "CREATE (a:CyKey {name: 'A', age: 10, born: date('2000-01-01')}), " + "(b:CyKey {name: 'B', age: 11, born: date('2000-01-05')}), " + "(c:CyKey {name: 'C', age: 20, born: date('2000-01-20')}), " + "(d:CyKey {name: 'D', age: 21, born: date('2000-01-25')}), " + "(a)-[:K]->(b), (a)-[:K]->(c), (a)-[:K]->(d)" + ) + match = "MATCH (me:CyKey)--(you:CyKey)" + by_key = [[10, 3], [11, 1], [20, 1], [21, 1]] + by_both = [[11, 1], [10, 3], [20, 1], [21, 1]] + cases: list[tuple[str, list[Any]]] = [ + (f"{match} RETURN me.age AS age, count(you.age) AS cnt ORDER BY me.age + count(you.age)", by_both), + ( + f"{match} WITH me.age AS age, count(you.age) AS cnt ORDER BY me.age + count(you.age) RETURN age, cnt", + by_both, + ), + (f"{match} RETURN DISTINCT me.age AS age, count(you.age) AS cnt ORDER BY me.age + count(you.age)", by_both), + (f"{match} RETURN me.age AS age, count(you.age) AS cnt ORDER BY age + count(you.age)", by_both), + # Sorted, skipped and limited before the WITH's WHERE, as Cypher reads it; a node keying the groups, read by + # property. + ( + f"{match} WITH me.age AS age, count(you.age) AS cnt ORDER BY me.age + count(you.age) SKIP 1 LIMIT 2 " + "WHERE cnt = 1 RETURN age", + [20], + ), + (f"{match} WITH me, count(you) AS cnt ORDER BY me.age * count(you) RETURN me.name", ["B", "C", "D", "A"]), + # A subquery in a term made of items, which _over_items leaves to be worked out in the query that groups. + ( + f"{match} WITH me, count(you) AS cnt ORDER BY COUNT {{ (me)--() }} * count(you) * 100 - me.age " + "RETURN me.name", + ["D", "C", "B", "A"], + ), + # An aggregate that is no item; a comprehension that names its own variable as the key's; the key in a function. + # On Oracle the comprehension's term compiles to 4.7 MB, which Oracle is still parsing minutes later: + over an + # aggregate's result is written out at every level until #235 is fixed. + (f"{match} RETURN me.age AS age, count(*) AS c ORDER BY me.age + count(you.age)", by_both), + *( + [] + if cypher_pm.type is DatabaseType.ORACLE + else [ + ( + f"{match} RETURN me.age AS age, count(you.age) AS cnt " + "ORDER BY me.age + size([me IN [1] | me]) + count(you.age)", + by_both, + ) + ] + ), + (f"{match} RETURN me.age AS age, count(*) AS cnt ORDER BY toString(me.age) + count(*)", by_key), + # Two keys, one a part of the other. + ( + f"{match} RETURN me.age AS a, me.age + 1 AS b, count(*) AS cnt " + "ORDER BY me.age + 1 + count(*), me.age * 2 - count(*)", + [[11, 12, 1], [10, 11, 3], [20, 21, 1], [21, 22, 1]], + ), + # Beside a variable-length walk, where the key is read as elsewhere. + ( + "MATCH (me:CyKey)-[:K*1..1]-(you:CyKey) RETURN me.age AS age, count(you) AS cnt " + "ORDER BY me.age * count(you)", + [[11, 1], [20, 1], [21, 1], [10, 3]], + ), + (f"{match} RETURN me.age AS age, count(you.age) AS cnt ORDER BY me.age", by_key), + # A key that is an expression, and several keys. + ( + f"{match} RETURN me.age + 1 AS k, count(*) AS cnt ORDER BY me.age + 1 + count(*)", + [[12, 1], [11, 3], [21, 1], [22, 1]], + ), + ( + f"{match} RETURN me.name AS name, me.age AS age, count(*) AS cnt ORDER BY me.age * count(*) DESC, me.name", + [["A", 10, 3], ["D", 21, 1], ["C", 20, 1], ["B", 11, 1]], + ), + ] + # Each that differs, so a store's every difference shows at once. + differing: list[str] = [] + for cypher, expected in cases: + try: + given = rows(cypher) + except Exception as e: # noqa: BLE001 + differing.append(f"{cypher}: {str(e).splitlines()[0]}") + continue + if given != expected: + differing.append(f"{cypher}: {given}") + assert differing == [] + # A temporal key: A's 30 days are past C's 10. + temporal = ( + f"{match} RETURN me.name AS name, me.born AS born, count(*) AS cnt " + "ORDER BY me.born + duration({days: 10 * count(*)})" + ) + assert [it[0] for it in rows(temporal)] == ["B", "C", "A", "D"], temporal + clear(cypher_pm) diff --git a/tests/unit/test_oracle_item_property.py b/tests/unit/test_oracle_item_property.py index 506f2b5..4d32df5 100644 --- a/tests/unit/test_oracle_item_property.py +++ b/tests/unit/test_oracle_item_property.py @@ -318,6 +318,14 @@ def read(cypher: str) -> str: assert "JSON_VALUE(JSON_QUERY(n1.props, '$.m' RETURNING JSON), '$.a.size()' RETURNING NUMBER)" in nested, nested +def test_a_grouping_key_an_order_term_over_the_items_reads_is_read_through_its_text_once_for_item_and_group_by(): + # liberation-data/drivine4j#413: the term read the key from the properties again, `JSON_VALUE(n1.props, '$.age' …)`, + # which isn't what the GROUP BY holds, and Oracle refused it (ORA-00979). It reads the items' columns now. + sql = oracle("MATCH (me:P)--(you:P) RETURN me.age AS age, count(you.age) AS cnt ORDER BY me.age + count(you.age)") + assert sql.count("JSON_SERIALIZE") == 2, sql + assert "JSON_VALUE(n1.props, '$.age" not in sql, sql + + def test_a_map_whose_key_holds_a_parenthesis_is_read_by_its_path_and_two_values_read_together_are_left_as_they_are(): # The parenthesis is in a literal, so the map taken out of the properties is still the one argument. sql = oracle("MATCH (n:Doc) RETURN size(n.`a)`.b)")