Repository navigation
fix(gfql): row-values series carry the table index so NOT IN keeps its rows under the index path - #2121
Conversation
…s rows
NOT (b.id IN [...]) on the destination alias returned 96 rows under
index_policy='force' where the scan returns 111; on 20k nodes 880 rows and 174
whole destination ids vanished. Positive IN, the equivalent <> conjunction and
NOT IN on the source alias were correct.
The row evaluator mixed two index conventions: six producers built results
positionally (reset_index(drop=True).assign -> RangeIndex) while most consumers
align by label, including the .where chain inside the tri-valued NOT builder,
which broadcasts on the table's labels. They agree only on a RangeIndex frame.
The index path's hop returns nodes via take_rows / select_by_ids, which keep the
original labels with gaps, so inside NOT a 0..27 mask aligned against labels
[0,1,2,3,4,6..29]: 28 and 29 had no mask row and every label past the gap took
its neighbour's value -- the dropped ids were exactly {9, 28, 29}.
A single helper gives a positionally built series the table's own index; the
shared row-values helper returns through it instead of re-resetting, and the
five inline producers route through it. #2020 fixed the positive-IN pushdown by
applying by position; NOT used a different builder and was never reached.
Pinned: force/use/off parity on the 30-node repro, a non-RangeIndex frame
control, and a unit pin on the helper's index. Detector pins fail on master.
gfql row+cypher+index suites: 4889 passed with cuDF deselected (libnvrtc gap
on this box, identical on master).
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
Temporal comparison, temporal IN and list comparison build a reset work frame and label-assign evaluator series into it; on the gappy frames the index path returns the rows shift past the gap. Add _gfql_assign_positional and pin the families under off/use/force. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
…sured master behavior Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
… raising (3b) NOT (x IN [...]) covering every row raised 'AST evaluator unsupported' on pandas, IN matching nothing raised 'cudf does not support mixed types', list compares broadcast 0 against 1: the evaluators type masks from the values they see and an empty frame has none. where_rows validates then short-circuits on an empty frame; tri-valued results are boolean even when empty. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
Read-only review (parallel session) — findings on head 215a297 vs master 88b99ccNothing here touches the branch; everything below is reproducible from the listed inputs. Mode: findings, fixes deferred. A patch of the missing tests is attached at the end. Target: What checks out
IMPORTANT1. The fix closes 6 producers; 10 same-class producers still return a RangeIndex, so NOT/OR still misalign under
|
…pty path keeps strict reporting Review found three gaps in the first cut. The empty-frame short-circuit skipped the absent-property report, so a typo in a property name went unreported under strict whenever the prefilter happened to empty the frame; the predicate is now evaluated on the empty frame like any other and a comparison with a zero-row operand answers a zero-row mask. The alignment helpers relied on set_axis, which cuDF does not have, so the fix was a no-op there; they now set the index, which both engines support, and the pins run on cuDF. And the invariant held for six producers only: aligning at the evaluator's return closes it for every producer, so quantifiers, list comprehensions, range, subscript, slice and concatenation under NOT and OR now agree across index policies, each pinned. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
|
Thanks — this landed on target. All six IMPORTANT items are addressed at 629aa47; replying by number. 1. Ten same-class producers still misalign. Closed as a class, not narrowed. Pinned as 2. Both helpers are identity functions on cuDF. Confirmed and fixed the way you suggested: 3. Empty-frame short-circuit drops strict schema errors. Confirmed — this was the real regression. The short-circuit is removed. The reason it existed was 4. Broad 5. No cuDF detector. Added, see 2. The pre-existing control keeps its place with a docstring saying it passes on master too. 6. RTD docs-drift gate. Confirmed and quantified: the gate is On the suggestions: the five master-wrong/PR-right cases are worth having and I will take the patch's shapes as a follow-up rather than hand-copying them now; DRY between Gates at this head: |
The GPU receipt found the last empty-frame hole: `[t.type] = ['robot']` on an emptied cuDF frame reached the mask coercion as an empty object column, and cuDF rejects where(object, False). An empty mask has no null to fill, so it is cast directly. Master raises on the same input, so this closes the shape rather than restoring anything. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
|
GPU receipt at the current head, and it found one more case that CI cannot see. dgx-spark, GB10, The first GPU pass at 629aa47 failed one cell: Worth noting for the docs lane: this PR's |
Defect
MATCH (a)-[e]->(b) WHERE NOT (b.id IN [...]) RETURN b.idreturned 96 rows underindex_policy='force'whereoff/useand plain pandas return 111 (30 nodes / 120 edges, seed 3001, seeds[3, 5, 10]); on 20k nodes / 100k edges, 880 rows (174 whole destination ids) vanished. No extra rows, deterministic,NOT-only, destination-alias-only, index-path-only. PositiveIN, the equivalent<>conjunction, andNOT INon the source alias were all correct.Cause
The row evaluator mixes two index conventions. Six producers build their result positionally (
table_df.reset_index(drop=True).assign(...)[col]→ freshRangeIndex), while most consumers align by label (.loc[mask],assign, and the.wherechain inside the tri-valuedNOTbuilder, which broadcasts on the table's own labels). They agree only when the frame has aRangeIndex. The scan hop happens to produce one; the index path's hop returns the nodes frame viatake_rows/select_by_ids, which preserve the original row labels — with gaps. InsideNOT,out.where(~true_mask, …)then aligned a 0..27 mask against labels like[0,1,2,3,4,6,…,29]: labels 28/29 had no mask row (→ NaN → dropped) and every label past the gap took its neighbour's value — exactly the dropped ids{9, 28, 29}.#2020 (c3312ea) fixed this class for the positive-
INpushdown by applying masks by position;NOTgoes through a different builder and was never reached.Fix
One helper,
_gfql_on_table_index, gives a positionally built series the table's own index; the shared_gfql_series_from_row_valuesreturns through it (instead of re-resetting to aRangeIndex), and the five inline producers (IN, temporalIN, both list-comparison returns, mask-fill) route through it. Label-aligned and positional consumers now agree on any frame. No.wheresite changes.Proof
off/use/forceall equal plain-pandas truth (111; 99,754).RangeIndexCypher pin is a both-sides control.row+cypher+indexsuites: 4,889 passed, 420 skipped, 17 xfailed (cuDF deselected: the one cuDF failure islibnvrtc.so.12missing on this box and reproduces identically on master).INrow and the polars int/float category gate remain separate defects, tracked next).Anyparams carry the file'shygiene-okmarker).Found while verifying the
indexing.rstexamples withgfql_explainunder everyindex_policy.Also in this PR: #2116 item 3b — a WHERE that excludes every row raised instead of returning no rows
The colleague's exact text ("AST evaluator unsupported") fires when
NOT (alias.col IN [...])excludes every surviving row: the alias prefilter leaves an empty frame,where_rowsevaluates the predicate on it, andINbuilds its tri-valued mask from an empty Python list, which pandas types float64 (cuDF: object) and the NOT gate declines. Same root, three pre-existing symptoms on master: pandas NOT-IN-all raised; cuDFalias.col IN [...]matching no row raised "cudf does not support mixed types" (#2125, retitled); pandas[t.id] < ['a']with no survivor raised "Lengths must match to compare". Fix at the cause:where_rowsvalidates the predicate (_gfql_parse_row_expr) then short-circuits on an empty frame;_gfql_tri_valued_seriestypes the four tri-valued producers boolean even when empty. Pins intest_where_rows_empty_frame_3b.py: 7 shapes × pandas/cuDF →[], controls, bad-predicate-still-rejected, two unit pins; 19 pass here, 9 fail on master. Full compute-lane suites 4907 passed; changed-line coverage 89.1%.🤖 Generated with Claude Code
https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp