From 67db5ba5225311e06b3cb61090c2dc46e0fd40c4 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Fri, 6 Nov 2026 01:36:40 +1000 Subject: [PATCH] Test the labels of a node a MATCH finds after a grouping WITH by their text on Oracle, which crashed the session building its cursor (#418) Port of liberation-data/drivine4j#606. On Oracle 23.26 a labelled node a MATCH finds after a WITH that aggregates or is DISTINCT (or any WITH after one) could crash the session when tested against the multivalue label index (ORA-00600 [QJSNMV_GET_MVI_MD:2]). A new store hook, grouped_labels, writes such a test: has_labels on every store but Oracle, where it is holds_label's INSTR form. OPTIONAL MATCH and shortestPath laterals inherit the flag, and a WHERE n:Label on a node found after the grouping uses it too. Closes #418 --- docs/04-statements.md | 11 ++ drivine/cypher/sql/oracle_cypher_compiler.py | 14 ++ drivine/cypher/sql/sql_cypher_compiler.py | 33 +++- tests/unit/test_cypher_grouped_labels.py | 154 +++++++++++++++++++ 4 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 tests/unit/test_cypher_grouped_labels.py diff --git a/docs/04-statements.md b/docs/04-statements.md index 362605d..65514b5 100644 --- a/docs/04-statements.md +++ b/docs/04-statements.md @@ -240,6 +240,17 @@ people = await pm.query(spec.bind({"city": "Sydney"})) not prevent it. The subquery then reads the node table whole, once ([drivine#612](https://github.com/liberation-data/drivine/issues/612), [drivine4j#595](https://github.com/liberation-data/drivine4j/issues/595)). +- On Oracle, a node a compiled `MATCH` finds after a `WITH` that groups its rows (one that aggregates or is + `DISTINCT`, or a `WITH` after one) tests its labels by their text, which the index over labels can't answer, in its + pattern and in a `WHERE` such as `WHERE m:Movie`. With the index's form, Oracle 23.26 sometimes crashed the session + building the statement's cursor (ORA-03113, with ORA-00600 `[QJSNMV_GET_MVI_MD:2]` in the alert log, or ORA-07445 + `[qjsnmvFindOpt()+16]` after a rolled-back delete), on `MATCH (p:Person)-[r:ACTED_IN]->() WITH p, collect(DISTINCT + [r]) AS l, count(DISTINCT [r]) AS n MATCH (p)-[r2:ACTED_IN]->(:Movie {title: 'The Matrix'}) RETURN …` + ([#418](https://github.com/liberation-data/drivine-python/issues/418), + [drivine4j#601](https://github.com/liberation-data/drivine4j/issues/601), + [drivine#620](https://github.com/liberation-data/drivine/issues/620)). Such a node is mostly found by its key, from + a node the `WITH` passed on; one found by its label alone is read from the whole node table. The nodes before the + `WITH`, a `WHERE`'s label test of a node the `WITH` passed on, and every other read keep the index. - What each statement gives is the [Cypher spec](https://github.com/liberation-data/drivine-cypher-spec), which the three Drivine libraries run alike. diff --git a/drivine/cypher/sql/oracle_cypher_compiler.py b/drivine/cypher/sql/oracle_cypher_compiler.py index 282b798..e5fa38a 100644 --- a/drivine/cypher/sql/oracle_cypher_compiler.py +++ b/drivine/cypher/sql/oracle_cypher_compiler.py @@ -864,6 +864,20 @@ def keyed_labels(self, labels: str, all_: Sequence[str]) -> str: self._keyed.add(test) return test + def grouped_labels(self, labels: str, all_: Sequence[str]) -> str: + """ + Each label by the labels' text (`holds_label`), which the index over labels can't answer, for a node a `MATCH` + finds after a `WITH` that groups its rows. Answered by that index, Oracle (23.26) sometimes crashed the session + building the cursor (ORA-00600 `[QJSNMV_GET_MVI_MD:2]`, ORA-03113 to the client), after planning, as it put the + label test it had rewritten onto the index's column back as it was: `MATCH (p:Person)-[r:ACTED_IN]->() WITH p, + collect(DISTINCT [r]) AS l, count(DISTINCT [r]) AS n MATCH (p)-[r2:ACTED_IN]->(:Movie {title: 'The Matrix'}) + RETURN …` (#418, liberation-data/drivine4j#601, liberation-data/drivine#620); after a rolled-back delete of + nodes of the label it crashed in the same index as a `DELETE`'s read did (ORA-07445 `[qjsnmvFindOpt()+16]`, + #410). The nodes before the `WITH` keep the index's form. Such a node is mostly found from a node the `WITH` + passed on, by its key, so only a node found by its label and nothing else is read without the index. + """ + return " AND ".join(holds_label(labels, it) for it in all_) + def key_value(self, json: str) -> str: """A number, as the key's `NUMBER`; anything else, or a number past one, null.""" return f"JSON_VALUE({json}, '$' RETURNING NUMBER NULL ON ERROR)" diff --git a/drivine/cypher/sql/sql_cypher_compiler.py b/drivine/cypher/sql/sql_cypher_compiler.py index 97ccd7b..55f00c8 100644 --- a/drivine/cypher/sql/sql_cypher_compiler.py +++ b/drivine/cypher/sql/sql_cypher_compiler.py @@ -404,6 +404,10 @@ def __init__(self, outer: Scope, joined: Callable[[str], str]) -> None: self.walks = False """Whether a variable-length walk is among its sources, or among those of a `WITH` it reads: a term it sorts by is then bound in no subquery (see `_project`).""" + self.after_grouping = False + """Whether it reads the rows of a `WITH` that groups them, as one that aggregates or is `DISTINCT` does, or of a + `WITH` that reads those, or is the lateral query of an `OPTIONAL MATCH` or a shortest path read beside such + rows: a node a pattern finds here is then tested for its labels as `grouped_labels` tests them.""" self._label_tests: dict[str, list[_LabelTest]] = {} """The tests among its `where` of the labels of each node, by the node's key, in the order they were added: see `relabel`.""" @@ -976,6 +980,14 @@ def keyed_labels(self, labels: str, all_: Sequence[str]) -> str: """ return self.has_labels(labels, all_) + def grouped_labels(self, labels: str, all_: Sequence[str]) -> str: + """ + Whether the labels, a JSON list, hold each of `all_`, for a node a `MATCH` finds after a `WITH` that groups its + rows (see `_Select.after_grouping`): one read beside the grouped rows, which the query can't merge into its own. + As `has_labels` tests them. A store whose planner fails on that shape tests them otherwise: see Oracle's. + """ + return self.has_labels(labels, all_) + @abstractmethod def elements(self, json: str, alias: str) -> str: """The items of the JSON list as rows named `alias`, with columns `item` and `item_no`, counted from 1.""" @@ -1745,6 +1757,12 @@ def _begin(self, parameters: Parameters) -> None: end, or their key compared with a value (`_id_equality`, `_keyed_in`), each in the part that binds the node (`_anchor`). Each key is a column of its own query, so this holds them for the whole statement. """ + self._grouped: set[str] = set() + """ + The keys of the nodes a pattern found after a `WITH` that groups its rows (`_Select.after_grouping`): a label a + condition tests of one is tested as its pattern's are, by `grouped_labels`. A node the `WITH` passed on is not + among them. + """ self._made_anew: set[str] = set() """ The variables made here for a randomUUID() made first, for each row: no variable of the statement's, though one @@ -1894,6 +1912,11 @@ def _with_having(self, select: _Select, with_: With, outer: Scope | None = None, name = self._alias("w") next_ = self._select(outer or {}) next_.walks = select.walks + next_.after_grouping = ( + select.after_grouping + or with_.projection.distinct + or any(contains(it.expression, is_aggregate) for it in with_.projection.items) + ) # A value made anew for each row is one value however often the clauses after read it: see `once`. Kept apart # from the query that reads it, where that query must not have it merged in: see `_cases_first`. anew = any(holds_anew(it.expression) for it in with_.projection.items) @@ -2044,6 +2067,7 @@ def _optional_match( With `seed`, the variable of a UUID made first for each row, each randomUUID() of `where` is read beside it. """ inner = self._select(row.scope) + inner.after_grouping = row.after_grouping self._patterns(inner, patterns, met_by_paths) if where is not None: if seed is None: @@ -2416,10 +2440,13 @@ def _pattern( value = _node(name) if node.variable is not None: select.bind(node.variable, value) + if select.after_grouping: + self._grouped.add(value.nid) else: raise DrivineError(f"{node.variable} is not a node, and is used as one in a pattern.") if node.labels: - select.label_test(value, node.labels, self.has_labels(value.labels, node.labels)) + labels = self.grouped_labels if select.after_grouping else self.has_labels + select.label_test(value, node.labels, labels(value.labels, node.labels)) if node.properties is not None: select.where.append(self._properties_match(value.props, node.properties, select.scope)) if node.labels or node.properties is not None: @@ -2506,6 +2533,7 @@ def _shortest(self, select: _Select, pattern: Pattern, where: Expression | None) if relationship is None or relationship.range is None or relationship.group is not None: unsupported("A shortest path of anything but one variable-length relationship") inner = self._select(select.scope) + inner.after_grouping = select.after_grouping hops: list[_Hop] = [] # Its ends named, to find the shortest between each two of them where they were not bound before it. first, last = pattern.nodes[0], pattern.nodes[-1] @@ -4512,7 +4540,8 @@ def _value_of(self, expression: Expression, scope: Scope) -> Value: if isinstance(expression, HasLabels): subject = self._expression(expression.subject, scope) if isinstance(subject, Node): - return Bool(f"({self.has_labels(subject.labels, expression.labels)})") + labels = self.grouped_labels if subject.nid in self._grouped else self.has_labels + return Bool(f"({labels(subject.labels, expression.labels)})") raise DrivineError("A label is tested of a node.") if isinstance(expression, ListLiteral): return self._list(expression, scope) diff --git a/tests/unit/test_cypher_grouped_labels.py b/tests/unit/test_cypher_grouped_labels.py new file mode 100644 index 0000000..8df89fa --- /dev/null +++ b/tests/unit/test_cypher_grouped_labels.py @@ -0,0 +1,154 @@ +""" +#418, liberation-data/drivine4j#601, liberation-data/drivine#620: on Oracle, a node a `MATCH` finds after a `WITH` that +groups its rows is tested for its labels by their text, which the index over labels can't answer (`grouped_labels`). +Answered by that index, Oracle sometimes crashed the session building the cursor of the Cypher spec's "Maps and lists +holding relationships compare by identity" (ORA-00600 `[QJSNMV_GET_MVI_MD:2]`). Every other test of a node's labels is +as it was, so the index still finds the nodes of a label. The spec case pins the rows. drivine4j's GroupedLabelsTest is +the reference. +""" + +import re + +import pytest + +from drivine.backend.oracle.oracle_graph import holds_label +from drivine.cypher.sql.oracle_cypher_compiler import OracleCypherCompiler +from drivine.cypher.sql.postgres_cypher_compiler import PostgresCypherCompiler +from drivine.cypher.sql.sql_cypher_compiler import SqlCypherCompiler +from drivine.cypher.sql.sqlite_cypher_compiler import SqliteCypherCompiler + +SPEC_CASE = ( + "MATCH (p:Person)-[r:ACTED_IN]->() WITH p, collect(DISTINCT [r]) AS l, count(DISTINCT [r]) AS n " + "MATCH (p)-[r2:ACTED_IN]->(:Movie {title: 'The Matrix'}) RETURN p.name, size(l), n, [r2] IN l ORDER BY p.name" +) + + +def oracle(cypher: str) -> str: + return OracleCypherCompiler("g").compile(cypher, {}).sql + + +def indexed(alias: str, label: str) -> str: + return f"JSON_EXISTS({alias}.labels, '$[*]?(@ == \"{label}\")')" + + +def label_tests(sql: str, label: str) -> tuple[list[str], list[str]]: + """The aliases of the nodes `sql` tests for `label` in the index's form, and those it tests by the labels' text.""" + index = re.findall(rf"""JSON_EXISTS\((n\d+)\.labels, '\$\[\*]\?\(@ == "{re.escape(label)}"\)'\)""", sql) + serialized = dict.fromkeys(re.findall(r"JSON_SERIALIZE\((n\d+)\.labels\)", sql)) + return index, [it for it in serialized if holds_label(f"{it}.labels", label) in sql] + + +def test_tests_the_labels_of_the_later_node_by_their_text_and_the_grouped_node_through_the_index_in_the_spec_case() -> ( + None +): + sql = oracle(SPEC_CASE) + movie_index, movie_text = label_tests(sql, "Movie") + assert movie_index == [], sql + assert len(movie_text) == 1, sql + person_index, person_text = label_tests(sql, "Person") + assert len(person_index) == 1, sql + assert person_text == [], sql + assert indexed(person_index[0], "Person") in sql, sql + + +@pytest.mark.parametrize( + "cypher", + [ + "MATCH (p:Person) WITH count(p) AS c MATCH (m:Movie) RETURN c, m.title", + "MATCH (p:Person) WITH DISTINCT p.name AS name MATCH (m:Movie) RETURN name, m.title", + "MATCH (p:Person) WITH p, count(*) AS c WITH p, c MATCH (p)-[:ACTED_IN]->(m:Movie) RETURN m.title", + "MATCH (p:Person) WITH p, count(*) AS c WHERE c > 0 MATCH (p)-[:ACTED_IN]->(m:Movie) RETURN m.title", + "MATCH (p:Person) WITH p, count(*) AS c MATCH (p)-[:ACTED_IN]->(m:Movie:Film) RETURN m.title", + ], +) +def test_tests_by_their_text_after_a_with_that_aggregates_is_distinct_or_reads_one_that_does_each_label( + cypher: str, +) -> None: + sql = oracle(cypher) + index, text = label_tests(sql, "Movie") + assert index == [], sql + assert len(text) == 1, sql + if ":Film" in cypher: + assert len(label_tests(sql, "Film")[1]) == 1, sql + + +@pytest.mark.parametrize( + "cypher", + [ + "MATCH (p:Person) WITH p, count(*) AS c OPTIONAL MATCH (p)-[:ACTED_IN]->(m:Movie) RETURN c, m.title", + "MATCH (p:Person) WITH p, count(*) AS c OPTIONAL MATCH (p)-[:ACTED_IN]->(m) WHERE m:Movie RETURN c, m.title", + "MATCH (p:Person) WITH p, count(*) AS c MATCH s = shortestPath((p)-[:ACTED_IN*]->(m:Movie)) RETURN c, m.title", + ], +) +def test_tests_by_their_text_a_node_an_optional_match_or_a_shortest_path_finds_after_such_a_with_in_its_lateral( + cypher: str, +) -> None: + sql = oracle(cypher) + index, text = label_tests(sql, "Movie") + assert index == [], sql + assert len(text) == 1, sql + + +@pytest.mark.parametrize( + "cypher", + [ + "MATCH (p:Person) WITH p, count(*) AS c MATCH (m) WHERE m:Movie RETURN c, m.title", + "MATCH (p:Person) WITH p, count(*) AS c MATCH (p)-[:ACTED_IN]->(m) WHERE m:Movie RETURN c, m.title", + ], +) +def test_tests_by_their_text_a_label_the_where_of_such_a_node_tests_as_one_its_pattern_names(cypher: str) -> None: + sql = oracle(cypher) + index, text = label_tests(sql, "Movie") + assert index == [], sql + assert len(text) == 1, sql + + +def test_a_where_label_test_of_a_node_the_with_passed_on_or_before_the_with_keeps_the_index() -> None: + # A node the WITH passed on is no node found after it: its WHERE tests the WITH's column, in the index's form. + passed = oracle("MATCH (m) WITH m, count(*) AS c MATCH (m)-->() WHERE m:Movie RETURN m.title") + assert re.search(r"""JSON_EXISTS\(w\d+\.c\d+_labels, '\$\[\*]\?\(@ == "Movie"\)'\)""", passed), passed + assert label_tests(passed, "Movie")[1] == [], passed + before = oracle("MATCH (m) WHERE m:Movie WITH m, count(*) AS c RETURN m.title, c") + index, text = label_tests(before, "Movie") + assert len(index) == 1, before + assert text == [], before + + +@pytest.mark.parametrize( + "cypher", + [ + "MATCH (n:Movie) RETURN count(n)", + "MATCH (p:Person) WITH p MATCH (p)-[:ACTED_IN]->(m:Movie) RETURN m.title", + "MATCH (p:Person) WITH p ORDER BY p.name LIMIT 3 MATCH (p)-[:ACTED_IN]->(m:Movie) RETURN m.title", + "MATCH (m:Movie) WITH m, count(*) AS c RETURN m.title, c", + "MATCH (p:Person)-[:ACTED_IN]->(m:Movie) WITH m, count(p) AS c RETURN m.title, c", + ], +) +def test_tests_through_the_index_where_no_with_groups_or_before_the_with_that_does(cypher: str) -> None: + sql = oracle(cypher) + index, text = label_tests(sql, "Movie") + assert len(index) == 1, sql + assert text == [], sql + + +def test_another_union_branch_tests_through_the_index() -> None: + sql = oracle( + "MATCH (p:Person) WITH count(p) AS c MATCH (m:Movie) RETURN m.title AS t " + "UNION MATCH (m:Movie) RETURN m.title AS t" + ) + index, text = label_tests(sql, "Movie") + assert len(index) == 1, sql + assert len(text) == 1, sql + + +GROUPED = "MATCH (p:Person) WITH p, count(*) AS c MATCH (p)-[:ACTED_IN]->(m:Movie) RETURN m.title" + + +@pytest.mark.parametrize("compiler", [PostgresCypherCompiler, SqliteCypherCompiler], ids=lambda it: it.__name__) +def test_leaves_the_test_as_it_was_on_postgres_and_sqlite(compiler: type[SqlCypherCompiler]) -> None: + store = compiler("g") + sql = store.compile(GROUPED, {}).sql + aliases = dict.fromkeys(re.findall(r"\b(n\d+)\.labels", sql)) + assert [it for it in aliases if store.has_labels(f"{it}.labels", ["Movie"]) in sql], sql + if compiler is PostgresCypherCompiler: + assert re.search(r"""n\d+\.labels @> '\["Movie"]'""", sql), sql