Skip to content

Commit abf6aae

Browse files
lmeyerovclaude
andcommitted
Merge perf/gfql-fused-tolower-one-sided into fix/gfql-polars-agg-string-divergence (linear PR stack)
Stacked so the shared CHANGELOG stops conflicting on every landing: this branch now sits on top of perf/gfql-fused-tolower-one-sided rather than beside it. Files touched by both were 3-way merged with zero conflict markers; CHANGELOG resolved structurally, with this branch's entry re-inserted into the parent's file under the same section. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
2 parents 5ef3d0f + 648d222 commit abf6aae

8 files changed

Lines changed: 1654 additions & 11 deletions

File tree

bin/ci_cypher_surface_guard_baseline.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@
1313
"max_properties": 0
1414
}
1515
},
16-
"lowering_py_max_lines": 9255
16+
"lowering_py_max_lines": 9260
1717
}

bin/test-polars.sh

100755100644
File mode changed.

graphistry/compute/gfql/cypher/lowering.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@
130130
compile_cypher_call,
131131
)
132132
from graphistry.compute.gfql.cypher.ast_normalizer import ASTNormalizer
133+
from graphistry.compute.gfql.expr_const_fold import fold_constants
133134
from graphistry.compute.gfql.cypher.shortest_path_aliases import (
134135
_ShortestPathAliasSpec,
135136
_is_variable_length_relationship_pattern,
@@ -1795,7 +1796,11 @@ def _row_expr_arg(
17951796
node,
17961797
integer_identifiers=frozenset(),
17971798
)
1798-
return _render_expr_node(rewritten)
1799+
# Plan-time constant folding, so ONE canonical predicate text reaches the row
1800+
# evaluators and the residual translator whichever equivalent spelling was
1801+
# written. Runs AFTER the integer-division rewrite (order is load-bearing) and
1802+
# only where every engine provably agrees — see expr_const_fold, criterion (E).
1803+
return _render_expr_node(fold_constants(rewritten))
17991804

18001805

18011806
def _projected_source_replacement(binding: _StageColumnBinding) -> str:

graphistry/compute/gfql/expr_const_fold.py

Lines changed: 457 additions & 0 deletions
Large diffs are not rendered by default.

graphistry/tests/compute/gfql/cypher/test_expr_const_fold.py

Lines changed: 590 additions & 0 deletions
Large diffs are not rendered by default.

graphistry/tests/compute/gfql/test_const_fold_engine_parity.py

Lines changed: 453 additions & 0 deletions
Large diffs are not rendered by default.

graphistry/tests/compute/gfql/test_polars_lane_completeness.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@
4747
"graphistry/tests/test_compute_filter_by_dict.py": (
4848
"pandas oracle only; the sentence naming polars is a docstring cross-reference"
4949
),
50+
"graphistry/tests/compute/gfql/cypher/test_expr_const_fold.py": (
51+
"engine-free: parse -> fold -> render text assertions only, no frame is ever built; "
52+
"the cross-engine half of that pass lives in test_const_fold_engine_parity.py, which "
53+
"IS in the lane. The word polars here is a docstring cross-reference"
54+
),
5055
"graphistry/tests/compute/gfql/index/test_index_gpu_edge_match.py": (
5156
"cudf/GPU-gated (module-level importorskip('cudf') + skipif no GPU), not polars-gated; "
5257
"belongs to the separate GPU-lane gap, and the polars CPU lane could not run it"

graphistry/tests/compute/gfql/test_residual_polars_native.py

Lines changed: 142 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,14 @@ def _canon(df):
5252
class TestResidualTranslator:
5353
@requires_polars
5454
def test_tolower_eq_casefold(self):
55-
expr = fp._residual_polars_expr("(tolower(a.name) = tolower('ALICE'))", "a", COLS())
55+
expr = fp._residual_polars_expr("(tolower(a.name) = 'alice')", "a", COLS())
5656
assert expr is not None
5757
out = _pl_nodes().filter(expr)
5858
assert sorted(out["node_id"].to_list()) == [1, 2]
5959

6060
@requires_polars
6161
def test_tolower_eq_null_dropped(self):
62-
expr = fp._residual_polars_expr("(tolower(a.name) = tolower('bob'))", "a", COLS())
62+
expr = fp._residual_polars_expr("(tolower(a.name) = 'bob')", "a", COLS())
6363
out = _pl_nodes().filter(expr)
6464
assert sorted(out["node_id"].to_list()) == [3, 6] # null name row 4 dropped
6565

@@ -101,7 +101,14 @@ def test_negative_int_literal(self):
101101
@pytest.mark.parametrize("bad", [
102102
"(a.name <> 'x')", # unsupported operator
103103
"(a.name CONTAINS 'x')", # unsupported predicate
104-
"(tolower(a.name) = 'x')", # rhs not tolower-wrapped
104+
"(tolower(a.name) = tolower('x'))", # two-sided: the lowering folds it away first
105+
"(tolower(b.name) = 'alice')", # alias mismatch (checked with alias='a')
106+
"(toupper(b.name) = 'ALICE')", # alias mismatch, other case fn
107+
"(upper(zz.name) = 'ALICE')", # alias mismatch, GQL alias spelling
108+
"('x' = tolower(a.name))", # reversed operand order
109+
"(tolower(a.name) = b.name)", # rhs is a column, not a literal
110+
"(tolower(a.name) = 25)", # rhs is a number, not a string literal
111+
"(substring(a.name, 0, 2) = 'x')", # a foldable fn on the COLUMN is not this shape
105112
"((a.age = 25) AND (a.age = 30))", # compound
106113
"a.age = 25", # missing outer parens
107114
"(b.age = 25)", # alias mismatch (checked with alias='a')
@@ -117,7 +124,7 @@ def test_fast_lane_matches_chain_fallback(self, monkeypatch):
117124
"""The fast lane and the where_rows chain fallback agree byte-for-byte."""
118125
nodes = _pl_nodes()
119126
g = _pl_graph(nodes)
120-
exprs = ["(tolower(a.name) = tolower('Alice'))", "(a.age >= 25)"]
127+
exprs = ["(tolower(a.name) = 'alice')", "(a.age >= 25)"]
121128
fast = fp._connected_join_apply_node_residuals(
122129
g, nodes, "a", exprs, "node_id", engine=Engine.POLARS)
123130
# force the fallback by declining every translation
@@ -137,7 +144,7 @@ def test_mixed_group_falls_back_whole(self, monkeypatch):
137144
"""
138145
nodes = _pl_nodes()
139146
g = _pl_graph(nodes)
140-
exprs = ["(a.age >= 25)", "(tolower(a.name) = tolower('alice'))"]
147+
exprs = ["(a.age >= 25)", "(tolower(a.name) = 'alice')"]
141148
real = fp._residual_polars_expr
142149
calls = []
143150

@@ -171,7 +178,7 @@ def boom(*a, **k):
171178
raise AssertionError("fast lane must not engage on pandas frames")
172179
monkeypatch.setattr(fp, "_residual_polars_expr", boom)
173180
out = fp._connected_join_apply_node_residuals(
174-
g, nodes, "a", ["(tolower(a.name) = tolower('alice'))"], "node_id",
181+
g, nodes, "a", ["(tolower(a.name) = 'alice')"], "node_id",
175182
engine=Engine.PANDAS)
176183
assert sorted(out["node_id"].tolist()) == [1, 2]
177184

@@ -185,14 +192,14 @@ class TestResidualDtypeAndEscapeGates:
185192
def test_escaped_literal_declines(self):
186193
# renderer escapes ' \\ \n etc to \uXXXX text; raw regex compare would mismatch
187194
assert fp._residual_polars_expr(
188-
"(tolower(a.name) = tolower('It\\u0027s'))", "a", COLS()) is None
195+
"(tolower(a.name) = 'it\\u0027s')", "a", COLS()) is None
189196
assert fp._residual_polars_expr(
190197
"(a.name = 'C:\\u005Cx')", "a", COLS()) is None
191198

192199
@requires_polars
193200
@pytest.mark.parametrize("expr", [
194201
"(a.age = 'thirty')", # string literal vs numeric column
195-
"(tolower(a.age) = tolower('x'))", # tolower on numeric column
202+
"(tolower(a.age) = 'x')", # tolower on numeric column
196203
"(a.name >= 25)", # numeric literal vs string column
197204
])
198205
def test_dtype_mismatch_declines(self, expr):
@@ -203,7 +210,7 @@ def test_categorical_column_declines(self):
203210
nodes = pl.DataFrame({"node_id": [1], "cat": ["x"]}).with_columns(
204211
pl.col("cat").cast(pl.Categorical))
205212
assert fp._residual_polars_expr(
206-
"(tolower(a.cat) = tolower('x'))", "a", dict(nodes.schema)) is None
213+
"(tolower(a.cat) = 'x')", "a", dict(nodes.schema)) is None
207214
assert fp._residual_polars_expr("(a.cat = 'x')", "a", dict(nodes.schema)) is None
208215

209216
@requires_polars
@@ -348,3 +355,129 @@ def test_count_star_declines_two_star_fast_path(self, monkeypatch):
348355
res = g.gfql(q, engine="polars")
349356
assert not any(calls), "count(*) unexpectedly reached the fused lane"
350357
assert self._rows(res) # still answered (general path)
358+
359+
# --- CONSTANT FOLDING: one canonical residual shape reaches the translator ------
360+
361+
#: The BOARD's own spelling (benchmarks/graphbench/matched_q1_q9/gb_queries.py,
362+
#: md5 6e7ae268a5a41742587fcb87854b6e27): a ONE-SIDED toLower with an already
363+
#: lowercase literal. Master declines this and drops the whole fused lane.
364+
Q_ONE_SIDED = ("MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), "
365+
"(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) "
366+
"WHERE toLower(i.interest) = 'fine dining' AND p.gender = 'male' "
367+
"RETURN c.city AS city, count(p) AS n ORDER BY n DESC, city LIMIT 5")
368+
369+
def _spy_residual_texts(self, monkeypatch):
370+
"""Record the residual STRINGS the translator is asked to handle."""
371+
seen = []
372+
real = fp._residual_polars_expr
373+
374+
def spy(expr, alias, columns):
375+
out = real(expr, alias, columns)
376+
seen.append((expr, out is not None))
377+
return out
378+
379+
monkeypatch.setattr(fp, "_residual_polars_expr", spy)
380+
return seen
381+
382+
@requires_polars
383+
def test_two_sided_query_reaches_the_translator_already_folded(self, monkeypatch):
384+
"""CANONICALIZATION, observed at the fast-path boundary: the user writes the
385+
TWO-SIDED form, and what arrives here is the ONE-SIDED text. This is what
386+
makes a single matcher shape sufficient."""
387+
g = self._star_graph()
388+
seen = self._spy_residual_texts(monkeypatch)
389+
g.gfql(self.Q, engine="polars")
390+
tolower_exprs = [e for e, _ in seen if "tolower" in e]
391+
assert tolower_exprs, "no toLower residual reached the translator"
392+
assert all(e == "(tolower(i.interest) = 'fine dining')" for e in tolower_exprs), \
393+
f"expected the folded one-sided text, got {tolower_exprs}"
394+
395+
@requires_polars
396+
def test_one_sided_residual_engages_fused_lane(self, monkeypatch):
397+
"""STRUCTURAL LOCK-IN (not a timing gate): a single untranslatable residual
398+
declines the ENTIRE fused lane, so `served == 1` is the regression signal.
399+
A scaling-ladder gate is the wrong shape here -- the removed cost is a
400+
per-op constant, so the residual O(N) term dominates any growth ratio."""
401+
g = self._star_graph()
402+
calls = self._spy_fused(monkeypatch)
403+
g.gfql(self.Q_ONE_SIDED, engine="polars")
404+
assert calls.count(True) == 1, (
405+
f"fused lane served {calls.count(True)} times, expected 1 "
406+
"(0 => the one-sided toLower residual stopped translating)")
407+
408+
@requires_polars
409+
def test_one_sided_fused_matches_eager_chain_path(self, monkeypatch):
410+
g = self._star_graph()
411+
calls = self._spy_fused(monkeypatch)
412+
fused = g.gfql(self.Q_ONE_SIDED, engine="polars")
413+
assert calls and calls[-1], "fused lane did not engage (vacuous comparison)"
414+
monkeypatch.setattr(fp, "_residual_polars_expr", lambda *a, **k: None)
415+
eager = g.gfql(self.Q_ONE_SIDED, engine="polars")
416+
assert self._rows(fused) == self._rows(eager)
417+
# `Fine Dining` + `fine dining` both fold on the COLUMN side -> persons 1, 2, 4
418+
assert self._rows(fused) == [{"city": "London", "n": 2}, {"city": "london", "n": 1}]
419+
420+
@requires_polars
421+
@pytest.mark.parametrize("lit", ["FINE DINING", "Fine Dining", "fine Dining"])
422+
def test_one_sided_mixed_case_literal_matches_nothing_end_to_end(self, lit, monkeypatch):
423+
"""THE TRAP, end to end. A mixed-case ONE-SIDED literal must return the SAME
424+
(empty) answer through the fused lane as through the chain evaluator: the
425+
evaluator does NOT case-fold a bare literal, and neither may the translator.
426+
The two-sided form of the same query returns rows -- pinned below, so this is
427+
not a vacuous 'everything is empty' assertion. Every board literal is already
428+
lowercase, which is exactly why a wrong rule here would ship green."""
429+
g = self._star_graph()
430+
q = self.Q_ONE_SIDED.replace("'fine dining'", f"'{lit}'")
431+
calls = self._spy_fused(monkeypatch)
432+
fused = g.gfql(q, engine="polars")
433+
assert calls and calls[-1], "fused lane did not engage"
434+
monkeypatch.setattr(fp, "_residual_polars_expr", lambda *a, **k: None)
435+
eager = g.gfql(q, engine="polars")
436+
assert self._rows(fused) == self._rows(eager) == []
437+
# control: folding the literal (two-sided) DOES match -> the empty answer above
438+
# is a real semantic difference, not an inert query
439+
assert self._rows(g.gfql(q.replace(f"'{lit}'", f"toLower('{lit}')"), engine="polars"))
440+
441+
@requires_polars
442+
def test_one_sided_matches_pandas_oracle(self, monkeypatch):
443+
g = self._star_graph()
444+
gpd = graphistry.nodes(g._nodes.to_pandas(), "node_id").edges(
445+
g._edges.to_pandas(), "src", "dst")
446+
calls = self._spy_fused(monkeypatch)
447+
got = g.gfql(self.Q_ONE_SIDED, engine="polars")._nodes
448+
assert calls and calls[-1], "fused lane did not engage"
449+
got = (got.to_pandas() if hasattr(got, "to_pandas") else got).to_dict("records")
450+
assert got == gpd.gfql(self.Q_ONE_SIDED, engine="pandas")._nodes.to_dict("records")
451+
452+
@requires_polars
453+
@pytest.mark.parametrize("fn,lit", [
454+
("toUpper", "FINE DINING"), ("upper", "FINE DINING"), ("lower", "fine dining"),
455+
])
456+
def test_other_case_functions_engage_and_match_the_evaluator(self, fn, lit, monkeypatch):
457+
"""The generalization is not toLower-shaped: every case function the row
458+
evaluator supports takes the same lane, on the same canonical text."""
459+
g = self._star_graph()
460+
q = self.Q_ONE_SIDED.replace("toLower(i.interest) = 'fine dining'",
461+
f"{fn}(i.interest) = '{lit}'")
462+
calls = self._spy_fused(monkeypatch)
463+
fused = g.gfql(q, engine="polars")
464+
assert calls and calls[-1], f"{fn}: fused lane did not engage"
465+
monkeypatch.setattr(fp, "_residual_polars_expr", lambda *a, **k: None)
466+
assert self._rows(fused) == self._rows(g.gfql(q, engine="polars"))
467+
assert self._rows(fused), f"{fn}: vacuous (empty) comparison"
468+
469+
@requires_polars
470+
def test_non_ascii_two_sided_declines_the_lane_but_not_the_answer(self, monkeypatch):
471+
"""DISCLOSED NARROWING. A non-ASCII two-sided literal is outside the region
472+
where the engines provably agree, so the fold DECLINES, the residual stays
473+
two-sided, and the fused lane declines with it. That costs speed on an exotic
474+
shape and buys back a Python-vs-Rust case-table assumption master was making
475+
silently. The ANSWER must be unchanged -- that is what is asserted."""
476+
g = self._star_graph()
477+
q = self.Q_ONE_SIDED.replace("toLower(i.interest) = 'fine dining'",
478+
"toLower(i.interest) = toLower('FINE DINİNG')")
479+
calls = self._spy_fused(monkeypatch)
480+
declined = g.gfql(q, engine="polars")
481+
assert not any(calls), "non-ASCII two-sided residual unexpectedly served the fused lane"
482+
monkeypatch.setattr(fp, "_residual_polars_expr", lambda *a, **k: None)
483+
assert self._rows(declined) == self._rows(g.gfql(q, engine="polars"))

0 commit comments

Comments
 (0)