perf(gfql): real q1-q9 OLAP lowering on Polars by lmeyerov · Pull Request #1714 · graphistry/pygraphistry

lmeyerov · 2026-07-08T04:13:02Z

Summary

Adds the real-GFQL graph-benchmark q1-q9 Polars path and the supporting OLAP lowering/row-pipeline optimizations needed to run it without dataframe shortcuts or untimed precompute.

Stack position: above #1702 (perf/gfql-row-pushdown) and below #1704 (bench/gfql-fair-matrix).

Validation

git diff --check origin/perf/gfql-row-pushdown...perf/gfql-olap-real-q1q9
python -m py_compile benchmarks/gfql/graph_benchmark_q1_q9_real.py graphistry/compute/gfql_unified.py graphistry/compute/gfql/cypher/lowering.py graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py
python -m pytest -q graphistry/tests/compute/gfql/test_engine_polars_binding_rows.py graphistry/tests/compute/gfql/test_engine_polars_row_pipeline.py graphistry/tests/compute/gfql/cypher/test_row_pushdown.py
python -m pytest -q graphistry/tests/compute/gfql/cypher/test_lowering.py -k '1712 or 1273 or t1 or t3 or t4 or graph_benchmark'
./bin/typecheck.sh

DGX pressure evidence

Artifacts under plans/gfql-olap-optimization/:

dgx_t118_combined_polars.json
dgx_t119_combined_polars_confirm.json
dgx_t121_pokec_medium_combined.json

Graph-benchmark q1-q9 real GFQL Polars: parity OK across q1-q9. Performance is broadly stable vs T28; q5 remains a micro-query outlier to explain/optimize before final report claims.

Pokec medium native indexed traversal: row counts match prior #1658 artifact and GFQL pandas remains 8/8 faster than prior Memgraph medians.

Notes

Draft until CI and review-skill artifacts are refreshed for this exact PR scope.

