Skip to content

Commit efe17f2

Browse files
allanturner1234claude
authored andcommitted
perf(gfql): serve NAMED patterns on the native chain fast path
`_try_chain_fast_path` rejected ANY op carrying an alias, so a named `g.gfql([n(name='x'), e_forward(name='r'), n(name='y')])` fell to the full two-pass BFS purely because the ops were named. Naming is a PROJECTION concern, not a traversal one; it should not decide which engine path runs. RE-MEASURED on a 200-node / 800-edge pandas graph (data work ~0, so this is the per-query plan floor). Five ALTERNATING PAIRED runs vs origin/master 233b64c, each value the median of 200 reps after 20 warmups (range over the five runs in parentheses -- the run-to-run spread is real): named 3-op traversal 25.2 ms (24.2-27.0) -> 2.3 ms (2.1-2.7) ~11x the same + rows(binding_ops) 40.4 (38.1-43.6) -> 17.4 (16.8-19.8) ~2.3x The rows() output is byte-identical between arms; the traversal output is value-identical (dtypes deliberately changed, below). SCOPE, AND THE EARLIER CLAIM WAS WRONG. The previous description said "every Cypher multi-alias MATCH...RETURN names its ops, so the entire Cypher surface is locked out". An instrumented run of the graph-benchmark q1-q9 lane at 20k, both engines, both arms, falsifies that three independent ways: every consult on that lane is ops=1 and this change only edits the len(ops)==3 branch; g.gfql(<cypher>) for those shapes is served earlier by gfql_fast_paths.py (_execute_single_hop_grouped_aggregate_fast_path, _execute_two_hop_count_fast_path, _execute_seeded_typed_hop_fast_path) and never reaches here; and this path is pandas/cuDF only while the losing column is polars. Zero consults on q1/q2/q3/q4/ q8/q9 pandas and on all of q1-q9 polars, identical on both arms. NO BENCHMARK CELL MOVES. A positive control confirms the lever is live rather than inert (arm A DECLINE at the removed gate, arm B SERVE, same native 3-op named chain). Under the pyg-bench-proof-plus-lock-in rule this therefore carries NO perf claim on the board; the lane is specified as follow-up in the PR description. HOW THE TAGS ARE DERIVED: from the KEPT EDGES the fast path returns. `combine_steps` tags a node with an alias iff it still participates in a surviving edge, and those edges are exactly the surviving ones, so `isin` over their endpoint columns is the same predicate without the join. A seed whose edges all fail the filter yields an empty edge frame and is tagged False -- the dead-end case, and why the tag keys on edges rather than on the node filter. DECLINES KEPT DELIBERATELY: undirected + named (alias identity is not derivable from endpoint columns there); duplicate alias names (E201 lives in `combine_steps`, so serving them would BYPASS the check and silently succeed); and a resident index that would actually serve the shape (`_resident_seed_indexes` asks about index VALIDITY -- a registry-presence check declines on every query and makes the whole thing a measured no-op). CODE LOCATION: `_tag_fast_path_aliases` lives in `chain_fast_paths.py`, not `chain.py`. That is the pre-existing direction, confirmed not recalled: the module was created by 289c6ab "extract seeded fast-path specializations to dedicated modules (#1755)" and its docstring states the one-way import rule. The docstring now records it explicitly so the next helper does not land in chain.py again. DTYPES: ADOPT the conformant ones. The pandas full path upcasts non-id int64 -> float64 and bool -> object through its rows-pivot merges. Verified rather than assumed that this is an ARTIFACT: openCypher TCK clauses/return/Return2.feature [2] expects `1` for an integer node property (its value grammar distinguishes Integer from Float); Neo4j treats INTEGER and FLOAT as distinct property types and reports valueType()="INTEGER NOT NULL"; and this library's own polars and cuDF canonical paths already return int64/bool for the same query, so only the pandas merge upcasts. The fast path keeps int64/bool. User-visible where a named pandas pattern is served: 30 rather than 30.0, and a bool alias flag rather than object. Values unchanged. NOT aligned yet, deliberately out of scope: the pandas full path and the Cypher-layer projection still emit the artifact. TESTS: 4 deliberate rewrites, each commented with the rule it now encodes. test_fast_path_gating_returns_none_for_ineligible and the _FAST_SHAPES/_BYPASS_SHAPES differential move named shapes from decline to serve and add the new declines; test_pandas_datetime_property_declines and test_engine_mismatch_declines keep asserting the Cypher-layer decline and identical values, with the dtype rule now asserted explicitly instead of incidentally. NEW coverage for the new capability: test_fast_path_named_alias_columns_match_full_path (9 shapes x 2 engines) compares the alias flag COLUMNS and per-row values fast-vs-full, which the id-set differential cannot see, and is serve-asserted. CHANGELOG added (Performance + Changed). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
1 parent 49db91c commit efe17f2

6 files changed

Lines changed: 280 additions & 16 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2222
- **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.
2323
- **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.
2424
- **Seeded typed-hop property projection keeps its fast path through DISTINCT / ORDER BY / SKIP / LIMIT**: those trailing row ops are plain frame operations, so the fast path now delegates them to the canonical chain instead of declining the whole query.
25+
- **The pandas/cuDF chain fast path serves NAMED patterns**: `_try_chain_fast_path` rejected any op carrying an alias, so `g.gfql([n(name='x'), e_forward(name='r'), n(name='y')])` fell to the full two-pass BFS purely because the ops were named — naming is a PROJECTION concern, not a traversal one, and it should not decide which engine path runs. The alias flag columns `combine_steps` would have merged in are now reconstructed directly from the edges the fast path returns (`_tag_fast_path_aliases`): `combine_steps` tags a node with an alias iff it still participates in a surviving edge, and those edges are exactly the ones the fast path already produced, so `isin` over their endpoint columns is the same predicate computed without the join. A seed whose edges all fail the filter yields an empty edge frame and is tagged `False`, which is the dead-end case. Deliberate declines kept: **undirected + named** (an undirected edge makes a node reachable as either endpoint, so alias identity is not derivable from the endpoint columns), **duplicate alias names** (E201 is raised by `combine_steps`, so serving here would bypass the check and let alias reuse silently succeed), and **a resident index that would actually serve the shape** (checked with `_resident_seed_indexes`, i.e. index VALIDITY for these exact frames — a registry-presence check instead declines on every query and silently turns the whole optimization into a measured no-op). **Measured** on a 200-node / 800-edge pandas graph, where data-proportional work is ~0 so this is the per-query plan floor. Five alternating paired runs against `origin/master`, each the median of 200 reps after 20 warmups: the named 3-op traversal `25.2 ms (24.2–27.0) → 2.3 ms (2.1–2.7)`, **~11×**, and the same pattern followed by `rows(binding_ops=...)` `40.4 ms (38.1–43.6) → 17.4 ms (16.8–19.8)`, **~2.3×**. The `rows()` output is byte-identical between the two arms and the traversal output is value-identical (dtypes differ deliberately, see *Changed*). **Scope, stated plainly**: this is a NATIVE `g.gfql([...named...])` / `g.chain([...named...])` improvement only. It is pandas/cuDF-only, and an instrumented run of the graph-benchmark q1–q9 lane showed the chain fast path is **never consulted** for those Cypher queries — they are served earlier by the Cypher-layer fast paths in `gfql_fast_paths.py`, or hit a separate single-op gate — so **no benchmark cell moves** and no lane currently exercises this. A benchmark lane over the native named-chain surface is required follow-up before any perf claim is made on the board.
2526

2627
### Changed
28+
- **A served chain fast path now returns Cypher-conformant int/bool dtypes on named patterns too**: the pandas full path's rows-pivot upcasts non-id `int64 → float64` and `bool → object` through its merges. That is a pandas merge artifact, not a Cypher semantic — the openCypher TCK (`clauses/return/Return2.feature`, "Returning a node property value": `CREATE ({num: 1})` / `RETURN a.num` expects `1`, and the TCK value grammar writes an Integer as bare decimal digits and a Float with its decimals) and Neo4j (INTEGER and FLOAT are distinct property types; `valueType()` on an integer property reports `INTEGER NOT NULL`) both keep the integer, and this library's own **polars** and **cuDF** canonical paths already return `int64`/`bool` for the same query, so only the pandas merge upcasts. The chain fast path already preserved the conformant dtypes for UNNAMED patterns (documented in its docstring); serving named patterns extends that to the named surface rather than creating a new divergence, and the conformant dtype is deliberately kept rather than cast back to reproduce the defect. **User-visible** where a named pandas pattern is served by the fast path: an integer node property comes back `int64` instead of `float64` (`30` rather than `30.0`, including in JSON), and an alias flag column comes back `bool` instead of `object`. Values are unchanged. **Not aligned yet** (deliberately out of scope, follow-up): the pandas full path itself, and the Cypher-layer seeded projection, still emit the artifact — so pandas can still report `float64` for the same logical query depending on which internal path serves it.
2729
- **`is_lazy` is a type predicate, so the polars engine's eager/lazy split is checked instead of asserted**: `dtypes.is_lazy` decides WHICH member of the two-member `PolarsFrame` union a frame is, but it was declared `-> bool`, so that answer was discarded at the call boundary — an eager-only attribute (`.height`, `.columns`, `.schema`) reached after a lazy guard was unprovable, and the native polars chain closed the gap with `cast("pl.DataFrame", frame)`. A cast is not a type: it re-asserts, unchecked, the exact fact the predicate had just established, once per call site, and a wrong one surfaces as a production `AttributeError` rather than a CI error. `is_lazy` is now declared `-> TypeIs["pl.LazyFrame"]` (PEP 742), which narrows the *negative* branch as well as the positive — the eager side is the one that needed it, so `TypeGuard` would not have done — and the cast is gone. Four previously untyped chain-combine helpers (`_semi`, `_combine_edges`, `_apply_node_names`, and the `_known_empty` guard) now carry real signatures: `_semi`'s two frames share the `PolarsT` TypeVar because polars joins do not mix eagerness, and the two combine helpers take the `_LazyShim` collect-once duck-type they are actually called with. Typing `_apply_node_names` also retired a `getattr(next_step, "edges_empty", None)` — the shim declares that tri-state in `__slots__`, so a typo is now a checker error instead of a silent `None` that would have re-armed the very cardinality gate the guard disarms. Internal only: no runtime behaviour, no public API, and the `TypeIs` import is `TYPE_CHECKING`-only, so no new runtime dependency floor.
2830
- **GFQL execution context is declared rather than attached at runtime**: the private `_gfql_*` per-execution fields (index policy and registry, row-pipeline base graph, carried seed nodes, edge aliases, shortest-path backend, and the indexed-bindings handoff) are now declared on `Plottable` with defaults on `PlotterBase`, instead of being set with `setattr` and read back with `getattr(..., default)`. No public API or behaviour change; internal call sites are typed, and hand-rolled `Plottable` stand-ins must now construct the full context.
2931

