feat(gfql): native polars whole-entity count(DISTINCT n) / identity aggregation (#1709)#1750
Merged
Merged
Conversation
This was referenced Jul 20, 2026
Merged
lmeyerov
commented
Jul 20, 2026
| # reductions (e.g. ``RETURN count(DISTINCT b)``); it must resolve to the graph's node-id | ||
| # column, exactly like pandas ``_gfql_resolve_token``. (The ``alias.__gfql_node_id__`` prefixed | ||
| # form is already handled by ``_resolve_property``.) | ||
| _NODE_ID_TOKEN = "__gfql_node_id__" |
lmeyerov
commented
Jul 20, 2026
| # Active graph node-id column name, published around lowering so ``lower_expr`` can resolve the | ||
| # bare identity sentinel above to the real id column. None when unknown (identity sentinel then | ||
| # declines -> honest NIE, never a wrong column). | ||
| _NODE_ID: "contextvars.ContextVar[Optional[str]]" = contextvars.ContextVar("gfql_polars_node_id", default=None) |
lmeyerov
added a commit
that referenced
this pull request
Jul 20, 2026
…tvars 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
lmeyerov
commented
Jul 20, 2026
|
|
||
|
|
||
| def _lower_with_schema(table: Any, fn): | ||
| def _lower_with_schema(table: Any, fn, node_id: Optional[str] = None): |
…ode_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
…TINCT n)) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
…tvars 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
…ric) (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
lmeyerov
force-pushed
the
feat/gfql-1709-native-polars-rows-agg
branch
from
July 20, 2026 23:45
27b0f0f to
4968f45
Compare
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
g.gfql("MATCH (a {..}) RETURN count(DISTINCT a) AS c")and its grouped / WHERE-filtered / traversal variants ((a)-[]->(b) RETURN count(DISTINCT b)) previously declined on the polars engine. The Cypher aggregation lowerscount(DISTINCT b)to the__gfql_node_id__identity sentinel; the polarslower_exprIdentifierbranch resolved only the prefixedalias.__gfql_node_id__form, not the bare sentinel, so it NIE'd. Fix threads the graph node-id column through a_NODE_IDcontextvar (published alongside the schema in_lower_with_schema, wired through its four callers) so the bare sentinel resolves to the id column only if present, else declines — never invents or mis-resolves a column.Verification (independent — re-run by the reviewing agent, not just the author)
End-to-end
g.gfql()A/B vs the pandas oracle, all match:RETURN count(DISTINCT a)(single-source) ·(a)-[]->(b) RETURN count(DISTINCT b)(traversal) ·RETURN a.kind, count(DISTINCT a)(grouped)test_engine_polars_row_pipeline.py: 175 passed (independently re-run).ruff: clean. mypy: master baseline, 0 new.Scope / still honest-NIE (verified declines, not silent-wrong)
collect(b)— list-of-entities repr not ported (cf GFQL: avoid spurious entity-text stringification of returned entities (return structured/Arrow frames) #1650).Independent lever off master (touches
row_pipeline.py, a different function than #1749's cartesian). Refs #1709.🤖 Generated with Claude Code