Repository navigation
fix(gfql): treat binding seed ids as identities - #2001
Conversation
|
Is incorrectly stacked? |
| return None | ||
|
|
||
|
|
||
| def _run_generic(engine: _GFQLEngine, *, parallel_edge: bool) -> typing.Mapping[str, int]: |
There was a problem hiding this comment.
Is this sufficient, positive and negative testing on either side of the boundary?
There was a problem hiding this comment.
Yes—the regression now pins both sides of the boundary. Duplicate physical node-table rows for ids 1/2 are the positive case: the initial traversal wavefront must collapse them as the same graph identities (LA=6, NY=2, SF=1). A genuine parallel 1 -> 5 relationship is the negative control: relationship match rows must not be deduplicated, so only LA increases to 7. Both cases run across the engine matrix. I also removed monkeypatch.setattr: neutral SKIP 0 selects the generic route, and assert_fast_path(..., served=False) proves the grouped fast path was consulted and declined, so the value assertion cannot pass vacuously.
9de0b68 to
6963275
Compare
01b0c66 to
a888c33
Compare
|
Resolved. The stack was stale after #2000 was rewritten and landed. #2001 is now rebased directly onto |
Summary
Correctness oracle
The fixture duplicates storage rows for node ids 1 and 2 but adds no graph identities or relationships. Its hand-derived grouped counts are
LA=6, NY=2, SF=1; adding one parallel1 -> 5relationship changes only LA to 7. Before this fix the generic route returned10/3/1and12/3/1by multiplying paths from duplicate seed rows.Semantics boundary
GQL-conformant Cypher preserves binding-row duplicates for plain
RETURN/RETURN ALL; onlyDISTINCTdeduplicates (Neo4j Cypher RETURN). This change follows that boundary:Validation
a888c33f2d42f626bb0754fb1186b2d1c1bf6561master@d509e428b0b63a7dd6d92ee5271e75afd15ca2bf; zero merge commitsAny,object,cast,getattr,setattr,hasattr, or unqualifiedList/Dict; finite engine values useLiteral./bin/lint.sh: syntax, Ruff, type-hygiene, comment-density, and relative-import guards clean./bin/typecheck.sh: mypy 2.3.1, 336 source files, zero issuesReview resolutions
SKIP 0to select the generic route andassert_fast_path(..., served=False)to prove the grouped fast path was consulted and declined.Closes #1996
Reviewer handoff
Please review the incremental four-file diff for:
monkeypatch.setattror another dynamic-attribute escapeStack / landing
d509e428b0b63a7dd6d92ee5271e75afd15ca2bfmaster