Skip to content

Commit 5f4cade

Browse files
committed
docs+test(gfql): CHANGELOG and both-direction tests for the lean-combine shrink
Owner's merge conditions for this PR were a CHANGELOG entry, correctness tests in both directions, and a pyg-bench lane (the last is separate). Tests pin both sides of the new empty-left branch: - empty left shrinks to zero rows AND keeps columns/dtypes identical, so the downstream merge still produces the same schema; - the shrunk merge equals the unshrunk merge at the call site's actual how='left'; - non-empty lefts (1, 5, 999, 1000 rows — spanning both sides of the ratio gate) are byte-identical to the unshrunk merge, i.e. the new branch perturbs nothing; - a non-empty left whose keys miss entirely still yields one null-filled row per left row, rather than being collapsed; - end-to-end: a seed matching nothing gives lean-on == lean-off through chain(). Mutation-checked. Reverting the branch to `return right` fails the shrink test. A second mutation (also shrink when no keys overlap) is NOT caught — correctly: under how='left' both forms give null-filled rows, so there is no behaviour to distinguish. The docstring says so rather than implying coverage it does not have. Also records the safety precondition in the CHANGELOG: the shrink is used at exactly one call site, which is how='left'; a how='right'/'outer' merge retains unmatched right rows and would NOT be safe to shrink this way.
1 parent ab05e3a commit 5f4cade

2 files changed

Lines changed: 94 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
1212
- **GFQL secondary node property indexes (`create_index('node_prop', column=...)` / `g.gfql_index_node_props([...])`)**: a seed predicate on a NON-key column — `MATCH (m {id: 42})` where the graph's node id binding is some other column — previously cost a full node scan, because the registry only indexed the node-id binding and the CSR adjacencies. A property index is the same pay-as-you-go sidecar as the existing kinds: sorted distinct values over node **row positions** (CSR, so duplicate values are indexable), never reorders `.nodes`, fingerprint-validated so a `.nodes()` rebind is treated as absent (safe miss, never a wrong answer), engine-polymorphic (numpy host / cupy on-device), and policy-gated (`off`/`use`/`auto`/`force`). The seeded fixed-hop planner picks the **most selective** indexed scalar predicate in the seed filter using a free CSR-offset estimate, gathers those candidates, and applies every remaining predicate to them — so results are identical whether the index is present, absent, stale, or cost-gated out. `show_indexes()` lists property indexes; `drop_index('node_prop', column=...)` drops one. Only integer columns are indexable today (float NaN ordering, strings on cupy, and nulls all decline to the scan); widening that is additive. **Perf (dgx-spark, official LDBC SNB SF1, 3.18M nodes, warm median, value-identical 19-row result):** interactive-short IS7 `71.6 ms -> 19.5 ms` (**3.7x**), with a one-time `112 ms` build — the seed lookup itself goes from a `51.2 ms` scan to `0.096 ms`.
1313

1414
### Performance
15+
- **Seeded chain combine stops joining against the full frame when the intermediate is empty**: `_lean_prefilter_right` shrinks the big side of the combine's `how='left'` merge to the keys actually present on the left, but it declined to do so in the one case where shrinking is both maximally profitable and trivially correct — an **empty** left. A left merge keeps only the right rows that match, so a zero-row left yields a zero-row result whatever the right side holds; the merge was nonetheless materializing the whole graph-sized frame. It now hands back a zero-row slice of `right` (same columns and dtypes, so the merge still produces an identical schema). Measured: a single-node query whose 0-row intermediate was joined against 14M edges went **112.64 → 17.29 ms**. The shrink is used at exactly one call site, which is `how='left'`; a merge that retains unmatched right rows (`right`/`outer`) would NOT be safe to shrink this way, and the tests pin both directions — the empty-left case must return an empty result, and the non-empty cases must be byte-identical to the unshrunk merge.
16+
- **Native polars chain reuses the synthetic edge id as its stable row order**: when the executor had already added a synthetic edge-index column it also added a second, separate row-index column purely to restore input order at the end — two full `with_row_index` passes over the edge frame, and a redundant column carried through every intermediate. The existing synthetic id is already a contiguous, order-preserving row index, so it is now reused as the sort key and dropped once at the end. Only the pre-existing column is reused; when no synthetic id was added the separate order column is still created, so graphs that bring their own edge id are unaffected. Value-identical including row order (148.22 → 135.43 ms on a 3.18M-node / 14M-edge polars graph).
1517
- **GFQL indexed typed-edge traversals stop scanning the whole edge frame (#1658)**: the index path built the simple-equality `edge_match` mask over ALL `E` edges — one `(series == val)` over the full column — and then read that mask only at `rows[edge_keep[rows]]`, the handful of positions the CSR adjacency lookup had already returned. That put an **O(E) predicate scan inside an O(degree) traversal**, which is why an indexed seeded typed hop scaled with the graph while the underlying `g.hop()` stayed flat. The predicate is now evaluated on the gathered candidate rows instead: `_build_edge_row_filter` does the schema-level validation up front (the dtype-mismatch decline that keeps error parity with the scan is O(1) and unchanged) and `_EdgeMatchRowFilter.mask_for(rows)` applies `col == val` to just those rows, using each frame's native `==` exactly as before (so cuDF string columns stay on the cuDF layer). This is a cost tradeoff, not a free win, and it is bounded rather than assumed: candidate-row evaluation beats one whole-column compare only while the candidates stay a small fraction of the frame, and a fixed-point walk that reaches most of the graph inverts it — the out- and in-indices are filtered separately, so it can gather up to 2E elements by random access against the eager form's single sequential pass over E (measured before the guard: an undirected `to_fixed_point` typed walk gathered 1.94×E and ran 1.2–1.6× SLOWER than master). A cumulative-cost guard now switches to the whole-column mask once gathered rows reach E/8, so total predicate work is bounded at ~1.125×E in that regime while a seeded hop — which gathers ~degree — never approaches the threshold and never builds it. An evaluation failure mid-traversal abandons the indexed path entirely so the scan fallback stays parity-safe. Measured on a 3.18M-node / 14M-edge polars graph (LDBC SNB SF1-shaped), a seeded typed-edge query with alias markers goes **309.9 → 57.4 ms (5.4×)**, value-identical, with the full-column compare (previously 248 ms, 81% of the query) gone from the profile. Parity held across 650 differential comparisons — pandas + polars × 13 traversal shapes × 25 random seeds on a dense graph with null-carrying string and numeric columns — comparing columns, dtypes, row counts and full content, including the decline paths (membership `edge_match` still routes to the scan; a dtype mismatch still raises the same `GFQLSchemaError`). One honest limit on error parity: only the O(1) dtype gates are parity-exact. A *data-dependent* comparison failure (e.g. a list-valued cell in an object column) is now observed only if it lands in the gathered candidates, so the indexed path can succeed where the scan raises. That needs pathological cell values to reach, but it is a real narrowing of the previous guarantee. Pinned by two structural regression tests that assert the *shape* rather than a wall-clock number: the predicate must see only candidate rows, and growing the graph 8× at fixed degree must not grow the number of rows it examines.
1618
- **GFQL indexed fixed-hop bindings now dispatch BEFORE the canonical traversal (pandas / cuDF / native Polars)**: the resident-index fixed-hop path (`rows(binding_ops=...)`, the Cypher multi-alias `MATCH … RETURN` shape) was consulted only *inside* row materialization — after the engine had already run the full canonical traversal — so its compact path bag was computed on top of the graph-sized work it exists to avoid. Both the pandas/cuDF chain boundary and the native Polars chain now ask the same shared, structural gate first and, only when it can serve the WHOLE middle exactly, skip the canonical traversal and hand the compact state to the unchanged row materializer. Engagement stays operator/index/dtype/cost based — no query, schema, or hop-count recognition — and every unsupported, seeded, prefiltered, policy-bearing, shortest-path, or cost-gated shape still declines to canonical execution with identical results. On a standard LDBC SNB SF1 interactive-short profile this removed the redundant two-hop typed-mask and 16-merge buckets and cut the profiled call ~11.9× (1263 ms → 106 ms), exact 19-row oracle preserved.
1719
- **Indexed seed lookup now covers a unique node id PLUS extra scalar constraints**: `{node_id: v, other: w}` previously fell back to a full node-table scan even though the unique node-id index can gather at most one row; it now gathers that one row and applies the remaining constraints to it. Missing ids and non-matching extra constraints return the same empty result as before, and duplicate-id graphs (which cannot build the index) are unaffected.

graphistry/tests/compute/test_chain_lean_combine.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,3 +200,95 @@ def _spy_prefilter(left, right, key, engine):
200200
# both lean paths must fire on a seeded chain (else parity tests are vacuous)
201201
assert hits["intersect"] >= 1
202202
assert hits["prefilter"] >= 1
203+
204+
205+
# --- empty-left shrink (#1783): both directions --------------------------------------
206+
# The shrink returns a ZERO-ROW slice of `right` when `left` is empty. That is only sound
207+
# because the sole call site merges how='left', which discards unmatched right rows anyway.
208+
# These pin both directions: the empty case must shrink AND stay schema-identical, and the
209+
# non-empty cases must be untouched by the new branch.
210+
211+
def _wide_right(n_rows: int = 1000) -> pd.DataFrame:
212+
rng = np.random.default_rng(5)
213+
return pd.DataFrame({
214+
"id": np.arange(n_rows, dtype=np.int64),
215+
"f": rng.random(n_rows),
216+
"s": np.array([f"v{i % 7}" for i in range(n_rows)]),
217+
"b": rng.integers(0, 2, n_rows).astype(bool),
218+
})
219+
220+
221+
def test_empty_left_shrinks_to_zero_rows_preserving_schema():
222+
"""The optimization itself: nothing survives a left merge from an empty left."""
223+
right = _wide_right()
224+
left = right.iloc[:0][["id"]]
225+
out = _lean_prefilter_right(left, right, "id", Engine.PANDAS)
226+
227+
assert len(out) == 0, "empty left must not carry the full right frame into the merge"
228+
# Schema-identical, or the downstream merge would produce different columns/dtypes.
229+
assert list(out.columns) == list(right.columns)
230+
assert list(out.dtypes) == list(right.dtypes)
231+
232+
233+
def test_empty_left_merge_result_matches_the_unshrunk_merge():
234+
"""End-to-end equivalence at the call site's actual merge type."""
235+
right = _wide_right()
236+
left = right.iloc[:0][["id"]]
237+
shrunk = _lean_prefilter_right(left, right, "id", Engine.PANDAS)
238+
239+
got = left.merge(shrunk, on="id", how="left")
240+
expected = left.merge(right, on="id", how="left")
241+
pd.testing.assert_frame_equal(got, expected)
242+
243+
244+
@pytest.mark.parametrize("n_left", [1, 5, 999, 1000])
245+
def test_non_empty_left_is_unaffected_by_the_empty_branch(n_left):
246+
"""The other direction: adding the empty-left case must not perturb any non-empty
247+
one — neither the shrink-eligible small lefts nor the ones the ratio gate declines."""
248+
right = _wide_right()
249+
left = right[["id"]].head(n_left)
250+
out = _lean_prefilter_right(left, right, "id", Engine.PANDAS)
251+
252+
got = left.merge(out, on="id", how="left").sort_values("id").reset_index(drop=True)
253+
expected = left.merge(right, on="id", how="left").sort_values("id").reset_index(drop=True)
254+
pd.testing.assert_frame_equal(got, expected)
255+
assert len(got) == n_left
256+
257+
258+
def test_empty_left_with_no_matching_keys_is_still_empty():
259+
"""A NON-empty left whose keys miss entirely must still yield null-filled rows, one
260+
per left row — not a zero-row frame.
261+
262+
Note on what this does and does not pin: it constrains the RESULT, not the branch
263+
taken. Shrinking on 'no key overlap' would also be sound under how='left' (both give
264+
null-filled rows), so this test deliberately cannot distinguish those two
265+
implementations — verified by mutation. What it does catch is the result-level error:
266+
dropping the unmatched left rows entirely.
267+
"""
268+
right = _wide_right()
269+
left = pd.DataFrame({"id": np.array([10_000, 10_001], dtype=np.int64)})
270+
out = _lean_prefilter_right(left, right, "id", Engine.PANDAS)
271+
272+
got = left.merge(out, on="id", how="left")
273+
expected = left.merge(right, on="id", how="left")
274+
pd.testing.assert_frame_equal(got, expected)
275+
assert len(got) == 2 and got["f"].isna().all()
276+
277+
278+
def test_empty_left_shrink_holds_through_the_chain():
279+
"""The shape the fix exists for: a seed that matches nothing, so the intermediate is
280+
empty and the combine would otherwise join it against the whole frame. Lean-on must
281+
equal lean-off."""
282+
g, _ = _seeded_graph()
283+
q = [n({"id": -12345}), e_forward(), n()]
284+
285+
os.environ['GFQL_LEAN_COMBINE'] = '0'
286+
off = g.chain(q, engine='pandas')
287+
os.environ['GFQL_LEAN_COMBINE'] = '1'
288+
on = g.chain(q, engine='pandas')
289+
290+
pd.testing.assert_frame_equal(
291+
off._nodes.sort_values(list(off._nodes.columns)).reset_index(drop=True),
292+
on._nodes.sort_values(list(on._nodes.columns)).reset_index(drop=True),
293+
)
294+
assert len(on._nodes) == 0

0 commit comments

Comments
 (0)