perf(gfql): make schema-derived per-type facts opt-in, and build them in one pass - #1859
Merged
Conversation
… in one pass
Measured: per-type facts add ~104ms to a ~262ms gfql_index_all on a 420k-node /
2.8M-edge graph (+39%), and only typed count shapes can spend that -- roughly 40
such queries before the build pays for itself. Two fixes.
SINGLE PASS: the edge role aggregated each endpoint binding separately, so the
edge frame was grouped twice. build_col_stats_facts_by_type now takes a SEQUENCE
of columns and aggregates them together: edge side +52.0 -> +28.0 ms, total
+103.8 -> +75.1 ms. Decline is all-or-nothing across the columns so a caller
cannot get a partial answer and read it as complete.
OPT-IN: building from a bound GraphSchema now requires col_stats_by_type=True on
gfql_index_col_stats()/gfql_index_all(), default False. Binding a schema is a
declaration about the DATA, not consent to a costlier index build; with a schema
bound the opt-in measures +9.2%, and it scales with label count since label__X
costs one grouped pass per label. Explicit *_type_column requests are unaffected.
Caught by the pins while making the change: the shared pass initially
fingerprinted every fact across ALL aggregated columns, but get_col_stats_valid
recomputes over {column, type_column}, so every partition fact looked stale and
silently stopped engaging. Fingerprints are now per column.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
Reviewer caught the inconsistency behind '+1.8% with no schema, because the flag builds nothing': a flag that builds nothing is not a cheap flag, it is a silent no-op. Setting col_stats_by_type=True is an EXPLICIT request, so by the contract already used for node_type_column=/edge_type_column= it must raise rather than quietly do nothing. It now does, with the two reasons distinguished because the fixes differ: no schema bound (bind one, or name the columns), versus a bound schema declaring no type column the frames carry. Partial coverage still skips -- a declared label the frame lacks is legitimate, since a schema is a contract for the whole graph. Also stops the dead work: the schema candidate helpers ran unconditionally, scanning frame columns even with the flag off. Re-measured with the no-op configuration gone: the honest opt-in cost is +28.5ms on a 265ms index_all = +10.7% (2 label columns + 1 edge type column), and it scales with label count. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
…ench Perf magnitudes in a public repo double as an optimization roadmap. The docstrings added with this change quoted build costs and break-even counts outright; they now state the SHAPE of the constraint -- a grouped pass per type column, one per label under label__X, spendable only by typed count shapes -- which is what a reviewer or a future maintainer needs to not delete the gate. The numbers move to pyg-bench (#184), which is also the standing rule: claims live in pins and receipts, not in prose. Reasoning is deliberately NOT stripped: _dtype_kind still records that pandas' is_integer_dtype raises on cudf ListDtype, because that is a correctness constraint someone would otherwise 'simplify' away. Only the timings go. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
lmeyerov
added a commit
that referenced
this pull request
Aug 9, 2026
Facts are a pure accelerator, so a dead one is invisible: the query still returns the right answer via the scan, every value test stays green, and you pay the build for nothing. That is the failure mode behind three bugs in this stack -- the boolean label form never engaging, the shared-pass fingerprint making every fact look stale, and col_stats_by_type silently no-oping. Each consult now records op=col_stats with an outcome whose cases need different fixes: absent / stale / insufficient / served. 'insufficient' is what a whole-frame fact always is against a typed pattern -- the structural limitation that took a full investigation to diagnose is now one trace line. Gated by the same _trace_active() check the adjacency decisions use, so it costs nothing outside index_trace()/gfql_explain; pinned both ways. It also makes engagement testable against a PUBLIC surface rather than by patching private callees -- which matters: patching a name that another module imported directly does not intercept, and produces a false negative that reads like a dead optimization. Stacked on #1859. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
This was referenced Aug 12, 2026
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.
Follow-up to #1858, from measuring the build cost nobody had measured.
The measurement
Board-shaped graph, 420k nodes / 2.8M edges, polars (dev box):
col_statswhole-frame onlyn_uniqueper partition, needed for the density proof, dominates)gfql_index_all= +39%Query side gains ~2.5 ms on a typed q8, so break-even is ~40 queries of that shape. On the 9-query board, eight of nine queries would pay and get nothing.
Two fixes
Single pass.
build_col_stats_facts_by_typenow takes a sequence of columns and aggregates them together, so the edge frame is grouped once rather than once per endpoint: edge side +52.0 → +28.0 ms, total +103.8 → +75.1 ms. Declines are all-or-nothing across the requested columns, so a caller cannot receive a partial answer and read it as complete.Opt-in. Building from a bound
GraphSchemanow needscol_stats_by_type=Trueongfql_index_col_stats()/gfql_index_all(), default False. Binding a schema is a declaration about the data, not consent to a costlier index build — with a schema bound the opt-in measures +9.2%, and it scales with label count sincelabel__Xcosts one grouped pass per label. Explicitnode_type_column=/edge_type_column=requests are unaffected: asked for by name, still built, still raising when unusable.A regression the pins caught
The shared pass initially fingerprinted every fact across all aggregated columns, but
get_col_stats_validrecomputes over{column, type_column}— so every partition fact looked stale on lookup and silently stopped engaging. Values stayed correct (a miss just scans), which is exactly why it needed an engagement pin rather than a value pin to catch. Fingerprints are now per column.Pins
Single-pass vs per-column equivalence; all-or-nothing decline across columns; the opt-in gate on both entry points including that whole-frame facts are unaffected; plus every #1858 pin still green.
🤖 Generated with Claude Code
https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi