Skip to content

Commit 4dd0daa

Browse files
lmeyerovclaude
andauthored
feat(gfql): native polars whole-entity count(DISTINCT n) / identity aggregation (#1709) (#1750)
* feat(gfql): native polars whole-entity count(DISTINCT n) via __gfql_node_id__ (#1709) Resolve the bare `__gfql_node_id__` identity sentinel (emitted by whole-entity aggregation lowering, e.g. RETURN count(DISTINCT b)) to the graph node-id column in the polars row-expression lowering, mirroring pandas _gfql_resolve_token. The prefixed `alias.__gfql_node_id__` form was already handled by _resolve_property; only the single-source bare form declined (NIE on 'with_'). Published via a _NODE_ID contextvar alongside _SCHEMA; declines (NIE) when the id column is unknown or absent so a multi-alias binding table never resolves to a wrong/absent column. Differential fuzz vs pandas oracle: 600/600 value-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * docs(gfql): CHANGELOG [Development] entry for #1709 (native count(DISTINCT n)) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * refactor(gfql): dedupe node-id sentinel + move polars lowering contextvars to a registry (review) Review feedback on #1750: _NODE_ID_TOKEN was a bare literal duplicating the canonical same_path_types.NODE_IDENTITY_COLUMN — now imports it. The polars-lowering contextvars (_SCHEMA/_NODE_ID) move to a new per-engine registry lowering_context.py (the contextvar analogue of reserved_columns.py), answering "per-engine or overall?" -> per-engine, since they thread polars-specific lowering state. Registered lowering_context.py in the polars per-file coverage baseline; dropped the now-unused `import contextvars`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * refactor(gfql): type _lower_with_schema (table: pl.DataFrame, fn generic) (review) Review feedback on #1750: _lower_with_schema(table: Any, fn) -> untyped. Now table: pl.DataFrame, fn: Callable[[], _LowerT] -> _LowerT (the lowering result flows through). mypy: 0 new errors; ruff clean; 175 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * fix(gfql): drop dead scalar-normalization branch in order_by_polars lower_order_by_keys always returns List[bool] for descending flags, so the isinstance(descending, list) ternary was statically dead and tripped mypy [list-item] under py3.8 (List[bool] where bool expected). Use descending directly for per-key nulls_last; behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 983d2af commit 4dd0daa

5 files changed

Lines changed: 191 additions & 19 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
1414
### Fixed
1515
- **GFQL polars engine natively runs `UNWIND` of a carried `collect()` list column**: `... WITH collect(x) AS xs UNWIND xs AS y RETURN ...` (exploding a list-valued column produced by `collect()`, the row-pipeline analogue of the IC6 unwind shape) previously declined on polars — `unwind_polars` only accepted a scalar literal list. Now explodes a `List`-dtype column reference (`with_columns` copy → `filter(list.len() > 0)` → `explode`), matching the pandas oracle exactly: empty-list/null cells → 0 rows, nulls *within* a list survive, source column retained; the pre-filter also makes it independent of the polars-2.0 `empty_as_null` default. Non-list columns, name collisions, unknown identifiers, and nested-list literals still decline (honest NIE, no pandas bridge). Differential fuzz vs pandas over ~3500 collect→UNWIND→{RETURN, grouped, filtered, ORDER BY, aggregate, nested-unwind} queries: 0 disagreements. (The `MATCH ... WITH collect(..) UNWIND .. MATCH` form — IC6 with a trailing re-traversal — is compile-rewritten into WITH→MATCH reentry, a separate path.) Tests in `test_engine_polars_row_pipeline.py`.
1616
- **GFQL polars engine natively runs the LDBC IC11/IC6 undirected variable-length bindings table (`-[*1..k]-`), unblocking the cross-alias `WHERE NOT(person=friend)` clause**: a bounded UNDIRECTED variable-length MATCH that materializes a bindings row table (`rows(binding_ops=...)`, emitted whenever a downstream clause — e.g. a cross-alias same-path `WHERE NOT a = b` / `a <> b` — references two node aliases on the path) previously declined on polars (`binding_rows_polars` hard-NIE'd every undirected multihop), while pandas handled it. This was the single residual blocking the official LDBC IC11/IC6 queries on polars (the cross-alias WHERE lowering itself was already native — directed var-length + `WHERE NOT a = b` already matched pandas). `binding_rows_polars` now supports undirected var-length with **`min_hops == 1`** via a doubled-pair join with immediate-backtrack avoidance, an exact port of the pandas oracle `_gfql_multihop_binding_rows` (`avoid_immediate_backtrack=True`): a `__prev__` marker (seeded null, dtype-matched to the id column) drops immediate backtracks each hop (Kleene mask: null prev kept), and the step-pair set reproduces pandas' edge multiplicity exactly — each non-loop edge contributes each directed orientation TWICE (so a length-1 pair appears x2, length-2 x4) while self-loops contribute `(u,u)` x2 only (not double-counted). The multiplicity rule was derived by instrumenting pandas' `step_pairs` (which flow from the var-length `edge_op.execute` hop + `orient_edges`), NOT by reading code alone. Scoped to `min_hops == 1` because that is where the raw-edge reconstruction provably matches pandas: every edge is trivially a length-1 path so the var-length hop's backward pruning removes nothing; `min_hops == 0` (zero-hop, undoubled) and `min_hops >= 2` (backward-pruned / long-walk divergence) still **decline with an honest `NotImplementedError`** rather than risk silent-wrong multiplicities. Differential fuzz vs the pandas oracle over ~2500 random graphs (self-loops, parallel + antiparallel edges, `*1..2`…`*1..5`, all WHERE/RETURN variants): 0 disagreements; the out-of-scope windows decline (never diverge). Tests in `test_engine_polars_binding_rows.py` (parity + multiplicity/backtrack/self-loop/string-id pins + `*0..2`/`*2..3`/`*2..2` decline pins).
17+
- **GFQL polars engine natively runs whole-entity `count(DISTINCT n)` / identity aggregation (#1709)**: `g.gfql("MATCH (a {..}) RETURN count(DISTINCT a) AS c")` and its grouped/filtered/traversal variants (`(a)-[]->(b) RETURN count(DISTINCT b)`) previously declined on polars — the Cypher aggregation lowers `count(DISTINCT b)` to the `__gfql_node_id__` identity sentinel, and the polars `lower_expr` `Identifier` branch resolved only the prefixed `alias.__gfql_node_id__` form, not the bare sentinel, so it NIE'd. Fix threads the graph node-id column through a `_NODE_ID` contextvar (published alongside the schema in `_lower_with_schema`, wired through the four callers) so the bare sentinel resolves to the id column **only if present**, else declines (never invents/mis-resolves a column). Differential fuzz vs the pandas oracle: 600 + 300 cases, 0 disagreements. Deliberately still honest-NIE (verified declines, not silent-wrong): whole-entity `collect(b)` (list-of-entities repr not ported, cf #1650) and multi-alias binding-table identity aggregation (bare id absent on connected-pattern binding tables — adjacent to #1273). Tests in `test_engine_polars_row_pipeline.py`.
1718
- **GFQL polars engine natively runs multi-source (node-cartesian) MATCH (#1273)**: comma-separated disconnected aliases — `g.gfql("MATCH (a {..}), (b {..}) RETURN a.id, b.id")` — previously declined on polars (the `rows(binding_ops)` `node_cartesian` branch hard-NIE'd) while pandas handled them. New `_cartesian_node_bindings_polars` mirrors the pandas oracle `_gfql_cartesian_node_bindings_row_table`: each alias is independently filtered, projected into the per-alias lookup schema (bare `alias` id, `alias.id`, `alias.<prop>`, and pandas' leaked `alias.alias=True` flag incl. property-shadowing), then left-major cross-joined for row-order parity. Scalar/aliased/`count(*)` projections over the cartesian match pandas exactly (verified end-to-end); whole-entity `RETURN a, b` stays an honest NIE (separate projection surface). Two parity-safe declines mirror shapes where pandas itself errors (proven on master, so both engines fail identically, never diverge): an anonymous node op, and ≥4 named aliases (pandas' bare-id merge residue collides). Differential fuzz vs pandas over ~10k lowered cases: 0 disagreements. Tests in `test_engine_polars_binding_rows.py`.
1819
- **GFQL polars chain — variable-length node aliases are hop-distance gated; the chain is now silent-wrong-free (#1741)**: a node named after a variable-length edge (`g.gfql("MATCH (a)-[*1..2]-(b) RETURN b")`) previously carried its alias regardless of hop distance, so an undirected walk that backtracked into the seed wrongly returned it (pandas correctly excludes it — trail semantics). The polars chain now auto-injects the #1741 hop-distance label (name resolved against the user's node columns via the `reserved_columns` registry, so a user column named like the internal one is never clobbered) and applies pandas' alias `[min_hop, max_hop]` window in `_apply_node_names`. The one shape the gate can't yet cover — a node alias after a **forward/reverse `min_hops>1`** edge, whose labels need pandas' layered backward walk (not ported) — now **declines with an honest `NotImplementedError` (#1748)** instead of returning nodes outside the window; the decline is precise (differential vs pandas: 30/30 named shapes NIE, 30/30 unnamed run and match, 0 over-decline). 4-engine A/B on the stacked build: pandas/cudf 144/144, polars/polars-gpu 112 agree / **0 disagree** / 32 honest-NIE — zero silent-wrong shapes remain on the polars chain. Adapts the mechanism from the retired #1742 (declined undirected varlen+alias, now natively gated). Tests: `TestVarlenAliasHopGate` (pandas-parity across directions, the `*2..3` decline + unnamed-still-runs pins, a column-collision test).
1920
- **GFQL native polars `label_node_hops` on the plain BFS — correct, direction-dependent hop labels (#1741 groundwork)**: the polars eager hop now emits pandas-parity node hop-distance labels instead of declining `label_node_hops`. The labeling rule is DIRECTION-DEPENDENT (the subtle part, derived by differential fuzz vs the pandas oracle, not by reading pandas alone): forward/reverse label EVERY destination of a hop first-wins (matching pandas `hop.py` `new_node_ids`), so a seed re-entered at hop 1 IS labeled; undirected labels destinations MINUS everything already visited and pre-seeds the seen-set with the seeds, so a seed re-reached by a backtracking walk stays NULL (its shortest-path distance). Two correctness fixes over the first cut: (1) the undirected seed pre-seed must NOT depend on `return_as_wave_front` — a seed filtered out of the frontier by `source_node_match` or suppressed in wave-front mode was being re-labeled (A/B vs pandas over direction × hops × `return_as_wave_front` × `source_node_match` × `destination_node_match` × 10 graphs: was 456/24, now 480/480); (2) a requested `label_node_hops` name that collides with an existing column is redirected to `<name>_1` like pandas' `resolve_label_col`, not the polars left-join auto-suffix `<name>_right`/`DuplicateError`. `label_edge_hops` and `min_hops>1` labels stay honest NIEs. Amplified `test_engine_polars_hop.py` (`TestHopLabelsDifferential` over the seed-filter / wave-front / multi-seed axes, an explicit direction-asymmetry contrast pin, and a collision test).
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
"""Per-engine lowering context for the native polars GFQL engine.
2+
3+
Contextvars threaded around a polars row-op lowering pass so the pure expr-lowering helpers
4+
(``lower_expr`` and friends) can read ambient state without plumbing it through every signature.
5+
Set + reset them as a pair around a lowering call — see ``_lower_with_schema`` in ``row_pipeline.py``.
6+
7+
Declared together in one place per engine (the contextvar analogue of ``reserved_columns.py``) so
8+
the lowering's ambient state is discoverable rather than inlined across the executor. New
9+
polars-lowering contextvars SHOULD be added here.
10+
"""
11+
import contextvars
12+
from typing import Optional
13+
14+
#: Table schema (column name -> polars dtype) of the frame being lowered — lets float-operand
15+
#: inference run for the NaN guard without a scan.
16+
SCHEMA: "contextvars.ContextVar[dict]" = contextvars.ContextVar("gfql_polars_schema", default={})
17+
18+
#: Active graph node-id column name — lets ``lower_expr`` resolve the bare whole-entity identity
19+
#: sentinel (``same_path_types.NODE_IDENTITY_COLUMN``) to the real id column. None when unknown, so
20+
#: the identity sentinel then declines to an honest NIE rather than resolving to a wrong column.
21+
NODE_ID: "contextvars.ContextVar[Optional[str]]" = contextvars.ContextVar("gfql_polars_node_id", default=None)

graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,9 @@
1313
"""
1414
from __future__ import annotations
1515

16-
import contextvars
1716
import operator
1817
import re
19-
from typing import TYPE_CHECKING, Any, Callable, Dict, List, Mapping, Optional, Sequence, Tuple, Union, cast
18+
from typing import TYPE_CHECKING, Any, Callable, Dict, List, Mapping, Optional, Sequence, Tuple, TypeVar, Union, cast
2019

2120
if TYPE_CHECKING:
2221
import polars as pl
@@ -33,8 +32,11 @@
3332

3433

3534
# Active row-table schema (col -> dtype), set around lowering so lower_expr can infer FLOAT
36-
# operands for the NaN guard. Free to populate — schema is already on the table, no scan.
37-
_SCHEMA: "contextvars.ContextVar[dict]" = contextvars.ContextVar("gfql_polars_schema", default={})
35+
# operands for the NaN guard. Lowering contextvars live in the per-engine `lowering_context`
36+
# registry (aliased here to keep call sites terse); the whole-entity identity sentinel is the
37+
# shared cypher-lowering constant, NOT a local literal.
38+
from .lowering_context import SCHEMA as _SCHEMA, NODE_ID as _NODE_ID
39+
from graphistry.compute.gfql.same_path_types import NODE_IDENTITY_COLUMN as _NODE_ID_TOKEN
3840

3941
# Ops needing the NaN guard: polars treats NaN as the LARGEST value (>/>=/== TRUE), but
4042
# IEEE/Python/pandas/Cypher compare NaN as FALSE (!= TRUE; Neo4j TCK agrees). Float operands
@@ -88,7 +90,7 @@ def _resolve_property(alias: str, prop: str, columns: Sequence[str]) -> Optional
8890
prefixed = f"{alias}.{prop}"
8991
if prefixed in columns:
9092
return prefixed
91-
if prop == "__gfql_node_id__" and alias in columns:
93+
if prop == _NODE_ID_TOKEN and alias in columns:
9294
# Whole-entity identity key (#1650 lowering groups by `alias.__gfql_node_id__`).
9395
# pandas' bindings table carries it as a join-residue column; the polars table
9496
# deliberately doesn't — its value IS the bare alias id column.
@@ -473,7 +475,16 @@ def lower_expr(node: ExprNode, columns: Sequence[str]) -> Optional[pl.Expr]:
473475
if isinstance(node, ListLiteral):
474476
return _lower_list_literal(node.items, columns)
475477
if isinstance(node, Identifier):
476-
return pl.col(node.name) if node.name in columns else None
478+
if node.name in columns:
479+
return pl.col(node.name)
480+
# Bare whole-entity identity sentinel -> the graph node-id column (pandas
481+
# _gfql_resolve_token bare form). Only when the id column is actually present;
482+
# otherwise decline (None -> NIE) rather than invent a column.
483+
if node.name == _NODE_ID_TOKEN:
484+
node_id = _NODE_ID.get()
485+
if node_id is not None and node_id in columns:
486+
return pl.col(node_id)
487+
return None
477488
if isinstance(node, PropertyAccessExpr):
478489
if isinstance(node.value, Identifier):
479490
src = _resolve_property(node.value.name, node.property, columns)
@@ -625,14 +636,21 @@ def _rewrap(g: Plottable, table_df: Any) -> Plottable:
625636
return frame_ops.row_table(_RowPipelineAdapter(g), table_df)
626637

627638

628-
def _lower_with_schema(table: Any, fn):
639+
_LowerT = TypeVar("_LowerT")
640+
641+
642+
def _lower_with_schema(table: "pl.DataFrame", fn: Callable[[], _LowerT],
643+
node_id: Optional[str] = None) -> _LowerT:
629644
"""Run a lowering callable with the table schema published to ``_SCHEMA`` (float-operand
630-
inference for the NaN guard)."""
631-
token = _SCHEMA.set(dict(table.schema))
645+
inference for the NaN guard) and the graph node-id column published to ``_NODE_ID`` (bare
646+
``__gfql_node_id__`` identity-sentinel resolution)."""
647+
schema_token = _SCHEMA.set(dict(table.schema))
648+
node_id_token = _NODE_ID.set(node_id)
632649
try:
633650
return fn()
634651
finally:
635-
_SCHEMA.reset(token)
652+
_SCHEMA.reset(schema_token)
653+
_NODE_ID.reset(node_id_token)
636654

637655

638656
def _project_preserving_height(table: Any, exprs: List[Any]) -> Any:
@@ -652,7 +670,7 @@ def _project_polars(g: Plottable, items: Sequence[SelectItem], extend: bool) ->
652670
"""Shared body of ``select_polars`` / ``with_columns_polars``; None if any item isn't
653671
lowerable (honest NIE, no pandas bridge)."""
654672
table = _active_table(g)
655-
exprs = _lower_with_schema(table, lambda: lower_select_items(items, list(table.columns)))
673+
exprs = _lower_with_schema(table, lambda: lower_select_items(items, list(table.columns)), node_id=g._node)
656674
if exprs is None:
657675
return None
658676
out = table.with_columns(exprs) if extend else _project_preserving_height(table, exprs)
@@ -714,7 +732,7 @@ def where_rows_polars(
714732
if expr is not None:
715733
if not isinstance(expr, str):
716734
return None
717-
lowered = _lower_with_schema(table, lambda: lower_expr_str(expr, columns))
735+
lowered = _lower_with_schema(table, lambda: lower_expr_str(expr, columns), node_id=g._node)
718736
if lowered is None:
719737
return None
720738
preds.append(lowered)
@@ -729,17 +747,16 @@ def where_rows_polars(
729747
def order_by_polars(g: Plottable, keys: Sequence[OrderKey]) -> Optional[Plottable]:
730748
"""Native polars sort; None if any key isn't lowerable."""
731749
table = _active_table(g)
732-
lowered = _lower_with_schema(table, lambda: lower_order_by_keys(keys, list(table.columns)))
750+
lowered = _lower_with_schema(table, lambda: lower_order_by_keys(keys, list(table.columns)), node_id=g._node)
733751
if lowered is None:
734752
return None
735753
exprs, descending = lowered
736754
# openCypher orders NULL as the LARGEST value: ASC -> nulls last, DESC -> nulls FIRST.
737755
# (Previously hardcoded nulls_last=True, which mis-ordered DESC keys and silently returned
738-
# the wrong `... DESC LIMIT k` top-k over a column containing NULLs.) Normalize `descending`
739-
# to a per-key list (it may arrive as a scalar bool) so `nulls_last` mirrors it per key.
740-
descending_list = descending if isinstance(descending, list) else [descending] * len(exprs)
741-
nulls_last = [not d for d in descending_list]
742-
return _rewrap(g, table.sort(exprs, descending=descending_list, nulls_last=nulls_last))
756+
# the wrong `... DESC LIMIT k` top-k over a column containing NULLs.) `descending` is one
757+
# bool per key (see lower_order_by_keys), so `nulls_last` mirrors it per key.
758+
nulls_last = [not d for d in descending]
759+
return _rewrap(g, table.sort(exprs, descending=descending, nulls_last=nulls_last))
743760

744761

745762
# Native aggs: count/sum/avg/min/max/count_distinct/collect/collect_distinct; stdev/percentile
@@ -887,7 +904,7 @@ def select_extend_polars(g: Plottable, items: Sequence[SelectItem]) -> Optional[
887904
the bindings-path aggregate lowering (pre-aggregation group keys / agg args),
888905
so it is required for binding-row queries (#1709). None → NIE."""
889906
table = _active_table(g)
890-
exprs = _lower_with_schema(table, lambda: lower_select_items(items, list(table.columns)))
907+
exprs = _lower_with_schema(table, lambda: lower_select_items(items, list(table.columns)), node_id=g._node)
891908
if exprs is None:
892909
return None
893910
out = table.with_columns(exprs)

graphistry/tests/compute/gfql/coverage_baselines/ci-polars-py3.12.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
"graphistry/compute/gfql/lazy/engine/polars/dtypes.py": 92.0,
1010
"graphistry/compute/gfql/lazy/engine/polars/hop.py": 87.0,
1111
"graphistry/compute/gfql/lazy/engine/polars/hop_eager.py": 90.0,
12+
"graphistry/compute/gfql/lazy/engine/polars/lowering_context.py": 90.0,
1213
"graphistry/compute/gfql/lazy/engine/polars/nan_clean.py": 90.0,
1314
"graphistry/compute/gfql/lazy/engine/polars/pattern_apply.py": 67.0,
1415
"graphistry/compute/gfql/lazy/engine/polars/predicates.py": 83.0,

0 commit comments

Comments
 (0)