…dcast (#1707) + count(non-active node alias) routing (#1708) Two correctness fixes that let graph-benchmark q1-q9 run as real GFQL Cypher (no dataframe shortcuts) on pandas/cuDF, and remove a polars silent-wrong answer. `RETURN count(*)` (and any all-scalar-literal projection) returned constant 1 instead of the true count. Cypher WITH/RETURN preserves row cardinality, but polars DataFrame.select of an all-scalar projection collapses to 1 row, so the synthetic __cypher_group__=1 for keyless count(*) made the downstream group_by count see 1 row. select_polars now broadcasts an all-scalar projection to the frame height via with_columns(...).select(names). +5 parity + 2 value-regression tests. `MATCH (a)-[e]->(b) RETURN b.id, count(a)` (graph-bench q1 in-degree) failed with "one MATCH source alias at a time" — the aggregate referenced the other endpoint alias, routing to the single-alias table instead of the bindings-row table. _lower_general_row_projection now forces the bindings source for count(<bare node alias>) (non-distinct) over a non-active pattern node alias — sound on the bindings table (count of matched paths per group = in-degree). Narrow: property/compound cross-source aggregates keep the conservative fail-fast. +3 tests (incl. cuDF). Regression sweep (cypher/ + polars engine + row-pipeline): 2177 passed, 11 skipped, 15 xfailed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

@2m

Every chain()/Cypher call on a graph without an edge-id binding attached a synthetic per-edge index via reset_index + rename, which deep-copied AND block-consolidated the entire edge frame (~70ms @2m edges) — even for node-only queries, and repeatedly per boundary-call sub-chain. Now: shallow copy + assign the frame index as the id column — identical id values (any index type), no O(E) data copy. The column is internal-only and dropped on every exit path, so only uniqueness semantics matter (unchanged). Measured @500k nodes / 2M edges (pandas): 1-row point RETURN 95.7 -> 13.5 ms (7x) full-scan RETURN a 134 -> 55 ms (2.4x) seeded 1-hop 192 -> ~170 ms polars unchanged (own chain path) Scaling (1-row RETURN): 15.6@50k/100@500k -> 6.8@50k/13.5@500k/42@1M. Residual O(N/E) tail (backward-pass hydration merges, endpoints reconciliation) remains tracked in #1670/#887; a hydration-skip variant was evaluated and rejected (row-order semantics risk for ~1.5ms). Suites: compute 4301 passed + gfql 3014 passed (9 pre-existing umap/dask env failures, verified identical pre-fix via stash). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

@2m

Same fix as the chain.py edge-index attach (8765c0bd), applied to hop.py — the traversal hot path. reset_index(drop=False) + rename deep-copied + block-consolidated the whole post-filter edge frame (~80ms @2m edges) on every hop, even seeded ones. Now a shallow copy + assigning the frame index as the id column: identical id values (dedup/join key only, never used positionally — verified no downstream .iloc/.loc/.index dependence), no O(E) copy. Consistent ~10-25ms lower per raw hop/chain traversal @500k/2M pandas; the Cypher RETURN traversal is still dominated by binding materialization (tracked #887). No user-frame contamination; hop + chain + gfql suites green (152 + 3014 passed; 1 pre-existing umap env failure unrelated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

…d patterns (#1709) Native polars bindings-row tables — the rows(binding_ops=...) op emitted by Cypher multi-alias lowering. Unblocks traversal-shaped Cypher on polars: graph-benchmark q1/q2 (top-k in-degree), q8/q9 (fixed 2-hop counts), multi-alias property projections, cross-alias grouped aggregates. - binding_rows_polars: seed filter -> per-edge orient (fwd/rev/undirected, no-dedupe concat) + inner join -> node-filter semi join -> per-alias left-join assembly (alias.{col} node props, edge_alias.{col} payload, bare alias id cols). Same meaningful schema as pandas; pandas' internal join-residue columns intentionally not replicated. - select_extend_polars: native with_(extend=True) (bindings-path aggregate lowering emits it; previously NIE'd). - group_by key_prefixes guard: whole-row bindings grouping would have been silently dropped by the native dispatch once rows() stopped NIE-ing -> now declines honestly (latent wrong-answer trap). - DECLINES (None -> honest NIE, NO-CHEATING): var-length/multihop edges, shortestPath scalar bindings, node query=/edge query/endpoint-match params, hop labels, HAS_-label disambiguation, seeded re-entry contexts, node-cartesian mode, alias_endpoints variant, join-key dtype divergence. Conformance harness: _round_floats now renders non-bool numeric (incl clean-coercible object) columns as float64 on both sides — pandas' hop internals upcast int64->float64 on the 2nd alias (engine artifact; polars faithfully keeps Int64), so the astype(str) gate failed on '20.0' vs '20' for equal values. Same non-semantic noise class as the existing summation-order rounding and null-repr normalization; genuine value differences still fail. Tests: +20 (parity incl q1/q8/q9 shapes, NIE assertions, raw-table schema, path multiplicity, undirected self-loop); DEFERRED->supported moves in the row-pipeline + conformance corpora. gfql suite 3036 passed; compute 4323 passed (9 pre-existing umap/dask env failures). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

…raph-bench q3) Extends the native polars rows(binding_ops) builder (c9b178ad) to bounded directed variable-length segments (-[*1..k]->, -[:TYPE*i..k]->, exactly-k): iterative pair joins hop-by-hop, one row per distinct edge sequence (Cypher path multiplicity; pairs not deduped so parallel edges multiply per hop — matching pandas _gfql_multihop_binding_rows), zero-hop rows for min 0, same min/max defaults as pandas (bare hops=k means exactly-k). Unblocks the graph-benchmark q3 shape on polars: MATCH (a:Person)-[:FOLLOWS*1..2]->(b:Person) WHERE ... RETURN avg(b.age) verified parity vs pandas (values + count) on typed-label replicas. Still honestly deferred (NIE): unbounded [*] (needs fixed-point + termination error), undirected var-length (immediate-backtrack avoidance not ported), aliased var-length edges (pandas rejects outright). Non-aggregate var-length projections route via the traversal chain (separate pre-existing Phase-1 single-hop limit) and keep their honest NIE. Tests: +5 parity (q3 shape, exactly-k, typed, var+fixed compound, seeded avg), DEFERRED repointed at unbounded/undirected. gfql suite 3042 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

…erty joins (#1711) The Cypher lowering now computes which node aliases have their PROPERTIES referenced downstream and threads them as rows(binding_ops, attach_prop_aliases=...); both binding builders (pandas _gfql_connected_bindings_row_frame_from_state and polars binding_rows_polars) skip the O(N) property left-join for aliases not listed — their free bare id column suffices. count(*) attaches zero property columns; count(a) needs only a's bare id; RETURN b.id attaches only b. Conservative gate (attach-all default when unsure): only computed for the simple single-MATCH shape with no WITH stages, no WHERE anywhere, no repeated node alias (bound-identity `n.__gfql_node_id__` refs a RETURN-text walk can't see, #1490), and no collect() (carry/reentry hidden columns, #1413). The referenced set itself is exact (_expr_match_alias_usage non-aggregate refs = property/whole-entity uses). Primary win: unblocks the polars-gpu lazy-build (a large unpruned property-attached intermediate made GPU q8 regress 19×; pruning → the join-chain the de-risk probe clocked at ~12ms on GPU). Also trims polars-cpu q8 (244→185ms). pandas 2-hop is dominated by the state-build merges, not the property attach, so it's ~unchanged there (its bottleneck is separate). Tests: +2 (pushdown skips unused props on both engines; end-to-end parity). gfql 3042 + compute 4331 passed (9 pre-existing umap/dask env failures). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

…1709/#1711) binding_rows_polars now builds ONE deferred pl.LazyFrame and collects once on the active target (CPU / GPU via collect()). Under engine='polars-gpu' the whole join chain + property attach runs on cudf_polars in a single GPU collect. Paired with #1711 projection-pushdown (which prunes the unused property joins that otherwise bloat the GPU intermediate), this delivers the polars-gpu OLAP win WITHOUT the regression the lazy-build-alone caused. Column access uses .collect_schema().names() (schema-only, no data). The var-length loop drops the eager .height early-break (empty lazy joins are identical; the break was an optimization). NO-CHEATING preserved: a GPU-incapable plan node makes collect() raise NotImplementedError (honest NIE), never a silent CPU fallback. Validated on dgx (safe_run, all values correct) @100k nodes/1M edges: q8 2-hop count polars-gpu: 4294ms (lazy-alone regression) -> 54ms (79x); now the FASTEST engine (vs polars-cpu 90, cudf 439, pandas 1122). q1 top-k polars-gpu 30ms (fastest). #1711 also cut all engines on q8 (pandas 4386->1122, cudf 973->439, polars 244->90). (One transient GPU-JIT spike at 50k — non-monotonic 9/311/54ms across sizes.) gfql suite 3044 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

…rk q5/q6/q7 shape) The benchmark shortcuts bypassed the GFQL engine, so the multi-part MATCH...WHERE...WITH <subset> MATCH (alias)... shape (graph-benchmark q5/q6/q7) had NO real-engine correctness test — which is how the silent wrong answer in #1712 went uncaught. Adds the missing xfail(strict)-locked correctness test: a >1-node filtered subset carried via WITH must restrict the re-matched alias (numPersons=2, not 3), with and without collect. Flips to pass when #1712 lands; strict xfail forces re-enabling then. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

…d-alias filter (#1712 silent wrong) Two manifestations of the same silent wrong answer — a connected pattern sharing a node alias failed to intersect the two patterns' constraints, returning unfiltered counts (graph-benchmark q2/q5/q6/q7 shape). Both fixed: 1. Comma-form `MATCH (p)-[R1]->(i),(p)-[R2]->(c) WHERE i.k='x'`: the connected-match-join only applied a WHERE in the `expr_tree` form and SILENTLY DROPPED the structured `predicates` form (a plain `WHERE i.k='x'`). `_where_clause_expr_text` already renders both forms; relax the gate and NIE honestly if unrenderable rather than drop. 2. WITH-form `MATCH (p)-[R1]->(i) WHERE i.k='x' WITH p MATCH (p)-[R2]->(c)`: the bounded-reentry seed (the prefix-filtered `p`) was computed and passed as chain start_nodes, but never wired to `_gfql_start_nodes` for an all-call binding-ops chain (only the traversal->suffix-call boundary path set it), so the binding builder re-matched `p` from the whole graph. Wire it in `_execute_compiled_query_chain_non_union` when the chain is a rows(binding_ops) pipeline. Both correct on pandas and cuDF now (numPersons=2, not 3). This unblocks the benchmark q5/q6/q7 (expressible as the comma-form with tolower). Tests: the previously xfail(strict)-locked subset-carry test now passes (WITH + WITH-collect forms) + a comma-form intersection test. gfql 3047 + compute 4334 passed (9 pre-existing umap/dask env failures). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

…raph-bench q3/q4) `RETURN c.city, avg(p.age)` (group key on one alias, aggregate over another) NIE'd with the misleading "one MATCH source alias at a time". Now routes to the bindings-row table (which materializes every alias, one row per matched path — a standard GROUP BY). Extends the #1708 force-bindings gate from bare-alias count to any CLEAN grouped aggregate func(<alias>.<prop>) (avg/sum/min/max/count) over a non-active node alias. Guarded for soundness: only CLEAN agg/non-agg separation is admitted — an aggregate nested inside a larger expression (`me.age + count(you.age)`, or `age + count(...)` in ORDER BY) keeps the conservative fail-fast (the rejects-unsound-multi-source-overlap contract), detected structurally via _expr_has_aggregate / _is_pure_aggregate_call over RETURN + top-level ORDER BY. Also fixes a latent #1711 bug this exposed: _binding_prop_alias_set dropped an alias referenced only via a property inside an aggregate (`avg(p.age)` needs p's join) — it now includes property-access aliases from inside aggregates (_expr_property_access_node_aliases), and reads the top-level query.order_by (ReturnClause has no order_by attr). avg/sum/count(property) correct on pandas + cuDF (covers q3 avg, q4 count). min/max still NIE honestly (not multiplicity-sensitive, so they don't reach the force-bindings gate — a separate, non-benchmark-blocking gap). +3 tests. gfql 3050 + compute 4337 passed (9 pre-existing umap/dask env failures). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

…pute (#1710) Every query runs g.gfql(<canonical cypher>) on the GFQL schema (node_type/rel inline maps, toLower inline+timed); q2 orchestrated as two real GFQL calls; q5/q6/q7 as comma-form. Times pandas/cudf/polars/polars-gpu, values checked vs pandas. Verified on tiny data: pandas all q1-q9+q2; polars matches on q1/q2/q3/q4/q8/q9 (q5/q6/q7 NIE — comma-form connected-join not polars-native). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

Reduce real GFQL q1-q9 OLAP overhead with predicate pushdown, Polars-native binding fast paths, count-without-materialization paths, reusable graph-local caches, and compiled string-query plan caching. Add known-answer and cache-policy coverage for the optimized GFQL/Cypher paths and keep the benchmark runner lint-clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

graph_benchmark_q1_q9_real.py is archived verbatim in the private benchmark repo (ported/pygraphistry/ @52a22dd9 provenance); the changelog entry now states the scalar-fn semantics in Cypher-standard terms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myz4YLr6FhjyH7h7dHHTBV

lmeyerov · 2026-07-12T23:35:44Z

Competitor-mention hygiene (2026-07-12): benchmarks/gfql/graph_benchmark_q1_q9_real.py is removed from this PR — it lives verbatim in the private benchmark repo (ported/pygraphistry/, PROVENANCE-tracked). The changelog entry now states scalar-fn semantics in Cypher-standard terms. This PR is now mechanism-only with zero competitor references in the diff.

lmeyerov · 2026-07-15T09:42:40Z


    - name: Upload GFQL coverage audit
-      if: ${{ matrix.python-version == '3.12' }}
+      if: ${{ always() && matrix.python-version == '3.12' }}


lmeyerov · 2026-07-15T09:45:35Z

+        # (from the lazy `collect` NO-CHEATING contract), which we let propagate.
+        out_df = _lazy_collect(state)
+    except pl.exceptions.SchemaError:
+        return None


silent exn bad?

LazyFrame.schema / .columns resolve the schema AND emit a PerformanceWarning on every access; both ran per predicate per call on the seeded fast path (the SF1 profile showed the two warning sites firing on every query). Route polars schema reads through collect_schema via a local _schema_of, and give filter_by_dict a lazy-safe column probe. Zero PerformanceWarnings now; 200k/1M IS1 warm 12.1 -> 11.4ms. Deliberately avoids row_pipeline.py so this does not conflict with the pending #1714 OLAP rebase. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL

lmeyerov · 2026-07-20T07:50:21Z

Superseded by master — recommending close

Investigated this for the GFQL benchmark campaign (started the rebase onto master, then stopped once the evidence was clear):

Rebase commit 1 (5b875684, count(*) broadcast) conflicted in cypher/lowering.py — and master's side turned out to be a strict superset, already carrying both the GFQL/cypher: count(<non-active pattern alias>) NIEs at lowering with misleading 'one MATCH' error #1708 bare-alias-count clause and the Cypher/GFQL: admit multi-MATCH-source row projection without hidden carries (IC5 sub-residual B) #1273 clean-grouped-aggregate clause this commit adds.
Direction of the diff confirms it: git diff master..this-branch on the two hot files removes more than it adds — lowering.py −211/+84, row_pipeline.py −110/+30. Master is ahead, via feat(gfql): polars-native connected OPTIONAL MATCH (fixes pandas-ism crash + unseeded-arm dispatch + polars join twin) #1736/feat(gfql): polars whole-entity aggregation Cypher — HAS_label disambiguation + identity-key + key_prefixes (LDBC IC4 native) #1737.
The apparently-unique +1477 in gfql_unified.py was a false read on my part: master commit 4803adad extracted the fast-path helpers into graphistry/compute/gfql_fast_paths.py, so grepping gfql_unified.py alone missed them. Every fast path from this PR — _connected_join_two_star_fast_grouped_count, _execute_two_hop_count_fast_path, _connected_join_cached_node_filter, _execute_single_hop_grouped_aggregate_fast_path, _two_hop_count_binding_ops — is present on master today. This branch adds zero new product files.

Master also has 28 later commits touching gfql_unified.py that this branch predates (polars OPTIONAL MATCH pruning at 9.2× on IS7, the #1730/#1731 null-ordering and cache-staleness fixes).

The bench half of this PR is migrated in pyg-bench #90. Unless I've missed something you intended to keep here, this looks safe to close as superseded.

…peline.py Pure code move, no behaviour change. row_pipeline.py 1638 -> 1528 lines; the unbounded/variable-length arms live in polars/varlen_rows.py with a docstring explaining the exhaustion-depth walk and why deduping by endpoint is sound. Motivation is rebase surface: #1757, #1714 and the seeded-chain work all touch row_pipeline.py, and ~270 lines of var-length specialization sitting in the core flow made every one of those a conflict. RowPipelineMixin stays imported FUNCTION-LOCALLY exactly as at the original site — hoisting it to module scope reintroduces an import cycle (row.pipeline -> lazy engine -> back), so the move preserves it and says why. 2244 polars tests pass, ruff clean.

…ops) — unblocks LDBC IS6 on polars (#1709) (#1781) * feat(gfql): native polars unbounded directed var-length binding rows (LDBC IS6, #1709) The Cypher multi-alias bindings table (`rows(binding_ops=...)`) lowered natively for fixed-length and BOUNDED variable-length segments, but an unbounded fixed-point segment (`-[*]->` / `-[*0..]->`) declined with "polars engine does not yet natively support cypher row op 'rows'". That was the last shape blocking engine='polars' on LDBC SNB interactive-short-6, the one interactive-short query polars could not answer. Unbounded DIRECTED fixed point now lowers natively. Termination is data-dependent, so instead of pandas' expand-paths-until-empty (which materializes every partial path at every hop) the lowering runs a dedup-by-node frontier walk to find the exhaustion depth D, then reuses the SAME lazy bounded pair-join loop the `-[*1..k]->` arm uses with max_hops=D. Deduping by endpoint changes no walk's EXISTENCE, only its multiplicity, which the bounded loop then reproduces in full -- so the emitted rows are pandas-identical while the exponential path expansion happens once, lazily. A cycle reachable from the seed means infinitely many paths: that raises the same E108 "require terminating variable-length segments" error pandas raises, detected by a pigeonhole bound on the distinct nodes rather than after the blow-up. Also fixes a latent silent-wrong found by the differential fuzz: the polars builder rebuilt bindings from the CHAIN OUTPUT while pandas rebuilds from the pre-chain base graph. The traversal prunes to what IT considers matched, which is not the bindings builder's match set -- a zero-hop var-length segment binds a seed with no matching outgoing edge, and the traversal drops that node. So `-[*0..k]->` could silently return fewer rows than pandas. The builder now sources its node/edge tables from the same base graph pandas uses (and that the indexed builder was already handed). Honest declines unchanged in spirit (NIE, never a silent answer): undirected unbounded, aliased var-length relationships, and unbounded segments without to_fixed_point (pandas truncates at a bound this lowering cannot reconstruct). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NxxTFpKiqA4FQa9Pjy8yR1 * review: share the bounded var-length expansion; type the polars path bag lazily - extract `_directed_varlen_reachable_polars` so the unbounded fixed-point arm and the bounded `-[*1..k]->` arm run literally the same loop instead of two copies kept in sync by a comment. - pin the generic bindings builder's path bag as a LazyFrame. It always was one; mypy inferred eager because `filter_by_dict_polars` declared the eager type. - make `filter_by_dict_polars` frame-polymorphic via a constrained TypeVar (DataFrame | LazyFrame) rather than declaring one flavour and being called with both -- the eager viz lane and the lazy chain lane take the same `.filter(expr)` path, and the TypeVar keeps the caller's flavour on the way out. Removes the mypy noise this file was carrying, so the new code lands at delta 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NxxTFpKiqA4FQa9Pjy8yR1 * review: keep the localized ignores matching the new error code Widening filter_by_dict_polars to a constrained TypeVar changes the diagnostic at its two engine-neutral DataFrameT call sites from arg-type to type-var, so the existing localized ignore no longer applied. Retarget it and add the twin in gfql_fast_paths. Both sites are already gated on a polars engine; the cast is what the surrounding code has always asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NxxTFpKiqA4FQa9Pjy8yR1 * refactor(gfql): move the var-length row specializations out of row_pipeline.py Pure code move, no behaviour change. row_pipeline.py 1638 -> 1528 lines; the unbounded/variable-length arms live in polars/varlen_rows.py with a docstring explaining the exhaustion-depth walk and why deduping by endpoint is sound. Motivation is rebase surface: #1757, #1714 and the seeded-chain work all touch row_pipeline.py, and ~270 lines of var-length specialization sitting in the core flow made every one of those a conflict. RowPipelineMixin stays imported FUNCTION-LOCALLY exactly as at the original site — hoisting it to module scope reintroduces an import cycle (row.pipeline -> lazy engine -> back), so the move preserves it and says why. 2244 polars tests pass, ruff clean. * review(#1781): decline `-[*k..]->` for k>=2, and fix the coverage baseline Adversarial review of this branch found a silent-wrong regression and a red CI. BLOCKER — `-[*k..]->` with k >= 2 returned WRONG COUNTS, no error. The unbounded-directed arm of the `rows` gate had no `min_hops` guard. Cypher `-[*k..]->` lowers to `{hops: null, min_hops: k, to_fixed_point: true, direction: forward}`, so every k >= 2 was SERVED. Pandas' `step_pairs` come from the var-length `edge_op.execute` hop, which empties when its `max_reached_hop < min_hops` and otherwise drops edges labelled below min_hops; `max_reached_hop` is a dedup-by-node BFS eccentricity, not a longest-walk length, so the raw-edge reconstruction here expands a different edge multiset. On a 7-node acyclic graph, both engines answering: MATCH (a)-[*3..]->(b) RETURN count(*) pandas 0 polars 30 MATCH (a {id: 0})-[*2..]->(b) RETURN count(*) pandas 24 polars 41 85 of 1200 acyclic fuzz cases diverged, with zero declines. Master declines all of them. The second gate this PR's description relies on (`_is_native_multihop` in `_chain_traversal_polars`) never runs here: `RETURN count(*)` lowers to a pure-CALL chain, so the `rows` gate is the only gate. The existing decline test missed it because it pins `to_fixed_point=False` — a shape Cypher never emits. Now the unbounded arm requires resolved `min_hops <= 1`, mirroring the undirected arm; 0 and 1 (`-[*]->`, `-[*0..]->`, the IS6 walk) stay served. BLOCKER — CI was red and the coverage gate never ran. `varlen_rows.py` was missing from `coverage_baselines/ci-polars-py3.12.json`, whose own note requires a complete list, so `test-polars (3.12)` exited 3 and `changed-line-coverage` (which needs it) was SKIPPED — this branch's changed lines were never gated. Also fixed: * the cycle path raised a different exception CLASS and `.code` per engine (pandas GFQLTypeError/E303 via execute_call's wrapper, polars a raw GFQLValidationError/E108 because the native kernel runs before that wrapper). Repo control flow keys on `.code`. Now wrapped identically. * cycle detection was bounded by the GLOBAL node count with an eager collect per hop, so a two-node cycle reachable from one seed cost O(graph) collects (measured linear in global N) before raising. Now bounded by the REACHABLE set: a walk of h edges visits h+1 nodes, all within `seen`, so h >= |seen| IS a repeat. Exact in both directions, and the O(E) `node_cap` scan it replaces is gone entirely (`pairs_df.height == 0` is the same test). * the "hoisting reintroduces an import cycle" note was FALSE — disproven by hoisting. The sibling import moves to the import block and the mid-file `# noqa: E402` goes; `RowPipelineMixin` stays function-local, matching the engine convention. Unused typing imports and a stale line count removed. * `to_fixed_point` WITH an explicit bound was newly served and undocumented. It is parity-correct (pandas ignores the flag once max_hops is set) but was pinned by nothing; now tested and in the changelog. Tests, all mutation-checked (reverting each fix fails the matching test): * gate-level, k in 0..3, either side of the boundary * end-to-end through Cypher on the repro graph — the one that actually catches the blocker, since both engines answer and only values differ * the cycle test now asserts class and `.code` are ENGINE-INDEPENDENT rather than pinning polars' own code (the old form passed while they disagreed) * a long acyclic walk must not be mistaken for a cycle, and a small reachable cycle behind a large unreachable remainder must still raise — the two failure modes of the new bound Verified in-container on dgx-spark: 86 passed in the binding-rows file, 276 with the row-pipeline file, and `graphistry/tests/compute` with `--gpus all` gives an IDENTICAL 9-failure set to this branch's base (pre-existing dask/cuDF coercion), 6821 passed vs the base's 6804. mypy differential: 166 errors on both trees, so these changes add none. ruff clean under the repo config. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB * review(#1781): decline to_fixed_point with an explicit bound The re-review of the previous commit found that the gate rewrite which closed the unbounded `-[*k..]->` hole left a narrower one open, and that the test I added to pin it could not have caught it. Master declined on `bool(op.to_fixed_point)` ALONE. Restructuring the gate to key on the RESOLVED MAX incidentally let `to_fixed_point=True` combined with an explicit bound fall through to the bounded arm — where it hits the same reconstruction gap as the unbounded case, silently, for min_hops >= 3: fwd min=3 max=4 tfp=True pandas 0 polars 30 (master: NIE) fwd min=3 max=5 tfp=True pandas 0 polars 30 fwd min=4 max=5 tfp=True pandas 0 polars 6 fwd hops=3 tfp=True pandas 0 polars 24 reverse spellings: identical Not reachable from Cypher — `cypher/parser.py` sets to_fixed_point False for `*k` and `*i..k`, and only leaves it True for `*` and `*k..`, which always have max_hops None. So this is the AST / `rows(binding_ops=...)` wire surface only. Hard to reach is not correct, and the previous commit additionally claimed in the CHANGELOG that the shape "matches pandas", which is false. Declining it restores master's behaviour exactly, so it cannot regress anything that previously worked. WHY THE OLD TEST WAS BLIND, since the same mistake is easy to repeat: it compared flagged-vs-unflagged WITHIN each engine and its parametrization stopped at min_hops=2. A within-engine comparison never consults the pandas oracle, so it cannot see a divergence where BOTH engines answer; and min_hops <= 2 happens to agree on that fixture. It now asserts the DECLINE, runs to min_hops=4, and is joined by a value test on the divergence graph that compares ROW COUNTS against the oracle (engine-neutral: a bare `rows()` call emits engine-specific scaffolding columns on every shape, pre-existing and unrelated). Mutation-checked: removing the guard fails 9 tests, including the value test — the property the previous version lacked. Verified in-container on dgx-spark: binding-rows file 90 passed; `graphistry/tests/compute` with `--gpus all` gives 6825 passed and a failure set byte-identical to this branch's base (the 9 pre-existing dask/cuDF coercion failures). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

lmeyerov · 2026-07-30T02:01:52Z

Superseded by the #1758–#1763 stack — recommending close (2026-07-29)

Triaged while sweeping the backlog for anything that moves a benchmark cell. Three findings:

The base branch already merged. This PR targets perf/gfql-row-pushdown, not master — that was perf(gfql/cypher): single-alias predicate pushdown in the row pipeline (L4) #1702, merged 2026-07-16. The MERGEABLE status is against a merged base and does not mean it can land on master.
14 of the 16 new lowering.py symbols are already on master. _apply_connected_join_node_filter, _binding_prop_alias_set, _clause_has_mixed_aggregate_item, _connected_join_expr_literal_value, _connected_join_expr_property_ref, _connected_join_literal_op, _connected_join_required_property_aliases, _expr_has_aggregate, _expr_property_access_node_aliases, _is_pure_aggregate_call, _projection_ref_from_expr_safe, _pushdown_connected_join_atom_filter, _pushdown_connected_join_where_filters all exist on 3f7f79236. Master's lowering.py is 9260 lines vs 8508 at this PR's base — master moved past this contribution via the abstraction-tax stack. Only _apply_connected_join_node_predicate, _connected_join_lower_literal_value, _connected_join_lower_property_ref are absent, and they look like pre-rename variants of the landed versions.
It edits .github/workflows/ci.yml, which I cannot author.

The OLAP lowering this PR set out to deliver is on master and measured (7W/1T/1L @20k, 6W/2T/1L @100k). Rebasing +4031/-59 across 21 files onto a master that already contains the work is strictly worse than re-deriving whatever residue is left.

Proposed: confirm those 3 residual symbols are renames of the landed ones, then close this and file a narrow issue for anything genuinely missing. Not closing unilaterally — flagging for the owner.

lmeyerov · 2026-07-30T02:08:09Z

Follow-up resolved — nothing to file, no residual gap (2026-07-29)

My previous comment left two TODOs, and they were mine. Both are now discharged.

What the 3 residual symbols actually did. _connected_join_lower_property_ref, _connected_join_lower_literal_value and _apply_connected_join_node_predicate were toLower()/lower() pushdown: recognize toLower(n.prop) = 'lit' on either side of a connected-join comparison and push the predicate into that node's filter_dict before the join.

Master covers this, by a different and better mechanism. Master has no tolower in lowering.py because the case is handled earlier and more generally:

graphistry/compute/gfql/expr_const_fold.py const-folds it — toLower(i.interest) = toLower('Fine Dining') and ... = 'fine dining' become the same expression, with the ASCII-block semantics of String.toLowerCase and explicit toLower(null) handling.
graphistry/compute/gfql_fast_paths.py:597 then serves those as leading where_rows ops, i.e. still before the join, with :1084 applying residual node filters to the materialized nodes.
Both row engines implement the function itself (row/pipeline.py:1523, lazy/engine/polars/row_pipeline.py:205), including the lower/upper GQL aliases Neo4j also accepts.

That is strictly more than this PR did: folding removes the literal-side call entirely, so nothing at runtime evaluates toLower on a constant, and the fold is shared by every surface rather than living in one lowering path.

So: no follow-up issue. There is no functionality in this PR that master lacks. Closing the loop rather than leaving a dangling TODO on a closed PR.

lmeyerov force-pushed the perf/gfql-olap-real-q1q9 branch 3 times, most recently from dc01f38 to c7b5c71 Compare July 8, 2026 08:55

lmeyerov force-pushed the perf/gfql-row-pushdown branch from ed6e12c to 0a2e00a Compare July 8, 2026 08:55

lmeyerov force-pushed the perf/gfql-olap-real-q1q9 branch from c7b5c71 to ea3fcd9 Compare July 8, 2026 09:01

lmeyerov marked this pull request as ready for review July 8, 2026 09:58

lmeyerov force-pushed the perf/gfql-row-pushdown branch from e2d693e to d23a5f4 Compare July 9, 2026 01:03

lmeyerov force-pushed the perf/gfql-olap-real-q1q9 branch from ea3fcd9 to 36cf2ff Compare July 9, 2026 01:05

lmeyerov force-pushed the perf/gfql-row-pushdown branch from d23a5f4 to 75193ba Compare July 9, 2026 01:47

lmeyerov force-pushed the perf/gfql-olap-real-q1q9 branch from 36cf2ff to cc531c4 Compare July 9, 2026 01:47

lmeyerov force-pushed the perf/gfql-row-pushdown branch from 75193ba to c91b86d Compare July 9, 2026 02:08

lmeyerov force-pushed the perf/gfql-olap-real-q1q9 branch from cc531c4 to 180ad49 Compare July 9, 2026 02:09

lmeyerov force-pushed the perf/gfql-row-pushdown branch from c91b86d to 5c2afb9 Compare July 9, 2026 02:41

lmeyerov force-pushed the perf/gfql-olap-real-q1q9 branch from 180ad49 to f7cc095 Compare July 9, 2026 02:42

lmeyerov force-pushed the perf/gfql-row-pushdown branch from 5c2afb9 to a40b01c Compare July 9, 2026 02:50

lmeyerov force-pushed the perf/gfql-olap-real-q1q9 branch from f7cc095 to b09397f Compare July 9, 2026 02:50

lmeyerov force-pushed the perf/gfql-row-pushdown branch from a40b01c to 1657638 Compare July 9, 2026 04:03

lmeyerov force-pushed the perf/gfql-olap-real-q1q9 branch from b09397f to 321fe95 Compare July 9, 2026 04:07

lmeyerov and others added 12 commits July 9, 2026 10:16

docs(changelog): #1709 polars binding_ops MVP entry

67ebb8e

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

docs(changelog): fold var-length binding rows into the #1709 entry

9756a69

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rRBmezaBEi2CJgDGkMNgR

lmeyerov and others added 12 commits July 9, 2026 10:16

test(gfql): cover OLAP lowering helper paths

2a7cac2

test(gfql): cover conservative OLAP lowering branches

99b4aca

ci(gfql): expose coverage audit baseline failures

cf318ba

ci(gfql): exclude polars fast paths from pandas coverage

2a8a875

test(gfql): cover pandas aggregate fast-path branches

448adf3

ci(gfql): exclude singleton polars count branch

778eb25

fix(gfql): route polars single-node binding rows

fdca99c

test(gfql): cover polars binding rows in CI

2d17da5

ci(gfql): refresh cypher surface guard after residual stack

bac2196

chore(gfql): tighten olap lowering typing

adee130

lmeyerov force-pushed the perf/gfql-olap-real-q1q9 branch from 321fe95 to adee130 Compare July 9, 2026 17:25

lmeyerov commented Jul 15, 2026

View reviewed changes

This was referenced Jul 16, 2026

feat(gfql): add native Polars OLAP foundation #1728

Merged

feat(gfql): plan connected join predicate pushdown #1729

Merged

feat(gfql): execute connected join fast paths #1730

Merged

feat(gfql): add aggregate fast paths and final hardening #1731

Merged

lmeyerov closed this Jul 30, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

perf(gfql): real q1-q9 OLAP lowering on Polars - #1714

perf(gfql): real q1-q9 OLAP lowering on Polars#1714
lmeyerov wants to merge 25 commits into
perf/gfql-row-pushdownfrom
perf/gfql-olap-real-q1q9

lmeyerov commented Jul 8, 2026

Uh oh!

lmeyerov commented Jul 12, 2026

Uh oh!

lmeyerov Jul 15, 2026

Uh oh!

lmeyerov Jul 15, 2026

Uh oh!

lmeyerov commented Jul 20, 2026

Uh oh!

lmeyerov commented Jul 30, 2026

Uh oh!

lmeyerov commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

lmeyerov commented Jul 8, 2026

Summary

Validation

DGX pressure evidence

Notes

Uh oh!

lmeyerov commented Jul 12, 2026

Uh oh!

lmeyerov Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

lmeyerov Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

lmeyerov commented Jul 20, 2026

Superseded by master — recommending close

Uh oh!

lmeyerov commented Jul 30, 2026

Superseded by the #1758–#1763 stack — recommending close (2026-07-29)

Uh oh!

lmeyerov commented Jul 30, 2026

Follow-up resolved — nothing to file, no residual gap (2026-07-29)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant