Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 — `(<toLower|lower|toUpper|upper>(a.col) = 'lit')` and `(a.col <op> 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.
Expand Down
Loading
Loading