diff --git a/CHANGELOG.md b/CHANGELOG.md index 67bde7a5e9..615b1b1cc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,8 +30,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **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. - **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. - **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. +- **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: **alias names that collide with a binding column the tagger would overwrite** (a node alias equal to the node-id binding wrong-served — the flag OVERWROTE the id column while the full path raises — and an edge alias equal to the hop's FROM-side binding returned a DIFFERENT node set per lane; both found by adversarial parity testing and now declined, with decline + raise/parity pins; TO-side collisions keep parity and stay served), **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. ### Changed +- **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. - **The widened residual translator gets its cost back: the lowering is memoized, and the fused lane stops emitting a NaN mask it can prove is dead**. Widening `_residual_polars_expr` to `row_pipeline.lower_single_alias_predicate` (previous entry) turned a regex match into a full parse-and-lower on a path where every board residual was *already* native, and it regressed the matched graph-benchmark q1-q9 lane at both scales — q7 by 1.24x (20k) and 1.16x (100k) with disjoint per-slot ranges, q5 by 1.13x/1.09x also disjoint, q6 slower within overlap; the three cells that moved are exactly the three carrying residuals (`engine='polars'`, RUNS=101 WARMUP=10, 15 position-balanced slots, arms differing only by the pygraphistry tree, base `ed3904515` vs PR `7e9bd662d`). **Two measured mechanisms, two scoped fixes.** (1) *Parse tax*: `lower_single_alias_predicate` re-parsed and re-lowered on every call, at 3.3 us/call before the widening and 162 us/call after it — the cost is not the parse so much as the schema-width `LazyFrame` probe `_expr_output_dtype` runs per operand, and the predicate strings are a tiny fixed set per query. It is now memoized behind a bounded LRU (512 entries, so a long-lived process cannot grow it without limit), at 2.3 us/call. The key is `(expr, alias, columns_nan_free, ((column, dtype-repr), ...))`: names AND dtypes AND their order, because the same predicate over the same column names lowers differently when a dtype changes, and `str(dtype)` rather than the dtype object because polars' `DataType.__eq__` equates a dtype class with its parameterized instances and would HIT across dtypes that lower differently. Parser AVAILABILITY is deliberately NOT in the key — it is process state, not an argument — so it is probed ahead of the cache (~0.2 us), and no entry can outlive a parser that has gone away. (2) *A costlier expression*, which is why the loss grew with scale: the general lowering wraps every float comparison in `& col.is_nan().not()` so NaN compares IEEE/pandas/Cypher-style rather than polars-style (NaN = largest), and that mask took `p.age >= 23` from 0.23 ms to 0.52 ms at 100k. On the connected-join fused lane the mask is provably dead **for a bare column read**: every frame there is a filtered/joined projection of the graph's own `_nodes`, which `_coerce_input_formats` already ran through `nan_clean._pl_nan_to_null` on the way into `gfql()`, and selection/filtering/joining cannot introduce a NaN a column did not hold — the same justification the pre-widening narrow translator gave, and the same one `predicates.filter_expr_by_dict_polars` already relies on. So the lane opts in via a new `lowering_context.COLUMNS_NAN_FREE` contextvar, threaded by an explicit `lower_single_alias_predicate(..., columns_nan_free=True)` that nothing else sets. **The default is guard-ON**, so the general row-table lowering — whose frames have NOT been through gfql ingest — is untouched and any future caller is safe without knowing this exists. **Suppression is scoped to COLUMN REFERENCES, never to computed operands**: `n.a/n.b` is NaN at 0.0/0.0 and `sqrt(n.x)` is NaN at x<0 even on a perfectly clean column, so in-query float math manufactures NaN no ingest can have removed and keeps the mask on both lanes; a float LITERAL keeps its own (constant-false) term too, since only column reads are in scope. That restores exactly the guard-free set of the pre-widening narrow matcher. **Result**: q7 returns to parity with base — TIE at 1.02x at both scales, from 1.24x/1.16x — and q5 (0.95x/0.99x) and q6 (0.98x/1.01x) return to TIE as well, with no other cell moved and every cell's VALUE identical across all three arms at both scales. Tests pin the scoping in both directions: the fused lane emits a guard-free expression, the general row-table lowering still emits the mask, a computed operand keeps it and is answered IEEE-style over a real in-query 0.0/0.0, a natively-built polars frame carrying a genuine NaN is shown normalized by ingest before the lane ever sees it, and the memo is shown to agree with the uncached lowering on every shape while a dtype change alone produces a different expression. - **The native polars residual translator covers the whole single-alias predicate vocabulary the connected-join WHERE renderer can emit, because it now delegates to the row evaluator's own lowering instead of re-deriving it**: `_residual_polars_expr` (`graphistry/compute/gfql_fast_paths.py`) recognized exactly two hand-written regex shapes — `((a.col) = 'lit')` and `(a.col literal)` for `= >= <= > <` — and returned `None` for everything else, and a single `None` drops the entire fused single-collect connected-join plan and re-evaluates that alias's residual through a `where_rows` chain. It now parses the residual with the row-expression parser, rewrites `alias.col` to the bare column its own frame actually carries, and lowers it with `lower_expr` through the new `row_pipeline.lower_single_alias_predicate`. **Parity is by construction, not by re-derivation.** The fallback being replaced is `where_rows_polars`, which is that same parser plus that same `lower_expr` under the same `_SCHEMA` dtypes followed by `table.filter(...)`; `alias.col ↔ col` is a bijection over the one frame involved; so the fast lane builds the expression the fallback would build and applies it the way the fallback would apply it. The accept set, the decline set and the answers are the evaluator's, and the dtype, temporal-literal, int-literal-division and float-NaN guards are inherited rather than restated, so the fast lane cannot drift from the evaluator when those guards change. Newly covered — each pinned by a differential against the forced chain fallback on the same frame AND by the expected rows, so a differential both sides get wrong cannot pass as parity: `!=` and `<>` (including the sharp edge, 3-valued NULL: a NULL operand yields NULL and `filter` DROPS the row, exactly as for `=`, where a pandas-shaped object-dtype `!=` would have kept it), `IS NULL`, `IS NOT NULL`, `IN [literals]`, `OR`, `NOT`, nested boolean combinations, `CASE WHEN`, arithmetic, and the row lowering's function whitelist (`substring`, `size`, …). Three former declines are retired as unnecessary rather than preserved: the **escaped-literal** decline (`'it\u0027s'`, `'C:\u005Cx'`) is gone because the literal is now unescaped by the evaluator's own parser instead of compared as raw regex-captured text, and is replaced by a non-vacuous differential over rows that really contain a quote and a backslash; the purely **layout** rejections (missing outer parens, reversed operand order, a function on the column side) are gone because nothing is matched by text shape any more; and `=`/`!=`/`IS NULL`/`IN` on a **Categorical** column translate now, since those are not `.str` kernels and the evaluator answers them (`toLower`/`toUpper` on Categorical still declines, because `.str` on Categorical raises in polars only). Every remaining decline is a shape the fallback declines too, so the row op's designed parity-or-error `NotImplementedError` still surfaces instead of a raw polars `ComputeError`: a property access on another alias, a bare identifier (including the whole-entity identity sentinel, which resolves only through the row table's `_NODE_ID`), an absent column, a numeric literal against a String column and the converse, a case function on a non-String column, and any node type outside the row lowering's own whitelist (map/subscript/slice/quantifier/comprehension). **`STARTS WITH` / `ENDS WITH` / `CONTAINS` / `=~` are deliberately NOT covered, and the reason is reachability, not difficulty**: `_pushdown_connected_join_where_filters` cannot render those to a row filter, so on the polars engine the whole comma-pattern query is rejected upstream with `GFQLValidationError` and no such residual ever reaches this translator — pinned by a test that spies the translator and asserts it is never called. Teaching it those shapes would be unreachable code; the real gap is in the connected-join WHERE renderer, where pandas answers those queries today and polars does not. **The justification is the vocabulary the Cypher front end already accepts, not a number** — and the number, once measured, was negative: the board's ten residual translations were all ALREADY native under the narrow matcher, so the widening converted no decline there and only added cost, regressing q7 by 1.24x/1.16x (20k/100k, non-overlapping per-slot ranges). Both mechanisms and the fix are the next entry; the widened COVERAGE is unaffected by it. - **`is_polars_df` is a type predicate, and the engine-frame vocabulary has a canonical home**: `graphistry/compute/typing.py` now exports `PolarsFrame` (the `pl.DataFrame | pl.LazyFrame` union), a `PolarsT` TypeVar and `PolarsSeriesT`, TYPE_CHECKING-only so polars stays an optional dependency and no runtime import or version floor is added; `polars/dtypes.py`, which had defined the first two, re-exports them so there is ONE definition. `DataFrameT` is deliberately left pinned to `pd.DataFrame` — widening it into a cross-engine union fans out across every checked module, and the fix for a polars-only helper is to name polars, not to blur the pandas alias. Against that vocabulary `Engine.is_polars_df` is now declared `(df: object) -> TypeGuard["PolarsFrame"]` instead of `(df: Any) -> bool`, so the polars branch of an engine-dispatching helper is actually CHECKED against the polars API rather than silently accepted. `TypeGuard`, not `TypeIs` (PEP 742): `TypeIs` also narrows the negative branch, but to do that soundly it requires the narrowed type to be consistent with the declared input, and a polars frame is not a subtype of the pandas type these call sites declare — that is the difference from `dtypes.is_lazy`, whose input is already a `PolarsFrame`. A companion `is_polars_series` (the same import-light module check) narrows the SERIES call sites to `pl.Series`, which the frame guard would have mistyped. Consequences: the six per-branch `# type: ignore` comments in `Engine.py`'s dispatch block were written against an older stub set and named codes (`attr-defined`, `no-any-return`) that no longer applied — they were dead, suppressing nothing, and are now scoped to the one thing that genuinely cannot typecheck, the polars return value flowing out of a `DataFrameT`/`SeriesT` signature; `_pl_nan_to_null` takes the `PolarsFrame` union instead of the constrained `PolarsT` TypeVar, which a union argument cannot bind (an `@overload` set would keep the per-flavour precision but is unusable here — on the polars-less type-lint lane every signature collapses to `Any -> Any`); and two rebindings that mixed a polars result into a pandas-typed local (`gfql/row/frame_ops.py`, `gfql_unified.py`) return or branch directly instead. Typing only: no runtime behaviour, no public API, no new dependency. Measured on a stub-visible mypy lane with polars importable, repo-wide errors go 163 -> 157 (11 -> 9 files); the lane CI actually runs today is unaffected and still reports no issues. diff --git a/graphistry/compute/chain.py b/graphistry/compute/chain.py index 32fe1020f0..bcb2093d41 100644 --- a/graphistry/compute/chain.py +++ b/graphistry/compute/chain.py @@ -11,7 +11,7 @@ from .ast import ASTObject, ASTNode, ASTEdge, ASTCall, Direction, from_json as ASTObject_from_json, serialize_binding_ops from .typing import DataFrameT, SeriesT from .util import generate_safe_column_name -from .chain_fast_paths import _seeded_typed_hop_pandas_cudf +from .chain_fast_paths import _seeded_typed_hop_pandas_cudf, _tag_fast_path_aliases from graphistry.compute.validate.validate_schema import validate_chain_schema from graphistry.compute.gfql.same_path_types import ( WhereComparison, @@ -822,9 +822,11 @@ def _try_chain_fast_path( Same node/edge sets + VALUES as the full machinery (trackA_golden + hop/chain suites); the 1-hop additionally preserves int node dtypes (the full path upcasts - int→float via merge). Gated to unnamed/unqueried nodes + a plain single-hop edge; - filtered-undirected and seeded chains fall through. polars/dask/spark also fall - through (own fast path / lazy semantics).""" + int→float via merge — the merge is the artifact, int is the Cypher-conformant type). + Gated to unqueried nodes + a plain single-hop edge; NAMED ops are served (the alias + flags are reconstructed by `_tag_fast_path_aliases`) except when undirected or when + the same alias is reused. filtered-undirected and seeded chains fall through. + polars/dask/spark also fall through (own fast path / lazy semantics).""" from graphistry.compute.filter_by_dict import filter_by_dict if engine_concrete not in (Engine.PANDAS, Engine.CUDF): @@ -852,13 +854,28 @@ def _materialize_fast_path_graph() -> Plottable: if len(ops) != 3: return None n0, e1, n2 = ops - if not (isinstance(n0, ASTNode) and n0._name is None and n0.query is None): + # Aliases are a PROJECTION concern, not a traversal one: capture them, serve the + # traversal on the fast path, and tag the result (_tag_fast_path_aliases). Rejecting + # them here sent a NAMED `g.gfql([n(name=..), e(..), n(name=..)])` to the full + # two-pass BFS purely because the ops carried names — measured ~25.2 ms before vs + # ~2.3 ms after (medians of 5 paired runs), on a 200-node graph where data work is ~0. + # SCOPE, measured: this does NOT reach the Cypher `MATCH ... RETURN` surface for the + # benchmark shapes. Those are served earlier by `gfql_fast_paths.py` and never consult + # this function at all, so do not attribute a Cypher-surface win to this gate. + alias_n0, alias_e1, alias_n2 = n0._name, e1._name, n2._name + _named = [a for a in (alias_n0, alias_e1, alias_n2) if a is not None] + if len(_named) != len(set(_named)): + # Duplicate alias reuse is an E201 error, and `combine_steps` is what raises it. + # Serving these here would BYPASS that check and silently succeed — decline so the + # full path still errors. (Caught by test_polars_duplicate_alias_declines_like_pandas.) return None - if not (isinstance(n2, ASTNode) and n2._name is None and n2.query is None): + if not (isinstance(n0, ASTNode) and n0.query is None): + return None + if not (isinstance(n2, ASTNode) and n2.query is None): return None if not (isinstance(e1, ASTEdge) and e1.is_simple_single_hop() and e1.source_node_match is None - and e1.destination_node_match is None and e1._name is None + and e1.destination_node_match is None and e1.source_node_query is None and e1.destination_node_query is None and e1.edge_query is None and not e1.include_zero_hop_seed and not e1.prune_to_endpoints): # prune keeps only the arrival side -> full path @@ -868,6 +885,11 @@ def _materialize_fast_path_graph() -> Plottable: # below rather than falling through to the full two-pass machinery. source/dest # node match + edge_query (richer predicates) still bail above. direction = e1.direction + if direction == "undirected" and (alias_n0 is not None or alias_n2 is not None): + # An undirected edge makes a node reachable as EITHER endpoint, so "which alias + # does this node carry" is not derivable from the endpoint columns the way it is + # for a directed hop. Decline to the full path rather than guess. + return None unconstrained = not n0.filter_dict and not n2.filter_dict if not unconstrained and direction == "undirected": return None # filtered-undirected (OR of both directions) -> full path @@ -877,6 +899,33 @@ def _materialize_fast_path_graph() -> Plottable: src, dst, node = g._source, g._destination, g._node if src is None or dst is None or node is None: return None # no edge/node bindings -> can't fast-path; full path handles it + if alias_n0 == node or alias_n2 == node: + # A node alias EQUAL TO THE NODE-ID BINDING would make `_tag_fast_path_aliases` + # overwrite the id column with the bool flag (destroying the ids) while the full + # path raises on the same query ("The column label '' is not unique") — + # a wrong-serve found by adversarial parity testing. Decline; never serve. + return None + if alias_e1 is not None and direction in ("forward", "reverse") \ + and alias_e1 == (src if direction == "forward" else dst): + # An edge alias EQUAL TO THE HOP'S FROM-SIDE BINDING (forward+src / reverse+dst) + # made the two lanes return DIFFERENT node sets: the full path's flag overwrite + # corrupts its own node reduction, the fast path tags after reducing. TO-side + # collisions keep parity (pinned in tests) and stay served. Decline; never serve. + return None + if (alias_n0 is not None or alias_e1 is not None or alias_n2 is not None) \ + and direction != "undirected" and g._nodes is not None and g._edges is not None: + # DEFER TO THE INDEX when one would ACTUALLY serve. A resident index is + # policy-controlled and can beat this scan-based path, and it is what serves named + # patterns today, so taking them here would SHADOW it. + # `_resident_seed_indexes` is the right question: it returns None unless BOTH + # indexes are VALID for these exact frames (fingerprint + identity via get_valid). + # A registry-presence / policy check is NOT equivalent — `get_registry` returns a + # non-None registry even when nothing is indexed, so that spelling declines on every + # query and silently turns this whole optimization into a no-op (measured: the win + # went to exactly 0). Re-measure the win, not just the suite, after touching this. + from .chain_fast_paths import _resident_seed_indexes + if _resident_seed_indexes(g, g._nodes, g._edges, node, src, dst, direction) is not None: + return None concat = df_concat(engine_concrete) if unconstrained: # No node filter to reduce by: validate BOTH endpoints against the full @@ -902,7 +951,8 @@ def _materialize_fast_path_graph() -> Plottable: if engine_concrete in (Engine.PANDAS, Engine.CUDF): _fast_res = _seeded_typed_hop_pandas_cudf(g, n0, n2, e1, src, dst, node, direction) if _fast_res is not None: - return _fast_res + return _tag_fast_path_aliases( + _fast_res, alias_n0, alias_e1, alias_n2, src, dst, node, direction) from_col, to_col = (src, dst) if direction == "forward" else (dst, src) edges = g._edges if n0.filter_dict: @@ -935,7 +985,8 @@ def _materialize_fast_path_graph() -> Plottable: edges[[dst]].rename(columns={dst: node}), ]).drop_duplicates() nodes = cand[cand[node].isin(final[node])] - return g.nodes(nodes).edges(edges) + return _tag_fast_path_aliases( + g.nodes(nodes).edges(edges), alias_n0, alias_e1, alias_n2, src, dst, node, direction) endpoints = concat([ edges[[src]].rename(columns={src: node}), edges[[dst]].rename(columns={dst: node}), @@ -943,7 +994,8 @@ def _materialize_fast_path_graph() -> Plottable: nodes = g._nodes[g._nodes[node].isin(endpoints[node])] # match the full path's merge, which collapses duplicate node-id rows nodes = nodes.drop_duplicates(subset=[node]) - return g.nodes(nodes).edges(edges) + return _tag_fast_path_aliases( + g.nodes(nodes).edges(edges), alias_n0, alias_e1, alias_n2, src, dst, node, direction) @otel_traced("gfql.chain", attrs_fn=_chain_otel_attrs) diff --git a/graphistry/compute/chain_fast_paths.py b/graphistry/compute/chain_fast_paths.py index 45dd638b64..c4ef0e7869 100644 --- a/graphistry/compute/chain_fast_paths.py +++ b/graphistry/compute/chain_fast_paths.py @@ -4,6 +4,10 @@ typed 1-hop pandas/cuDF reduction and the seeded typed RETURN-destination pandas/cuDF and polars reductions. Pure code move, no behavior change. chain.py imports from here (one direction); this module imports only leaf modules (no back-edge into chain.py). + +New fast-path helpers belong HERE, not in chain.py — that is the direction this module was +created to establish and it is why `_tag_fast_path_aliases` lives alongside the seeded +reductions rather than next to `_try_chain_fast_path`. """ # ruff: noqa: E501 @@ -18,6 +22,53 @@ from graphistry.compute.gfql.index.registry import AdjacencyIndex, NodeIdIndex +def _tag_fast_path_aliases( + res: Plottable, + alias_n0: Optional[str], alias_e1: Optional[str], alias_n2: Optional[str], + src: str, dst: str, node: str, direction: Direction, +) -> Plottable: + """Attach the alias flag columns the full path's ``combine_steps`` would have merged in. + + The chain fast path's gate used to reject ANY named op, so a named + `g.gfql([n(name=..), e(..), n(name=..)])` fell to the full two-pass machinery purely + because the ops carried names — measured ~25.2 -> ~2.3 ms (medians of 5 paired runs) on + a 200-node graph where data-proportional work is ~0. Naming is a PROJECTION concern, + not a traversal one, so it should not change which engine path runs. NOTE the scope: this is the NATIVE chain + surface. The Cypher `MATCH ... RETURN` shapes on the graph benchmark are served + earlier by `gfql_fast_paths.py` and never reach here (measured, both engines). + + Why deriving the tags from the RETURNED EDGES matches the full path: `combine_steps` + tags a node with an alias iff it matched that step in the BACKWARD-PRUNED frame, i.e. + iff it still participates in a surviving edge. The edges this function receives are + exactly the surviving ones — the fast path has already applied the node filters, the + edge_match and the endpoint validation — so ``isin`` over their endpoint columns is the + same predicate, computed without the join. + + A seed whose edges all fail the type filter yields an empty edge frame, so it is tagged + False rather than True: that is the dead-end case, and it is why the tag keys on the + edges rather than on the node filter. + """ + if alias_n0 is None and alias_e1 is None and alias_n2 is None: + return res + nodes: Optional[DataFrameT] = res._nodes + edges: Optional[DataFrameT] = res._edges + if nodes is None or edges is None: + return res + from_col, to_col = (src, dst) if direction == "forward" else (dst, src) + node_flags: Dict[str, SeriesT] = {} + if alias_n0 is not None: + node_flags[alias_n0] = nodes[node].isin(edges[from_col]) + if alias_n2 is not None: + node_flags[alias_n2] = nodes[node].isin(edges[to_col]) + if node_flags: + nodes = nodes.assign(**node_flags) + if alias_e1 is not None: + edge_flags: Dict[str, bool] = {alias_e1: True} + edges = edges.assign(**edge_flags) + + return res.nodes(nodes).edges(edges) + + def _seeded_scalar_filters(fd: Optional[Dict[str, Any]], df: DataFrameT) -> Optional[Dict[str, Any]]: """Resolve a filter dict to plain scalar column==value pairs, or None to bail to the general path. Mirrors filter_by_dict.resolve_filter_column exactly for diff --git a/graphistry/tests/compute/gfql/index/test_indexed_bindings.py b/graphistry/tests/compute/gfql/index/test_indexed_bindings.py index 2c44c25ce5..40e2745527 100644 --- a/graphistry/tests/compute/gfql/index/test_indexed_bindings.py +++ b/graphistry/tests/compute/gfql/index/test_indexed_bindings.py @@ -6,7 +6,7 @@ """ from __future__ import annotations -from typing import Any, Dict, List, Sequence, Tuple, Type +from typing import Any, Dict, List, Optional, Sequence, Tuple, Type import numpy as np import pandas as pd @@ -14,7 +14,8 @@ import graphistry from graphistry.Engine import Engine -from graphistry.compute.ast import ASTEdge, e_forward, e_undirected, n +from graphistry.Plottable import Plottable +from graphistry.compute.ast import ASTEdge, ASTObject, e_forward, e_undirected, n ENGINES = ["pandas", "polars", "cudf"] @@ -148,6 +149,7 @@ def _run( if generic: import graphistry.compute.gfql.index.bindings as indexed_bindings import graphistry.compute.gfql_unified as gfql_unified + import graphistry.compute.chain as chain_mod monkeypatch.setattr( indexed_bindings, @@ -159,6 +161,30 @@ def _run( "_execute_seeded_typed_hop_fast_path", lambda *args, **kwargs: None, ) + # `generic` enumerates the acceleration seams to disable. The chain fast path now + # serves NAMED patterns (it previously rejected any alias, which is why it was + # absent here), so it has to be listed — otherwise the comparison stops being + # generic-vs-accelerated and becomes accelerated-vs-a-DIFFERENT-accelerated. + # + # But disable ONLY the NEW capability, not the seam. This path has always served + # UNNAMED middles, and `test_unnamed_middle_rows_call_matches_canonical` compares + # exactly that shape — blanket-disabling turns that case from fast-vs-fast into + # general-vs-fast and it fails for a reason that has nothing to do with what it + # tests. Verified: master's product code plus a blanket patch reproduces that + # failure on its own. + _real_fast_path = chain_mod._try_chain_fast_path + + def _fast_path_without_named( + g_in: Plottable, + ops: List[ASTObject], + engine_concrete: Engine, + start_nodes: Optional[pd.DataFrame] = None, + ) -> Optional[Plottable]: + if any(op._name is not None for op in ops): + return None + return _real_fast_path(g_in, ops, engine_concrete, start_nodes) + + monkeypatch.setattr(chain_mod, "_try_chain_fast_path", _fast_path_without_named) return g.gfql( query, engine=engine, diff --git a/graphistry/tests/compute/gfql/test_seeded_typed_hop_fastpath.py b/graphistry/tests/compute/gfql/test_seeded_typed_hop_fastpath.py index b57908b597..abccc0491f 100644 --- a/graphistry/tests/compute/gfql/test_seeded_typed_hop_fastpath.py +++ b/graphistry/tests/compute/gfql/test_seeded_typed_hop_fastpath.py @@ -13,11 +13,14 @@ including full-path side-channels (policy hooks, same-path WHERE, OPTIONAL null rows, WITH..MATCH carried seeds, list-`labels` columns, null ids). """ +from typing import Dict, Tuple + import numpy as np import pandas as pd import pytest import graphistry +from graphistry.Plottable import Plottable from graphistry.compute.ast import n, e_forward, e_reverse import graphistry.compute.chain as chain_mod import graphistry.compute.gfql_unified as gfql_unified @@ -726,6 +729,49 @@ def _pl_graph(self): return graphistry.nodes(pl.from_pandas(g._nodes), "id").edges( pl.from_pandas(g._edges), "src", "dst") + # ------------------------------------------------------------------ + # THE PANDAS FULL PATH'S int64->float64 / bool->object UPCAST IS AN ARTIFACT. + # + # It comes from the rows-pivot's merges, not from Cypher. Evidence, three independent + # sources (this is a DELIBERATE rule, not a test "fix"): + # 1. openCypher TCK `tck/features/clauses/return/Return2.feature` scenario [2] + # "Returning a node property value": `CREATE ({num: 1})` / `MATCH (a) RETURN a.num` + # expects `1`. The TCK value grammar (`tck/README.adoc`) writes an Integer as a + # bare string of decimal digits and a Float "in decimal form with all present + # decimals", so `1.0` would be a DIFFERENT expected value. + # 2. Neo4j (reference engine) Cypher manual: INTEGER and FLOAT are distinct property + # types and an INTEGER property read back is INTEGER + # (`valueType(n.prop)` -> "INTEGER NOT NULL"). + # 3. This repo's own POLARS full path already returns int64/bool for this exact + # query — only the pandas merge upcasts. So the upcast is engine-local, which is + # what an artifact looks like and what a semantic does not. + # => int64/bool is the CONFORMANT result. Where a fast path serves, it keeps the + # conformant dtype rather than casting back to reproduce the defect (casting back was + # also measured to be per-column and unprincipled: a blanket cast took the suite from + # 20 to 45 failures). + # NOT YET ALIGNED (deliberately out of scope, tracked as follow-up): the pandas full + # path itself, and the Cypher-layer projection pinned by + # `test_pandas_int_bool_dtype_parity` below, still emit the artifact. + _FULL_PATH_PANDAS_UPCASTS: Dict[str, Tuple[str, str]] = { + "a": ("int64", "float64"), "f": ("bool", "object")} + + def _assert_values_equal_conformant_dtypes(self, fast: Plottable, full: Plottable) -> None: + """Values identical; where the pandas full path upcast, the served path keeps the + Cypher-conformant dtype and we assert BOTH sides explicitly.""" + f, u = _canon_nodes(fast), _canon_nodes(full) + assert list(f.columns) == list(u.columns) + for col in f.columns: + rule = self._FULL_PATH_PANDAS_UPCASTS.get(col) + if rule is not None and str(u[col].dtype) == rule[1]: + conformant, artifact = rule + assert str(f[col].dtype) == conformant, ( + f"{col}: served path must keep the conformant dtype " + f"{conformant}, got {f[col].dtype}") + pd.testing.assert_series_equal( + f[col].astype(artifact), u[col], check_names=False) + else: + pd.testing.assert_series_equal(f[col], u[col], check_names=False) + def _fast_and_full(self, g, engine, q, expect_engage=True): hits = {"n": 0} real = gfql_unified._execute_seeded_typed_hop_fast_path @@ -756,9 +802,16 @@ def test_polars_int_bool_dtype_parity(self): pd.testing.assert_frame_equal(_canon_nodes(fast), _canon_nodes(full)) def test_pandas_datetime_property_declines(self): + """M2/dtype pin: the CYPHER seeded projection still DECLINES a datetime property. + DELIBERATE CHANGE: the assertion used to be a plain frame_equal against the full + path, which incidentally locked the pandas merge upcast (`a` as float64). That was + never the guarantee this test exists for — the guarantee is "the Cypher fast path + declines, and the answer is still right". Since the native chain fast path now + serves the declined shape, `a` comes back int64, which is the conformant type + (see _FULL_PATH_PANDAS_UPCASTS above). Values are still asserted identical.""" q = self.Q.replace("p.flag AS f", "p.ts AS t") fast, full = self._fast_and_full(self._typed_graph(), "pandas", q, expect_engage=False) - pd.testing.assert_frame_equal(_canon_nodes(fast), _canon_nodes(full)) + self._assert_values_equal_conformant_dtypes(fast, full) @pytest.mark.parametrize("engine", ["pandas", "polars"]) def test_edges_empty_frame_not_none(self, engine): @@ -772,11 +825,18 @@ def test_edges_empty_frame_not_none(self, engine): assert full._edges.shape[0] == 0 def test_engine_mismatch_declines(self): + """M2 pin: a requested-vs-actual engine mismatch DECLINES the Cypher seeded + projection. DELIBERATE CHANGE, same reason as test_pandas_datetime_property_ + declines: the first arm's frame_equal incidentally locked the pandas merge upcast. + The declined shape is now served by the native chain fast path, so `a`/`f` come + back int64/bool — the conformant types. Note arm 2 needs no change at all: the + POLARS full path already returns int64/bool, which is the third piece of evidence + that the pandas float64 is engine-local artifact rather than Cypher semantics.""" pytest.importorskip("polars") # polars frames + engine='pandas': full converts to pandas; fast must decline fast, full = self._fast_and_full(self._pl_graph(), "pandas", self.Q, expect_engage=False) assert type(fast._nodes).__module__.startswith("pandas") - pd.testing.assert_frame_equal(_canon_nodes(fast), _canon_nodes(full)) + self._assert_values_equal_conformant_dtypes(fast, full) # pandas frames + engine='polars' (reentry direction): also declines fast2, full2 = self._fast_and_full(self._typed_graph(), "polars", self.Q, expect_engage=False) assert type(fast2._nodes).__module__ == type(full2._nodes).__module__ diff --git a/graphistry/tests/compute/test_chain.py b/graphistry/tests/compute/test_chain.py index 1f084583c5..766e0d8ae3 100644 --- a/graphistry/tests/compute/test_chain.py +++ b/graphistry/tests/compute/test_chain.py @@ -1,9 +1,12 @@ import os +from typing import Callable, List, Sequence, Tuple + import pandas as pd import pytest -from graphistry.compute.ast import ASTEdgeUndirected, ASTNode, ASTEdge, n, e, e_undirected, e_forward, e_reverse +from graphistry.compute.ast import ASTEdgeUndirected, ASTNode, ASTEdge, ASTObject, n, e, e_undirected, e_forward, e_reverse from graphistry.compute.chain import Chain, _try_chain_fast_path +from graphistry.compute.typing import DataFrameT from graphistry.compute.predicates.is_in import IsIn, is_in from graphistry.compute.predicates.numeric import gt from graphistry.tests.test_compute import CGFull @@ -664,7 +667,7 @@ def topd(df): # shapes that ARE accelerated by the fast path -_FAST_SHAPES = [ +_FAST_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ ("node_only", lambda: [n()]), ("node_filter", lambda: [n({'attr': 20})]), ("node_pred", lambda: [n({'attr': is_in([10, 30])})]), @@ -680,13 +683,27 @@ def topd(df): ("edge_match_unconstrained", lambda: [n(), e_forward(hops=1, edge_match={'w': 5}), n()]), ("edge_match_seeded", lambda: [n({'attr': 10}), e_forward(hops=1, edge_match={'w': 5}), n()]), ("edge_match_dst_filter", lambda: [n(), e_forward(hops=1, edge_match={'w': 5}), n({'attr': 30})]), + # DELIBERATE RULE CHANGE: naming an op is a PROJECTION concern, not a traversal one, + # so it no longer decides which engine path runs. `_tag_fast_path_aliases` reconstructs + # the alias flag columns `combine_steps` would have merged in, so these are now FAST + # shapes. (Was `("named_node", ...)` under _BYPASS_SHAPES: that entry encoded the old + # "any alias -> decline" gate, not a semantic guarantee.) + ("named_src", lambda: [n(name='x'), e_forward(hops=1), n()]), + ("named_dst", lambda: [n(), e_forward(hops=1), n(name='y')]), + ("named_edge", lambda: [n(), e_forward(hops=1, name='r'), n()]), + ("named_all_fwd", lambda: [n(name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("named_all_rev", lambda: [n(name='x'), e_reverse(hops=1, name='r'), n(name='y')]), + ("named_filtered", lambda: [n({'attr': 10}, name='x'), e_forward(hops=1), n(name='y')]), ] # shapes that BYPASS the fast path (still must be correct via the full path) -_BYPASS_SHAPES = [ +_BYPASS_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ ("hops_2", lambda: [n(), e_forward(hops=2), n()]), ("filtered_undirected", lambda: [n({'attr': 10}), e_undirected(hops=1), n({'attr': 30})]), - ("named_node", lambda: [n(name='x'), e_forward(hops=1), n()]), + # Named + undirected STAYS a bypass: an undirected edge makes a node reachable as + # EITHER endpoint, so "which alias does this node carry" is not derivable from the + # endpoint columns the way it is for a directed hop. + ("named_undirected", lambda: [n(name='x'), e_undirected(hops=1), n(name='y')]), # prune_to_endpoints: fast path returns both endpoints; full path keeps only the # arrival side. Must bypass the fast path (regression guard for the prune gate). ("prune_endpoints_fwd", lambda: [n(), e_forward(hops=1, prune_to_endpoints=True), n()]), @@ -718,6 +735,361 @@ def test_fast_path_differential_parity_vs_full_path(engine, label, build): f"{label}: bypass shape must decline the fast path" +# Named shapes whose ALIAS FLAG COLUMNS (not merely node/edge sets) must match the full +# path. `_setsig` above compares ids only, so it cannot see a wrong alias tag. +_NAMED_ALIAS_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ + ("src_only", lambda: [n(name='x'), e_forward(hops=1), n()]), + ("dst_only", lambda: [n(), e_forward(hops=1), n(name='y')]), + ("edge_only", lambda: [n(), e_forward(hops=1, name='r'), n()]), + ("all_forward", lambda: [n(name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("all_reverse", lambda: [n(name='x'), e_reverse(hops=1, name='r'), n(name='y')]), + ("src_filtered", lambda: [n({'attr': 10}, name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("dst_filtered", lambda: [n(name='x'), e_forward(hops=1, name='r'), n({'attr': 30}, name='y')]), + ("edge_match", lambda: [n(name='x'), e_forward(hops=1, edge_match={'w': 5}, name='r'), n(name='y')]), + # DEAD END: attr==50 is node 4, which has no outgoing edge. The tag keys on the + # SURVIVING EDGES, so the alias must come back False/empty rather than True. + ("dead_end_seed", lambda: [n({'attr': 50}, name='x'), e_forward(hops=1, name='r'), n(name='y')]), +] + + +@pytest.mark.parametrize("engine", ["pandas", "cudf"]) +@pytest.mark.parametrize("label,build", _NAMED_ALIAS_SHAPES, + ids=[s[0] for s in _NAMED_ALIAS_SHAPES]) +def test_fast_path_named_alias_columns_match_full_path(engine, label, build): + """The capability this fast-path extension actually adds: when the traversal is + served without the BFS, the alias flag columns `combine_steps` would have merged in + are RECONSTRUCTED from the surviving edges, and must be identical to the full path's + — column set, per-row values and all. Serve-asserted so a gate regression that + declines everything fails here instead of passing vacuously.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + g = _fast_graph(engine) + if engine == "pandas": + assert _try_chain_fast_path(g, build(), Engine.PANDAS, None) is not None, \ + f"{label}: named shape must be SERVED by the fast path" + fast = g.gfql(build()) + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + + def flags(df: DataFrameT, key_cols: Sequence[str]) -> pd.DataFrame: + # cuDF frames satisfy DataFrameT structurally but only they carry .to_pandas() + pdf = pd.DataFrame(df.to_pandas() if "cudf" in type(df).__module__ else df) # type: ignore[attr-defined] + alias_cols = sorted(set(pdf.columns) & {'x', 'y', 'r'}) + cols = list(key_cols) + alias_cols + out = pdf[cols].sort_values(cols).reset_index(drop=True) + # bool vs object is the same merge artifact as int64 vs float64 (see + # test_fast_path_preserves_int_node_dtypes); this test pins the alias VALUES, + # and the dtype rule is pinned separately. + for c in alias_cols: + out[c] = out[c].astype(bool) + return out + + pd.testing.assert_frame_equal(flags(fast._nodes, ['v']), flags(full._nodes, ['v'])) + pd.testing.assert_frame_equal(flags(fast._edges, ['s', 'd']), flags(full._edges, ['s', 'd'])) + + +def _norm_all_cols(df: DataFrameT, key_cols: Sequence[str]) -> pd.DataFrame: + """Full-frame canonicalization: ALL columns, sorted column order, key-sorted rows.""" + # cuDF frames satisfy DataFrameT structurally but only they carry .to_pandas() + pdf = pd.DataFrame(df.to_pandas() if "cudf" in type(df).__module__ else df) # type: ignore[attr-defined] + cols = sorted(pdf.columns) + return pdf[cols].sort_values(list(key_cols)).reset_index(drop=True) + + +def _assert_full_frame_value_parity(fast: DataFrameT, full: DataFrameT, + key_cols: Sequence[str]) -> None: + """Same columns, same per-row VALUES on every column. Dtype is deliberately NOT + compared: the served lane keeps the Cypher-conformant int64/bool where the full + path's merges upcast to float64/object (pinned separately).""" + f, u = _norm_all_cols(fast, key_cols), _norm_all_cols(full, key_cols) + assert list(f.columns) == list(u.columns), f"column sets differ: {list(f.columns)} vs {list(u.columns)}" + assert len(f) == len(u), f"row counts differ: {len(f)} vs {len(u)}" + for c in f.columns: + pd.testing.assert_series_equal( + f[c], u[c], check_names=False, check_dtype=False, check_categorical=False) + + +# Named served shapes for FULL-FRAME parity. `_setsig` compares id sets and the flags +# test compares alias columns, so before this NO test compared the carried DATA columns +# ('attr', 'w') of a named served result against the full path. +_NAMED_VALUE_PARITY_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ + ("all_forward", lambda: [n(name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("all_reverse", lambda: [n(name='x'), e_reverse(hops=1, name='r'), n(name='y')]), + ("seed_filtered", lambda: [n({'attr': 10}, name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("edge_match", lambda: [n(name='x'), e_forward(hops=1, edge_match={'w': 5}, name='r'), n(name='y')]), +] + + +@pytest.mark.parametrize("engine", ["pandas", "cudf"]) +@pytest.mark.parametrize("label,build", _NAMED_VALUE_PARITY_SHAPES, + ids=[s[0] for s in _NAMED_VALUE_PARITY_SHAPES]) +def test_fast_path_named_full_frame_value_parity(engine, label, build): + """POSITIVE, whole-frame: a named served result must carry the same VALUES as the + full path on EVERY column — ids, data columns, and alias flags — not just the id + sets and flag columns the other tests pin. Also pins the served lane's documented + dtype promises: data ints stay int64 and alias flags are real bools.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + g = _fast_graph(engine) + if engine == "pandas": + assert _try_chain_fast_path(g, build(), Engine.PANDAS, None) is not None, \ + f"{label}: named shape must be SERVED by the fast path" + fast = g.gfql(build()) + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + _assert_full_frame_value_parity(fast._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(fast._edges, full._edges, ['s', 'd']) + # served-lane dtype promises (the full path upcasts via merge; pinned in + # test_fast_path_preserves_int_node_dtypes and the CHANGELOG dtype note) + fn = _norm_all_cols(fast._nodes, ['v']) + assert fn['attr'].dtype.kind == 'i', "served lane must keep int node attrs int" + for c in set(fn.columns) & {'x', 'y'}: + assert fn[c].dtype == bool, f"served alias flag {c} must be bool, got {fn[c].dtype}" + fe = _norm_all_cols(fast._edges, ['s', 'd']) + for c in set(fe.columns) & {'r'}: + assert fe[c].dtype == bool, f"served alias flag {c} must be bool, got {fe[c].dtype}" + + +# Named shapes whose CORRECT answer is EMPTY. The serve gate cannot see result +# cardinality, so these all engage the fast path — and an empty answer must come back +# as the right empty SHAPE (alias columns present, zero rows), not a throw and not a +# missing-column frame. +_NAMED_EMPTY_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ + # seed filter matches no node at all (distinct from dead_end_seed, which matches + # a node that has no surviving edge) + ("zero_seed", lambda: [n({'attr': 999}, name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("zero_dst", lambda: [n(name='x'), e_forward(hops=1, name='r'), n({'attr': 999}, name='y')]), + ("zero_edge_match", lambda: [n(name='x'), e_forward(hops=1, edge_match={'w': 999}, name='r'), n(name='y')]), +] + + +@pytest.mark.parametrize("engine", ["pandas", "cudf"]) +@pytest.mark.parametrize("label,build", _NAMED_EMPTY_SHAPES, + ids=[s[0] for s in _NAMED_EMPTY_SHAPES]) +def test_fast_path_named_empty_result_matches_full_path(engine, label, build): + """POSITIVE boundary: named patterns matching ZERO rows are still served, and the + empty result must be shape-identical to the full path — same columns INCLUDING the + alias flag columns, zero rows, no exception.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + g = _fast_graph(engine) + if engine == "pandas": + assert _try_chain_fast_path(g, build(), Engine.PANDAS, None) is not None, \ + f"{label}: empty-result named shape must still be SERVED" + fast = g.gfql(build()) + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + fn, un = _norm_all_cols(fast._nodes, ['v']), _norm_all_cols(full._nodes, ['v']) + fe, ue = _norm_all_cols(fast._edges, ['s', 'd']), _norm_all_cols(full._edges, ['s', 'd']) + assert len(fn) == 0 and len(fe) == 0, f"{label}: expected empty result" + assert list(fn.columns) == list(un.columns), "empty nodes must keep alias columns" + assert list(fe.columns) == list(ue.columns), "empty edges must keep alias columns" + assert {'x', 'y'} <= set(fn.columns) and 'r' in fe.columns + assert len(un) == 0 and len(ue) == 0 + + +def test_fast_path_named_zero_edge_graph_matches_full_path(): + """POSITIVE boundary: a graph with an EMPTY edge table. The named pattern is served, + and both lanes must agree on the all-empty answer with alias columns present.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + nodes = pd.DataFrame({'v': [0, 1], 'attr': [10, 20]}) + edges = pd.DataFrame({'s': pd.Series([], dtype='int64'), + 'd': pd.Series([], dtype='int64'), + 'w': pd.Series([], dtype='int64')}) + g = CGFull().nodes(nodes, 'v').edges(edges, 's', 'd') + ops = [n(name='x'), e_forward(hops=1, name='r'), n(name='y')] + assert _try_chain_fast_path(g, ops, Engine.PANDAS, None) is not None + fast = g.gfql(ops) + full = g.gfql(ops, policy=_FAST_NOOP_POLICY) + for res in (fast, full): + assert res._nodes.shape[0] == 0 and res._edges.shape[0] == 0 + assert {'x', 'y'} <= set(fast._nodes.columns) and 'r' in fast._edges.columns + assert list(sorted(fast._nodes.columns)) == list(sorted(full._nodes.columns)) + assert list(sorted(fast._edges.columns)) == list(sorted(full._edges.columns)) + + +@pytest.mark.parametrize("route", ["fast_default", "full_policy"]) +def test_fast_path_duplicate_alias_still_raises_e201(route): + """NEGATIVE, end-to-end: duplicate NODE alias reuse must still RAISE E201 through + the public API. The gate-level decline (asserted in + test_fast_path_gating_returns_none_for_ineligible) exists precisely so + `combine_steps` stays in charge of this error; if the decline ever regressed, alias + reuse would silently SUCCEED on the served lane — this pins the user-visible raise + on both routes so that regression cannot land quietly.""" + from graphistry.compute.exceptions import ErrorCode, GFQLValidationError + g = _fast_graph("pandas") + kwargs = {} if route == "fast_default" else {"policy": _FAST_NOOP_POLICY} + with pytest.raises(GFQLValidationError) as exc_info: + g.gfql([n(name='x'), e_forward(hops=1), n(name='x')], **kwargs) + assert exc_info.value.code == ErrorCode.E201 + assert "'x'" in str(exc_info.value) + + +def test_fast_path_cross_type_alias_share_declines_and_matches(): + """NEGATIVE boundary pinning a subtlety found while testing: a NODE alias and an + EDGE alias sharing one name is NOT an E201 — `combine_steps` checks duplicates + per frame (node pass vs edge pass), and the two flag columns land on different + frames. The gate still declines it conservatively (duplicate_alias_edge in the + ineligible list), which is safe exactly because the full path serves it — so pin + that the two routes AGREE: no raise, same values, the flag on both frames.""" + g = _fast_graph("pandas") + ops = lambda: [n(name='r'), e_forward(hops=1, name='r'), n()] # noqa: E731 + default_route = g.gfql(ops()) + policy_route = g.gfql(ops(), policy=_FAST_NOOP_POLICY) + for res in (default_route, policy_route): + assert 'r' in res._nodes.columns and 'r' in res._edges.columns + _assert_full_frame_value_parity(default_route._nodes, policy_route._nodes, ['v']) + _assert_full_frame_value_parity(default_route._edges, policy_route._edges, ['s', 'd']) + + +def test_fast_path_named_defers_to_valid_resident_index(): + """NEGATIVE (deliberate decline): when BOTH resident indexes validly cover the + directed hop, a NAMED pattern must DECLINE the chain fast path so the index path + (which the gate would shadow) serves it — and the answer must not change. The gate + keys on index VALIDITY, so an index that does NOT cover the shape (reverse hop with + only the out-adjacency built) must NOT cause a decline, and unnamed patterns are + not deferred at all.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + from graphistry.compute.gfql.index.api import create_index + g = _fast_graph("pandas") + gi = create_index(create_index(g, 'edge_out_adj'), 'node_id') + named = [n(name='x'), e_forward(hops=1), n(name='y')] + assert _try_chain_fast_path(gi, named, Engine.PANDAS, None) is None, \ + "named + valid covering index must defer (decline) to the index path" + assert _try_chain_fast_path(gi, [n(), e_forward(hops=1), n()], Engine.PANDAS, None) is not None, \ + "unnamed is not deferred: the index deferral is scoped to named patterns" + assert _try_chain_fast_path(gi, [n(name='x'), e_reverse(hops=1), n(name='y')], Engine.PANDAS, None) is not None, \ + "reverse hop is NOT covered by edge_out_adj alone, so no deferral" + # deferral must be a routing decision only — values identical to the full path + deferred = gi.gfql(named) + full = g.gfql(named, policy=_FAST_NOOP_POLICY) + assert _setsig(deferred) == _setsig(full) + _assert_full_frame_value_parity(deferred._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(deferred._edges, full._edges, ['s', 'd']) + + +def test_fast_path_named_datetime_categorical_columns_ride_along(): + """POSITIVE dtype edge: datetime64 and categorical NODE columns must ride through + the served named lane unchanged — same values as the full path, dtypes preserved + (the served lane never merges, so it must not degrade either dtype) — and a seed + FILTER over the categorical column must both stay served and agree on values.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + nodes = pd.DataFrame({ + 'v': [0, 1, 2, 3, 4], + 'ts': pd.to_datetime(['2024-01-01', '2024-01-02', '2024-01-03', '2024-01-04', '2024-01-05']), + 'cat': pd.Categorical(['a', 'b', 'a', 'c', 'b']), + }) + edges = pd.DataFrame({'s': [0, 1, 2, 3, 0], 'd': [1, 2, 3, 4, 2], 'w': [5, 6, 7, 8, 9]}) + g = CGFull().nodes(nodes, 'v').edges(edges, 's', 'd') + ops = [n(name='x'), e_forward(hops=1, name='r'), n(name='y')] + assert _try_chain_fast_path(g, ops, Engine.PANDAS, None) is not None + fast = g.gfql(ops) + full = g.gfql(ops, policy=_FAST_NOOP_POLICY) + _assert_full_frame_value_parity(fast._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(fast._edges, full._edges, ['s', 'd']) + assert str(fast._nodes['ts'].dtype) == 'datetime64[ns]' + assert str(fast._nodes['cat'].dtype) == 'category' + # categorical seed filter: still served, same answer + ops2 = [n({'cat': 'a'}, name='x'), e_forward(hops=1), n(name='y')] + assert _try_chain_fast_path(g, ops2, Engine.PANDAS, None) is not None + f2 = g.gfql(ops2) + u2 = g.gfql(ops2, policy=_FAST_NOOP_POLICY) + _assert_full_frame_value_parity(f2._nodes, u2._nodes, ['v']) + _assert_full_frame_value_parity(f2._edges, u2._edges, ['s', 'd']) + + +# Alias names that SHADOW a real column WITHOUT breaking parity. Today BOTH lanes +# overwrite the shadowed column with the flag, identically — that (pre-existing, +# full-path) contract is what these pin, so a lane can't drift to a different +# overwrite/raise behavior alone. The FROM-side binding columns and the node-id +# binding are excluded here: those wrong-served (diverged) before, are now GATED to +# decline, and are pinned by the two regression tests below. +_ALIAS_SHADOW_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ + ("node_alias_shadows_node_data_col", lambda: [n(name='attr'), e_forward(hops=1), n()]), + ("edge_alias_shadows_edge_data_col", lambda: [n(), e_forward(hops=1, name='w'), n()]), + # TO-side binding columns keep parity (the from-side ones do not, see xfail below) + ("edge_alias_shadows_dst_binding_fwd", lambda: [n(), e_forward(hops=1, name='d'), n()]), + ("edge_alias_shadows_src_binding_rev", lambda: [n(), e_reverse(hops=1, name='s'), n()]), + # cross-frame names are NOT collisions: nodes have no 'w', edges have no 'v' + ("node_alias_named_like_edge_col", lambda: [n(name='w'), e_forward(hops=1), n()]), + ("edge_alias_named_like_node_id", lambda: [n(), e_forward(hops=1, name='v'), n()]), +] + + +@pytest.mark.parametrize("label,build", _ALIAS_SHADOW_SHAPES, + ids=[s[0] for s in _ALIAS_SHADOW_SHAPES]) +def test_fast_path_alias_shadowing_column_matches_full_path(label, build): + """NEGATIVE-ish boundary: alias names that collide with existing data/binding + columns must behave IDENTICALLY on both lanes (today: same silent overwrite the + full path has always done; cross-frame same-name cases are no-ops). Edge key + columns may themselves be overwritten here, so edges are keyed on 'w' (unique).""" + g = _fast_graph("pandas") + fast = g.gfql(build()) + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + _assert_full_frame_value_parity(fast._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(fast._edges, full._edges, ['w']) + + +@pytest.mark.parametrize("build", [ + lambda: [n(), e_forward(hops=1, name='s'), n()], + lambda: [n(), e_reverse(hops=1, name='d'), n()], +], ids=["fwd_alias_is_src_binding", "rev_alias_is_dst_binding"]) +def test_fast_path_edge_alias_colliding_with_from_binding_matches_full_path(build): + """REGRESSION (wrong-serve, found by adversarial parity testing, now GATED): an + edge alias equal to the hop's FROM-side binding column (forward+name=src, + reverse+name=dst) used to be SERVED with a DIFFERENT node set than the full path + (fast [0..4] vs full [1..4] on this fixture — the full path's flag overwrite + corrupts its own node reduction). The gate now DECLINES it; TO-side collisions + keep parity and stay served (pinned above). Both routes must agree: both raise, + or both return the same frames.""" + from graphistry.Engine import Engine + g = _fast_graph("pandas") + assert _try_chain_fast_path(g, build(), Engine.PANDAS, None) is None, \ + "edge alias == FROM-side binding must DECLINE the fast path" + try: + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + full_raised = None + except Exception as ex: # noqa: BLE001 — parity contract, not error-type contract + full, full_raised = None, type(ex) + if full_raised is not None: + with pytest.raises(full_raised): + g.gfql(build()) + else: + fast = g.gfql(build()) + assert full is not None + _assert_full_frame_value_parity(fast._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(fast._edges, full._edges, ['w']) + + +@pytest.mark.parametrize("build", [ + lambda: [n(name='v'), e_forward(hops=1), n()], + lambda: [n(), e_forward(hops=1), n(name='v')], +], ids=["n0_alias_is_node_binding", "n2_alias_is_node_binding"]) +def test_fast_path_alias_colliding_with_node_id_binding_matches_full_path(build): + """REGRESSION (wrong-serve, found by adversarial parity testing, now GATED): a + node alias equal to the NODE ID BINDING column ('v') used to be SERVED, silently + overwriting the id column with the bool alias flag while the full path raised. + The gate now DECLINES it, so the two lanes are observationally equivalent: both + raise, or both return the same frames (today: both raise pandas' ValueError).""" + from graphistry.Engine import Engine + g = _fast_graph("pandas") + assert _try_chain_fast_path(g, build(), Engine.PANDAS, None) is None, \ + "node alias == node-id binding must DECLINE the fast path" + try: + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + full_raised = None + except Exception as ex: # noqa: BLE001 — parity contract, not error-type contract + full, full_raised = None, type(ex) + if full_raised is not None: + with pytest.raises(full_raised): + g.gfql(build()) + else: + fast = g.gfql(build()) + assert full is not None + _assert_full_frame_value_parity(fast._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(fast._edges, full._edges, ['s', 'd']) + + @pytest.mark.parametrize("engine", ["pandas", "cudf"]) def test_fast_path_preserves_int_node_dtypes(engine): """Documented behavior change: the 1-hop fast path PRESERVES node-attribute @@ -758,6 +1130,16 @@ def test_fast_path_gating_returns_none_for_ineligible(): # #1755 lever-3: typed edges are now accepted (edge filter on the frontier) [n(), e_forward(hops=1, edge_match={'w': 5}), n()], [n({'attr': 10}), e_forward(hops=1, edge_match={'w': 5}), n()], + # DELIBERATE RULE CHANGE (was asserted INELIGIBLE as "named_node"): an alias is a + # PROJECTION concern, so it no longer gates the traversal path. The old assertion + # encoded the gate itself, not a semantic the fast path could not meet — the alias + # flag columns are reconstructible from the surviving edges + # (`_tag_fast_path_aliases`), which is exactly what `combine_steps` computes. + [n(name='x'), e_forward(hops=1), n()], + [n(), e_forward(hops=1), n(name='y')], + [n(), e_forward(hops=1, name='r'), n()], + [n(name='x'), e_forward(hops=1, name='r'), n(name='y')], + [n(name='x'), e_reverse(hops=1, name='r'), n(name='y')], ] for ops in eligible: assert _try_chain_fast_path(g, ops, Engine.PANDAS, None) is not None, f"should accept {ops}" @@ -765,10 +1147,35 @@ def test_fast_path_gating_returns_none_for_ineligible(): ineligible = [ ("hops_2", [n(), e_forward(hops=2), n()], None, Engine.PANDAS), ("filtered_undirected", [n({'attr': 10}), e_undirected(hops=1), n({'attr': 30})], None, Engine.PANDAS), - ("named_node", [n(name='x'), e_forward(hops=1), n()], None, Engine.PANDAS), + # NEW declines that come with serving aliases. Undirected: a node is reachable as + # EITHER endpoint, so alias identity is not derivable from the endpoint columns. + ("named_undirected", [n(name='x'), e_undirected(hops=1), n(name='y')], None, Engine.PANDAS), + ("named_undirected_edge", [n(), e_undirected(hops=1, name='r'), n(name='y')], None, Engine.PANDAS), + # Duplicate alias reuse is E201, and `combine_steps` is what raises it; serving + # here would BYPASS the check and let alias reuse silently succeed. + ("duplicate_alias_nodes", [n(name='x'), e_forward(hops=1), n(name='x')], None, Engine.PANDAS), + ("duplicate_alias_edge", [n(name='r'), e_forward(hops=1, name='r'), n()], None, Engine.PANDAS), ("node_query", [n(query='attr > 5'), e_forward(hops=1), n()], None, Engine.PANDAS), ("prune_endpoints", [n(), e_forward(hops=1, prune_to_endpoints=True), n()], None, Engine.PANDAS), ("seeded", [n()], seed, Engine.PANDAS), + # MIXED supported + unsupported: an alias is now a served concern, but it must + # never FLIP an otherwise-ineligible shape to served — the unsupported piece + # (multi-hop, queries, richer predicates, prune, seeds) still declines the whole op list. + ("named_hops_2", [n(name='x'), e_forward(hops=2), n(name='y')], None, Engine.PANDAS), + ("named_node_query", [n(query='attr > 5', name='x'), e_forward(hops=1), n(name='y')], None, Engine.PANDAS), + ("named_edge_query", [n(name='x'), e_forward(hops=1, edge_query='w > 5'), n(name='y')], None, Engine.PANDAS), + ("named_source_node_match", [n(name='x'), e_forward(hops=1, source_node_match={'attr': 10}), n(name='y')], None, Engine.PANDAS), + ("named_prune_endpoints", [n(name='x'), e_forward(hops=1, prune_to_endpoints=True), n(name='y')], None, Engine.PANDAS), + ("named_seeded", [n(name='x'), e_forward(hops=1), n(name='y')], seed, Engine.PANDAS), + # BINDING-COLLISION declines (wrong-serves found by adversarial parity testing): + # a node alias equal to the node-id binding would overwrite the id column where + # the full path raises; an edge alias equal to the hop's FROM-side binding made + # the two lanes return different node sets. TO-side collisions keep parity and + # remain served (see test_fast_path_alias_shadowing_column_matches_full_path). + ("n0_alias_is_node_binding", [n(name='v'), e_forward(hops=1), n()], None, Engine.PANDAS), + ("n2_alias_is_node_binding", [n(), e_forward(hops=1), n(name='v')], None, Engine.PANDAS), + ("edge_alias_is_src_binding_fwd", [n(), e_forward(hops=1, name='s'), n()], None, Engine.PANDAS), + ("edge_alias_is_dst_binding_rev", [n(), e_reverse(hops=1, name='d'), n()], None, Engine.PANDAS), ("non_eager_engine", [n()], None, Engine.DASK), ("two_ops", [n(), e_forward(hops=1)], None, Engine.PANDAS), ]