Skip to content

fix(gfql): preserve bag multiplicity for whole-entity projections - #2000

Merged
lmeyerov merged 8 commits into
masterfrom
fix/gfql-1994-whole-entity-bag
Aug 27, 2026
Merged

fix(gfql): preserve bag multiplicity for whole-entity projections#2000
lmeyerov merged 8 commits into
masterfrom
fix/gfql-1994-whole-entity-bag

Conversation

@lmeyerov

@lmeyerov lmeyerov commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • preserve openCypher bag multiplicity for fixed-length, non-DISTINCT whole-node projections
  • render single- and multi-node entity projections from Polars binding rows
  • keep DISTINCT, variable-length paths, relationship-free matches, and WITH-to-MATCH reentry on their established lanes
  • keep the seeded endpoint fast path aligned with the generic bag result

Correctness oracle

For nodes 1-5 and edges (1,2) (1,3) (2,3) (3,4), MATCH (a)-->(b) RETURN b now returns b.id = [2,3,3,4], including node 3 once per relationship match. Parallel-edge and two-hop fixtures discriminate a bag from a node set. Cross-engine pins cover pandas, Polars, and cuDF; the final DGX canary covers explicit Polars-GPU.

Validation

  • rebased and linearized onto landed fix(gfql): a NULL id is not an identity — one NULL-endpoint contract on every engine (#1995) #1999/master; exact head 696327559297d8ddcfc65e3955ba2a6e4d8e0404
  • focused pandas/Polars CPU receipt: 1,935 passed, 32 skipped, 159 cuDF-selected cells deselected
  • first local run additionally passed 2,079 cells; its only 7 failures were cuDF parameters blocked by absent local libnvrtc.so.12
  • ./bin/lint.sh: clean (Ruff, type-hygiene, comment-density, relative-import gates)
  • ./bin/typecheck.sh: 336 source files clean
  • pre-rebase guarded DGX canary covered cuDF/NVRTC/Polars-GPU; the rebase was content-preserving and the review fix is comment-only
  • authoritative post-rebase status is the GitHub Checks panel

Review resolution

  1. EndpointFrameT stays: it preserves the caller's pandas/cuDF/Polars frame flavor across the runtime dispatcher; DataFrameT intentionally models pandas during type checking. Commit 696327559 adds the guard-compliant one-line explanation.
  2. Keep fixed-length, non-DISTINCT whole-node bag semantics. Current Cypher documents plain RETURN and GQL-conformant RETURN ALL as equivalent and duplicate-preserving; DISTINCT performs deduplication.

Reviewer handoff

Exact state: rebased linear head 696327559297d8ddcfc65e3955ba2a6e4d8e0404 on master@190b802ba867081f253f21e5c22679de59f7e8f4.

Reviewer decisions:

  1. Confirm fixed-length, non-DISTINCT whole-node results use binding-row multiplicity, while DISTINCT, variable-length, reentry, and relationship-free shapes stay excluded.
  2. Review _AliasView field mapping for prefixed multi-entity Polars rows, including OPTIONAL-null entity markers and structured versus text rendering.
  3. Confirm the seeded typed-hop fast path preserves the same bag and correctly declines the zero-row dtype corner to the full path.
  4. Confirm the controls prove property projections and existing non-bag lanes did not move.

Safe local reproduction:

python -m pytest -q -k 'not cudf' graphistry/tests/compute/gfql/test_whole_entity_projection_bag_1994.py graphistry/tests/compute/gfql/cypher/test_lowering.py graphistry/tests/compute/gfql/test_alias_scoping_semantics.py graphistry/tests/compute/gfql/test_engine_polars_row_pipeline.py graphistry/tests/compute/gfql/test_hop_semantics_pins.py graphistry/tests/compute/gfql/test_row_multiplicity_semantics.py
./bin/lint.sh
./bin/typecheck.sh

Stack / landing

Position: #1999 is landed; #2000 is rebased directly on master. After #2000 lands, rebase successor #2001 onto fresh master before its own checks and merge.

Landing rule: merge commit only, exact-head guard, every check settled green, then ancestry verification. Do not delete this branch until dependent successor rebases are complete.

@lmeyerov

Copy link
Copy Markdown
Contributor Author

This probably needs to get restacked on 1999 because likely conflicts

from graphistry.Engine import is_polars_df
from graphistry.compute.typing import DataFrameT

EndpointFrameT = typing.TypeVar("EndpointFrameT")

@lmeyerov lmeyerov Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, clarify why we track else drop...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept and clarified in 696327559. The TypeVar preserves the caller's exact pandas/cuDF/Polars frame flavor through the runtime dispatcher; DataFrameT intentionally models pandas during type checking, so replacing the signature with DataFrameT -> DataFrameT would erase Polars return precision. The declaration now carries a single guard-compliant constraint comment. Focused CPU, lint, and mypy are green.

@lmeyerov

Copy link
Copy Markdown
Contributor Author

I can go either way here. What does gql do vs cypher ?

@lmeyerov
lmeyerov changed the base branch from master to fix/1995-null-endpoint-contract August 21, 2026 20:11
lmeyerov and others added 8 commits August 27, 2026 01:24
…projection

`MATCH (a)-->(b) RETURN b` answered the deduplicated node SET instead of the
openCypher bag: 3 rows where the 4-edge match bag binds node 3 twice. Silent,
and self-inconsistent -- every property spelling of the same projection
(`RETURN b.id`, and `RETURN a, b`) was already row-correct.

Two vetoes sent it to the per-alias node table: the multiplicity predicate
bailed on any bare-alias projected item, and the projection lowering vetoed
binding rows whenever the plan carried a whole-row output. Both now admit a
whole-entity node output over a relationship pattern.

The lane switch alone was not enough. polars could not render a whole entity
off a binding-row frame at all -- `MATCH (a)-->(b) RETURN a, b` raised
NotImplementedError there while pandas and cuDF answered it. The polars
projector now resolves each alias through a per-alias view (the twin of the
pandas _projection_alias_rows), so single-entity and multi-entity binding rows
render alike, and alias-relative property sources resolve through the same map.

Deliberately unchanged: DISTINCT (asks for exactly the node-set dedup), a
whole-row WITH carry into re-entry (the trailing MATCH cannot yet separate
matched from unmatched rows on a duplicated prefix), and relationship-free
patterns. The seeded fast path recognizes the whole-entity bag lowering so the
seeded shape stays on the fast lane, and defers on a zero-row bag so the
empty-frame dtype contract stays single-sourced in the full path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjbKuKheqDu78oapRT5AYm
Variable-length arms keep the node-set lane: their openCypher bag is the
relationship-unique walk expansion, not the edge bag this lane counts, and the
polars text fallback cannot render that frame. Excluding them matches the
multiplicity predicate's documented scope and leaves the varlen shape byte-for-
byte as before.

Pins updated, each because the lowering legitimately moved:
- test_alias_scoping_semantics: `RETURN a AS b` pinned the 3-name node set while
  its own sibling property spelling pinned the 4-row bag -- it encoded the
  defect. Now the bag.
- test_hop_semantics_pins: the polars whole-entity NIE pin carried an AUDIT NOTE
  asking for a row-level parity assertion once the gate was narrowed. Flipped to
  exactly that.
- test_engine_polars_row_pipeline: `MATCH (n)-[e]->(m) RETURN n, m` moved from
  DEFERRED to NATIVE_LOWERED (parity + polars-typed both hold).
- cypher/test_lowering: two bracketless-relationship tests asserted the raw
  node-table column names of the node-set lowering; binding rows spell them
  "{alias}.{field}". The third pinned a compile-time refusal of a query whose
  `root` is out of scope after an aggregating WITH; it is still refused, now at
  execution.
- test_row_multiplicity_semantics: the strict-xfail residual is now a real
  assertion (2 xfails removed).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjbKuKheqDu78oapRT5AYm
@lmeyerov
lmeyerov force-pushed the fix/gfql-1994-whole-entity-bag branch from 9de0b68 to 6963275 Compare August 27, 2026 08:54
@lmeyerov
lmeyerov changed the base branch from fix/1995-null-endpoint-contract to master August 27, 2026 08:56
@lmeyerov

Copy link
Copy Markdown
Contributor Author

Recommendation: keep the non-DISTINCT bag behavior. Current Cypher documents plain RETURN and RETURN ALL as equivalent, with ALL added for GQL conformance; its exact example returns the same node twice when two relationships bind it, while RETURN DISTINCT returns it once. That matches #2000: preserve binding-row multiplicity for fixed-length whole-node projection, and deduplicate only for DISTINCT. Official reference: https://neo4j.com/docs/cypher-manual/current/clauses/return/

@lmeyerov
lmeyerov merged commit d509e42 into master Aug 27, 2026
76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant