Merge pull request #1819 from graphistry/fix/gfql-polars-agg-string-divergence

fix(gfql): one Cypher type contract for sum()/avg() across pandas, polars and cuDF

Author: lmeyerov

CHANGELOG.md

@@ -34,6 +34,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - **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.
 
 ### Fixed
+- **`sum()`/`avg()` over a non-numeric column now raise the same typed error on every engine — polars no longer answers `avg(<string>)` with a silent `null`, and pandas no longer answers `sum(<string>)` with the string CONCATENATION**: the aggregate kernels are written four times (pandas/cuDF row pipeline, native polars row pipeline, the OLAP single-hop grouped fast path, and the fused lazy lane #1823 added in front of it) and each had inherited its host library's opinion about non-numeric input, so the SAME query answered differently depending on the engine: `avg(n.name)` raised on pandas but returned `null` on polars, while `sum(n.name)` returned `'abac'` on pandas and leaked a raw `polars.exceptions.InvalidOperationError` through the GFQL surface. "Match the other engine" was not available, because the two engines were wrong in OPPOSITE directions — so the contract is pinned to Cypher instead. openCypher/Neo4j declares `avg(input)` and `sum(input)` over `INTEGER | FLOAT | DURATION` only (neo4j/docs-cypher `functions/aggregating.adoc`) and enforces it: Neo4j 5.26.26 answers `RETURN avg(r.s)` over strings with *"AVG(...) can only handle numerical values, duration, or null."* and `sum(date(...))` with *"Type mismatch: expected Float, Integer or Duration but was Date"*; Kuzu 0.11.3 rejects both at bind time. A non-numeric input is therefore a **`GFQLTypeError` (E302) naming the aggregate and the user's output alias**, on every engine and on both the row pipeline and the fast path. **This is a deliberate contract change on the pandas engine**: `sum(<string column>)` used to return the concatenation, which is a silent wrong answer with no meaning in Cypher, and it was already inconsistent with `avg()`, which raised. Also brought to the same contract, all found by a full aggregate × dtype differential sweep rather than one at a time: `sum` over temporal and categorical columns (polars returned `null`, pandas raised), `sum`/`avg` over an all-null column (now `0` / `null` per Neo4j's *"`sum(null)` returns `0`"*, substituted rather than delegated — the host kernels answer that case `0`/`''`/`NaT`/`TypeError` depending on dtype, and polars raised), and `min`/`max`/`collect`/`count(DISTINCT)` over a **categorical** column, which Cypher accepts as `ANY` but which pandas rejected outright and cuDF answered with a *category label instead of a count*. A 180-cell polars × cuDF × polars-gpu matrix over 6 aggregates × 10 dtype columns (`_MATRIX_AGGS` × `_MATRIX_COLS`, each compared against the pandas oracle on BOTH the value and the error class) now shows **zero divergences**. Error-path only: the guards read a dtype and do not touch value computation on any served shape.
+- **A raw `polars.exceptions.*` could reach the caller from the native polars row pipeline**: on the pandas/cuDF surface `execute_call` wraps any kernel exception as `GFQLTypeError(E303)`, but the native polars path runs BEFORE `execute_call` and so skipped that wrapper entirely — `polars.exceptions.InvalidOperationError: `sum` operation not supported for dtype `str`` was reaching users verbatim, third-party class and all. Native row ops now funnel polars errors through the same wrapper with the same code and message shape, preserving the polars text as the exception cause.
 - **Three BOUNDED variable-length shapes returned a silently different count on `engine='polars'` (#1787)**: the native polars `rows(binding_ops=...)` builder rebuilds a variable-length segment from the raw matching edge table, and for three bounded shapes that rebuild produces a different edge multiplicity than the pandas oracle — with no error, just a different number. They now raise `NotImplementedError` like every other shape this lowering cannot reproduce, which is a deliberate behaviour change: these were *served* before, so a silent wrong answer becomes a loud, actionable error, and `engine='pandas'` still answers all of them. The three: (1) directed `-[*k..m]->` with `min_hops >= 3`, and `min_hops >= 2` when the segment starts from a filtered seed — `max_reached_hop` in `compute/hop.py` is a dedup-by-node BFS eccentricity rather than a longest-walk length, so the oracle prunes to empty where the rebuild expands a different edge multiset; (2) the DEGENERATE undirected window `-[*1..1]-` / `-[*1]-`, which halved the count — it resolves to `min == max == 1` and so is not "multihop", yet pandas still routes it through the variable-length hop, which is exactly why it slipped past the existing gates (the gate therefore keys on an explicit variable-length window, not on `is_multihop`); (3) undirected `-[*1..k]-` that does not start from the full node set (filtered seed, or a non-first segment), where the doubled-pair expansion over-counts. Every neighbouring shape stays native and is pinned as such — unseeded `min_hops <= 2`, the directed twin of each undirected decline, the directed degenerate window, and the plain `-[]-` edge — so the gate is a scalpel rather than a blanket refusal of variable-length segments. Same root-cause family as the unbounded shapes #1781 declined; the gate should shrink again once the multiplicity is reconstructible. Found by differential fuzzing against the pandas oracle, and pinned by an engine-parametrized suite (pandas / polars / cuDF / polars-gpu) that encodes the intended per-engine behaviour — the polars engines must decline exactly where the pandas-API engines must answer — plus a seeded fuzz that fails if the gate declines too much.
 - **`rows(table='edges')` silently returned the wrong table after a NAMED traversal**: the named-middle rewrite — which turns `[...named ops..., rows()]` into `rows(binding_ops=...)` so a Cypher multi-alias `RETURN` lowers to a bindings table — skipped itself when the call already carried `binding_ops`, `source` or `alias_endpoints`, but not when it carried a non-default `table`. So naming any op in the middle changed which table came back: an odd-length middle silently produced the BINDINGS table instead of the requested edges (no error, `table=` simply ignored), and an even-length middle — a path ending on an edge, e.g. `(person)-[r:KNOWS]-` — produced a non-alternating op list and raised "require ... a single connected alternating node/edge path". An unnamed middle was unaffected, which is what made it look shape-specific rather than naming-specific. Both chain surfaces are fixed (the generic chain and the native polars chain carry the rewrite independently, so fixing one left the other wrong). Note the guard keys on a NON-DEFAULT table: `rows()` declares `table='nodes'` and always emits it, so an explicit `rows(table='nodes')` is indistinguishable from a bare `rows()` and still rewrites — pinned as a known limitation rather than left to be rediscovered.
 - **Indexed bindings bypass returned the WHOLE edge table for `rows(table='edges')`**: the bypass exists to SKIP the canonical traversal, so whatever it hands the suffix is the pre-traversal graph. That is sound for a bindings table — the indexed path bag already is the answer — but the gate declined only for `source` / `alias_endpoints` / `alias_prefilters`, not for a non-default `table`, so a `rows(table='edges')` after a named middle read the full edge frame instead of the traversal-narrowed one (measured on a 12-edge fixture: 12 rows instead of 3, on pandas, cuDF and native polars, and the wrong values survive a following `select`). Both gates now decline a non-default `table` (`chain._plan_indexed_middle` and the native polars `_try_indexed_middle_polars`), matching the named-middle rewrite's guard. This became reachable only once that rewrite stopped firing for a non-default `table` — before it, the rewrite converted the call on BOTH the indexed and the scan path, so the two agreed on the wrong table. Same class as the earlier "indexed bypass could return every node for `rows()` over an unnamed pattern".

bin/test-polars.sh

@@ -34,6 +34,9 @@ POLARS_TEST_FILES=(
     # only ever run here (the file has no module-level importorskip, so nothing else flags it)
     graphistry/tests/compute/gfql/test_exec_context_scoping.py
     graphistry/tests/compute/gfql/test_engine_polars_cypher_conformance.py
+    # aggregate x dtype type contract: the polars params of this file are the ONLY lane where the
+    # native polars aggregate guard and the raw-polars-exception wrap are exercised
+    graphistry/tests/compute/gfql/test_aggregate_type_contract.py
     graphistry/tests/compute/gfql/test_engine_polars_conformance_matrix.py
     graphistry/tests/compute/gfql/test_polars_string_predicate_nonstring.py
     graphistry/tests/compute/gfql/cypher/test_order_by_null_placement.py

docs/source/gfql/spec/cypher_mapping.md

@@ -437,10 +437,25 @@ analysis = g.gfql([
 | `LIMIT 10` | `limit(10)` | Row cap |
 | `WHERE <row expr>` | `where_rows(expr="...")` | Scalar expression subset |
 | `count(*)` | `group_by(keys=[...], aggregations=[("cnt", "count")])` | Grouped count |
-| `sum(n.val)` | `group_by(..., aggregations=[("total", "sum", "val")])` | Grouped sum |
+| `sum(n.val)` | `group_by(..., aggregations=[("total", "sum", "val")])` | Grouped sum; see [Aggregate input types](#aggregate-input-types) |
 | `collect(n.x)` | `group_by(..., aggregations=[("xs", "collect", "x")])` | Nulls excluded from collection |
 | Named patterns | `rows(source="alias")` | Scope row table to a named match alias |
 
+### Aggregate input types
+
+`sum` and `avg` accept `INTEGER`, `FLOAT` and `DURATION`, matching Cypher — **plus `BOOLEAN`, which
+is a deliberate GFQL extension.** Neo4j rejects it (*"expected Float, Integer or Duration but was
+Boolean"*); GFQL accepts it because summing an indicator column is idiomatic in the dataframe
+surface GFQL also serves, and every engine already agrees on the answer. `sum` over a boolean
+counts the true values; `avg` gives their fraction.
+
+Any other input type **raises**. This is stricter than earlier releases, where a string column
+returned its *concatenation* on pandas and leaked a raw polars error on polars — wrong in two
+different directions.
+
+Empty and all-null inputs follow Cypher rather than SQL: `sum` returns **0**, `avg` returns
+**null**.
+
 ## Key Differences
 
 | Feature | Python | Wire Protocol |