graphistry/compute/chain.py

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from .ast import ASTObject, ASTNode, ASTEdge, ASTCall, Direction, from_json as ASTObject_from_json, serialize_binding_ops
1212
from .typing import DataFrameT, SeriesT
1313
from .util import generate_safe_column_name
14-
from .chain_fast_paths import _seeded_typed_hop_pandas_cudf
14+
from .chain_fast_paths import _seeded_typed_hop_pandas_cudf, _tag_fast_path_aliases
1515
from graphistry.compute.validate.validate_schema import validate_chain_schema
1616
from graphistry.compute.gfql.same_path_types import (
1717
WhereComparison,
@@ -822,9 +822,11 @@ def _try_chain_fast_path(
822822
823823
Same node/edge sets + VALUES as the full machinery (trackA_golden + hop/chain
824824
suites); the 1-hop additionally preserves int node dtypes (the full path upcasts
825-
int→float via merge). Gated to unnamed/unqueried nodes + a plain single-hop edge;
826-
filtered-undirected and seeded chains fall through. polars/dask/spark also fall
827-
through (own fast path / lazy semantics)."""
825+
int→float via merge — the merge is the artifact, int is the Cypher-conformant type).
826+
Gated to unqueried nodes + a plain single-hop edge; NAMED ops are served (the alias
827+
flags are reconstructed by `_tag_fast_path_aliases`) except when undirected or when
828+
the same alias is reused. filtered-undirected and seeded chains fall through.
829+
polars/dask/spark also fall through (own fast path / lazy semantics)."""
828830
from graphistry.compute.filter_by_dict import filter_by_dict
829831

830832
if engine_concrete not in (Engine.PANDAS, Engine.CUDF):
@@ -852,13 +854,28 @@ def _materialize_fast_path_graph() -> Plottable:
852854
if len(ops) != 3:
853855
return None
854856
n0, e1, n2 = ops
855-
if not (isinstance(n0, ASTNode) and n0._name is None and n0.query is None):
857+
# Aliases are a PROJECTION concern, not a traversal one: capture them, serve the
858+
# traversal on the fast path, and tag the result (_tag_fast_path_aliases). Rejecting
859+
# them here sent a NAMED `g.gfql([n(name=..), e(..), n(name=..)])` to the full
860+
# two-pass BFS purely because the ops carried names — measured ~25.2 ms before vs
861+
# ~2.3 ms after (medians of 5 paired runs), on a 200-node graph where data work is ~0.
862+
# SCOPE, measured: this does NOT reach the Cypher `MATCH ... RETURN` surface for the
863+
# benchmark shapes. Those are served earlier by `gfql_fast_paths.py` and never consult
864+
# this function at all, so do not attribute a Cypher-surface win to this gate.
865+
alias_n0, alias_e1, alias_n2 = n0._name, e1._name, n2._name
866+
_named = [a for a in (alias_n0, alias_e1, alias_n2) if a is not None]
867+
if len(_named) != len(set(_named)):
868+
# Duplicate alias reuse is an E201 error, and `combine_steps` is what raises it.
869+
# Serving these here would BYPASS that check and silently succeed — decline so the
870+
# full path still errors. (Caught by test_polars_duplicate_alias_declines_like_pandas.)
856871
return None
857-
if not (isinstance(n2, ASTNode) and n2._name is None and n2.query is None):
872+
if not (isinstance(n0, ASTNode) and n0.query is None):
873+
return None
874+
if not (isinstance(n2, ASTNode) and n2.query is None):
858875
return None
859876
if not (isinstance(e1, ASTEdge) and e1.is_simple_single_hop()
860877
and e1.source_node_match is None
861-
and e1.destination_node_match is None and e1._name is None
878+
and e1.destination_node_match is None
862879
and e1.source_node_query is None and e1.destination_node_query is None
863880
and e1.edge_query is None and not e1.include_zero_hop_seed
864881
and not e1.prune_to_endpoints): # prune keeps only the arrival side -> full path
@@ -868,6 +885,11 @@ def _materialize_fast_path_graph() -> Plottable:
868885
# below rather than falling through to the full two-pass machinery. source/dest
869886
# node match + edge_query (richer predicates) still bail above.
870887
direction = e1.direction
888+
if direction == "undirected" and (alias_n0 is not None or alias_n2 is not None):
889+
# An undirected edge makes a node reachable as EITHER endpoint, so "which alias
890+
# does this node carry" is not derivable from the endpoint columns the way it is
891+
# for a directed hop. Decline to the full path rather than guess.
892+
return None
871893
unconstrained = not n0.filter_dict and not n2.filter_dict
872894
if not unconstrained and direction == "undirected":
873895
return None # filtered-undirected (OR of both directions) -> full path
@@ -877,6 +899,20 @@ def _materialize_fast_path_graph() -> Plottable:
877899
src, dst, node = g._source, g._destination, g._node
878900
if src is None or dst is None or node is None:
879901
return None # no edge/node bindings -> can't fast-path; full path handles it
902+
if (alias_n0 is not None or alias_e1 is not None or alias_n2 is not None) \
903+
and direction != "undirected" and g._nodes is not None and g._edges is not None:
904+
# DEFER TO THE INDEX when one would ACTUALLY serve. A resident index is
905+
# policy-controlled and can beat this scan-based path, and it is what serves named
906+
# patterns today, so taking them here would SHADOW it.
907+
# `_resident_seed_indexes` is the right question: it returns None unless BOTH
908+
# indexes are VALID for these exact frames (fingerprint + identity via get_valid).
909+
# A registry-presence / policy check is NOT equivalent — `get_registry` returns a
910+
# non-None registry even when nothing is indexed, so that spelling declines on every
911+
# query and silently turns this whole optimization into a no-op (measured: the win
912+
# went to exactly 0). Re-measure the win, not just the suite, after touching this.
913+
from .chain_fast_paths import _resident_seed_indexes
914+
if _resident_seed_indexes(g, g._nodes, g._edges, node, src, dst, direction) is not None:
915+
return None
880916
concat = df_concat(engine_concrete)
881917
if unconstrained:
882918
# No node filter to reduce by: validate BOTH endpoints against the full
@@ -902,7 +938,8 @@ def _materialize_fast_path_graph() -> Plottable:
902938
if engine_concrete in (Engine.PANDAS, Engine.CUDF):
903939
_fast_res = _seeded_typed_hop_pandas_cudf(g, n0, n2, e1, src, dst, node, direction)
904940
if _fast_res is not None:
905-
return _fast_res
941+
return _tag_fast_path_aliases(
942+
_fast_res, alias_n0, alias_e1, alias_n2, src, dst, node, direction)
906943
from_col, to_col = (src, dst) if direction == "forward" else (dst, src)
907944
edges = g._edges
908945
if n0.filter_dict:
@@ -935,15 +972,17 @@ def _materialize_fast_path_graph() -> Plottable:
935972
edges[[dst]].rename(columns={dst: node}),
936973
]).drop_duplicates()
937974
nodes = cand[cand[node].isin(final[node])]
938-
return g.nodes(nodes).edges(edges)
975+
return _tag_fast_path_aliases(
976+
g.nodes(nodes).edges(edges), alias_n0, alias_e1, alias_n2, src, dst, node, direction)
939977
endpoints = concat([
940978
edges[[src]].rename(columns={src: node}),
941979
edges[[dst]].rename(columns={dst: node}),
942980
]).drop_duplicates()
943981
nodes = g._nodes[g._nodes[node].isin(endpoints[node])]
944982
# match the full path's merge, which collapses duplicate node-id rows
945983
nodes = nodes.drop_duplicates(subset=[node])
946-
return g.nodes(nodes).edges(edges)
984+
return _tag_fast_path_aliases(
985+
g.nodes(nodes).edges(edges), alias_n0, alias_e1, alias_n2, src, dst, node, direction)
947986

948987

949988
@otel_traced("gfql.chain", attrs_fn=_chain_otel_attrs)

graphistry/compute/chain_fast_paths.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
typed 1-hop pandas/cuDF reduction and the seeded typed RETURN-destination pandas/cuDF and
55
polars reductions. Pure code move, no behavior change. chain.py imports from here (one
66
direction); this module imports only leaf modules (no back-edge into chain.py).
7+
8+
New fast-path helpers belong HERE, not in chain.py — that is the direction this module was
9+
created to establish and it is why `_tag_fast_path_aliases` lives alongside the seeded
10+
reductions rather than next to `_try_chain_fast_path`.
711
"""
812
# ruff: noqa: E501
913

@@ -18,6 +22,53 @@
1822
from graphistry.compute.gfql.index.registry import AdjacencyIndex, NodeIdIndex
1923

2024

25+
def _tag_fast_path_aliases(
26+
res: Plottable,
27+
alias_n0: Optional[str], alias_e1: Optional[str], alias_n2: Optional[str],
28+
src: str, dst: str, node: str, direction: Direction,
29+
) -> Plottable:
30+
"""Attach the alias flag columns the full path's ``combine_steps`` would have merged in.
31+
32+
The chain fast path's gate used to reject ANY named op, so a named
33+
`g.gfql([n(name=..), e(..), n(name=..)])` fell to the full two-pass machinery purely
34+
because the ops carried names — measured ~25.2 -> ~2.3 ms (medians of 5 paired runs) on
35+
a 200-node graph where data-proportional work is ~0. Naming is a PROJECTION concern,
36+
not a traversal one, so it should not change which engine path runs. NOTE the scope: this is the NATIVE chain
37+
surface. The Cypher `MATCH ... RETURN` shapes on the graph benchmark are served
38+
earlier by `gfql_fast_paths.py` and never reach here (measured, both engines).
39+
40+
Why deriving the tags from the RETURNED EDGES matches the full path: `combine_steps`
41+
tags a node with an alias iff it matched that step in the BACKWARD-PRUNED frame, i.e.
42+
iff it still participates in a surviving edge. The edges this function receives are
43+
exactly the surviving ones — the fast path has already applied the node filters, the
44+
edge_match and the endpoint validation — so ``isin`` over their endpoint columns is the
45+
same predicate, computed without the join.
46+
47+
A seed whose edges all fail the type filter yields an empty edge frame, so it is tagged
48+
False rather than True: that is the dead-end case, and it is why the tag keys on the
49+
edges rather than on the node filter.
50+
"""
51+
if alias_n0 is None and alias_e1 is None and alias_n2 is None:
52+
return res
53+
nodes: Optional[DataFrameT] = res._nodes
54+
edges: Optional[DataFrameT] = res._edges
55+
if nodes is None or edges is None:
56+
return res
57+
from_col, to_col = (src, dst) if direction == "forward" else (dst, src)
58+
node_flags: Dict[str, SeriesT] = {}
59+
if alias_n0 is not None:
60+
node_flags[alias_n0] = nodes[node].isin(edges[from_col])
61+
if alias_n2 is not None:
62+
node_flags[alias_n2] = nodes[node].isin(edges[to_col])
63+
if node_flags:
64+
nodes = nodes.assign(**node_flags)
65+
if alias_e1 is not None:
66+
edge_flags: Dict[str, bool] = {alias_e1: True}
67+
edges = edges.assign(**edge_flags)
68+
69+
return res.nodes(nodes).edges(edges)
70+
71+
2172
def _seeded_scalar_filters(fd: Optional[Dict[str, Any]], df: DataFrameT) -> Optional[Dict[str, Any]]:
2273
"""Resolve a filter dict to plain scalar column==value pairs, or None to bail
2374
to the general path. Mirrors filter_by_dict.resolve_filter_column exactly for

0 commit comments

Comments
 (0)