fix(gfql): pre-existing engine-parity/correctness bugs (ORDER BY DESC nulls, predicate from_json, str value-safety)

CHANGELOG.md

@@ -11,6 +11,13 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Documentation
 - **GFQL engine-selection docs (pandas / polars / cuDF / polars-gpu)**: New :doc:`Choosing a GFQL Engine <gfql/engines>` page — a numbers-first, persona-tested guide to the four interchangeable engines. Adds the one-keyword `engine='polars'` speedup (up to ~38× over pandas on real graphs, no GPU), a motivating warm-median comparison table on real public graphs (LiveJournal 35M / Orkut 117M), a decision matrix (workload shape × size × hardware → engine, with the measured ~10K-edge CPU crossover, GPU-work-bound rule, polars-gpu memory-pressure caveat, and GPU-or-error contract), a cuDF-vs-polars-gpu disambiguation (eager-op vs fused-lazy; cuDF is not deprecated), an honest "when *not* to use Polars" section, the differential-parity guarantee, and a methodology + reproducer-script disclosure. Rewrote the top of `gfql/performance.rst` to lead with the engine comparison (de-marketed the prose), wired the new page into the GFQL toctree + recommended paths, and added Polars/polars-gpu to the engine examples in `gfql/quick.rst` and `gfql/about.rst` (previously only pandas/cuDF were documented). Driven by 4-persona doc user-testing (pandas DS, RAPIDS user, perf engineer, skeptical evaluator).
 
+### Fixed
+- **GFQL Cypher `CASE` with mixed-dtype branches is now engine-consistent on cuDF (no more `GFQLTypeError`)**: pandas coerces the two `CASE` branches to a common type, but cuDF's `.where` raised `TypeError: cudf does not support mixed types`, surfaced as a hard `GFQLTypeError`. This bit `CASE WHEN path IS NULL THEN -1 ELSE length(path) END` over an UNREACHABLE `shortestPath` (int `-1` branch vs an object/null hops branch) — pandas returned `-1`, cuDF errored. The row-AST `CASE` evaluator now unifies the branch dtypes and retries, so cuDF returns the same value pandas does. Regression test `test_case_mixed_dtype.py`.
+- **GFQL polars engine raises a clean typed error (not an opaque polars `InvalidOperationError`) for a string predicate on a non-string column**: `WHERE col STARTS WITH/CONTAINS/regex ...` on a Categorical/Enum/numeric column raised a clean `GFQLSchemaError` ("string predicate used on non-string column") on pandas/cuDF but leaked polars' internal `InvalidOperationError: expected String type, got: cat` on polars. `filter_by_dict_polars` now raises the same `GFQLSchemaError` (categorical treated as non-string, exactly as `filter_by_dict`), so all three engines agree. Regression test `test_polars_string_predicate_nonstring.py`; also confirms connected-join predicate pushdown is parity-correct on polars for string/numeric/eq/bool columns.
+- **GFQL string predicates (`Contains`/`Startswith`/`Endswith`/`Match`/`Fullmatch`) are now value-safe on non-string columns**: applying a string predicate to a numeric/temporal/bool column raised an opaque `AttributeError: Can only use .str accessor with string values!` on pandas and cuDF. They now follow openCypher semantics — a string op over a non-string value is null → excluded (matching the established per-cell behavior on an object column holding non-strings) — and never stringify the column (which would diverge pandas↔cuDF, e.g. wrongly matching `5 CONTAINS '5'`). String, mixed-object, and all-null columns are unchanged. Regression tests in `test_str.py` across dtypes; cuDF parity confirmed.
+- **GFQL predicate `from_json` no longer downgrades comparison predicates to numeric-only (fixes JSON round-trip of string/temporal `=`, `<`, `>`, etc.)**: the predicate deserialization registry mapped `EQ`/`NE`/`GT`/`GE`/`LT`/`LE`/`Between` to the numeric-only `predicates.numeric` classes, while Cypher lowering and the public predicate API build the richer `predicates.comparison` versions (which also accept strings and temporals). A JSON round-trip (remote GFQL, serialization, caching) therefore rebuilt a string/temporal equality or a temporal comparison as numeric-only, raising `GFQLTypeError "val must be numeric"`. `from_json` now registers the `comparison` versions; numeric-valued predicates are unaffected (comparison is a superset). Round-trip regression tests in `test_from_json.py`.
+- **GFQL Cypher `ORDER BY ... DESC` now places NULLs FIRST (openCypher NULL-largest), fixing silent-wrong `DESC ... LIMIT k` top-k**: openCypher orders NULL as the largest value (`ASC` → nulls last, `DESC` → nulls first), but the general row pipeline hardcoded nulls-last for every key on all engines — so any `ORDER BY <col> DESC` over a column containing NULLs was mis-ordered and `DESC ... LIMIT k` silently dropped the NULL group that should rank first. Fixed on pandas/cuDF (per-key null-indicator single-pass in `row/pipeline.py`; stability-free, so cuDF's non-stable sort is safe) and polars (per-key `nulls_last` list in `order_by_polars`). Hand-derived-oracle regression suite `test_order_by_null_placement.py` across pandas/polars/cuDF. Pre-existing bug (not introduced by the OLAP split); the OLAP fast paths already ordered nulls correctly, so this brings the general/fallback path into agreement.
+
 ### Added
 - **GFQL Cypher row-pipeline single-alias predicate pushdown**: Planner-generated `rows(alias_prefilters=...)` hints pre-filter eligible node and single-hop edge alias frames before binding joins on pandas/cuDF while retaining the original post-join filters for exact semantics and engine-safe fallback.
 - **GFQL viz-filter-pipeline acceptance suite + regression benchmark (viz-filter L3)**: `test_viz_pipeline_conformance.py` — curated full-panel pipelines (node+edge filters, exclusion-dominates composition, `(pred OR IS NULL)` keep-null leaves, both EXISTS prune-isolated flavors, searchAny composition, deterministic paging), graph-state prune shapes with exact node+edge pins, a 40-seed panel-state fuzzer with an independent plain-pandas second oracle, and a case/regex/unicode trick matrix (ß/İ full-case-mapping pins, metachar literal-vs-regex, null cells, Categorical) — all parity-or-NIE across pandas/cuDF/polars/polars-gpu. `benchmarks/gfql/viz_filter_pipeline.py` — six streamgl-viz panel scenarios (filters, keep-self GRAPH prune, EXISTS prune, node/edge search, combined) at 100K/1M/10M with native-frame-per-engine fairness, an NIE-tolerant matrix, and JSON receipts (first receipt: 100k × 4 engines, everything within the ~350ms interactive reference except pandas combined). Documented findings from the first runs: the same-path WHERE route dedupes parallel edges (diverges from the panel algebra's edge multiplicity — pinned + tracked), and edge-alias searchAny declines on polars (tracked).

bin/test-polars.sh

@@ -19,6 +19,8 @@ POLARS_TEST_FILES=(
     graphistry/tests/compute/gfql/test_engine_polars_row_pipeline.py
     graphistry/tests/compute/gfql/test_engine_polars_cypher_conformance.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
     graphistry/tests/compute/gfql/test_conformance_ledger.py
     # index tests exercise the seeded-index hook in the polars hop entry (hop.py) — without
     # them the hook dominates the now-thin file and trips its per-file coverage floor

graphistry/compute/gfql/lazy/engine/polars/predicates.py

@@ -13,7 +13,7 @@
 from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Union
 
 from graphistry.compute.predicates.ASTPredicate import ASTPredicate
-from graphistry.compute.predicates.str import Fullmatch, Match
+from graphistry.compute.predicates.str import Contains, Endswith, Fullmatch, Match, Startswith
 from graphistry.compute.filter_by_dict import resolve_filter_column
 from .dtypes import is_numeric as _dtype_numeric, is_stringlike as _dtype_stringlike
 
@@ -325,6 +325,23 @@ def filter_by_dict_polars(df: "pl.DataFrame", filter_dict: "Optional[Dict[str, A
                     f"comparison on column {resolved_col!r}; use engine='pandas' for this "
                     f"query (no pandas fallback; parity-or-error by design)"
                 )
+            # String predicate on a non-`String` column: pandas/cuDF raise a clean
+            # GFQLSchemaError (E302) at schema validation, but polars would build `.str.<op>` and
+            # raise an OPAQUE `InvalidOperationError` ("expected String type, got: cat") at collect
+            # on a Categorical/Enum/numeric column. Raise the SAME clean, typed error so all three
+            # engines agree (categorical is treated as non-string here, exactly as filter_by_dict).
+            if isinstance(resolved_val, (Contains, Startswith, Endswith, Match)):
+                _col_dtype = df.schema.get(resolved_col)
+                if _col_dtype is not None and _col_dtype != pl.String:
+                    from graphistry.compute.exceptions import ErrorCode, GFQLSchemaError
+                    raise GFQLSchemaError(
+                        ErrorCode.E302,
+                        f'Type mismatch: string predicate used on non-string column "{resolved_col}"',
+                        field=col,
+                        value=f"{resolved_val.__class__.__name__}(...)",
+                        column_type=str(_col_dtype),
+                        suggestion='Use numeric predicates like gt() or lt() for numeric columns',
+                    )
             expr = predicate_to_expr(resolved_col, resolved_val, df.schema.get(resolved_col))
             if expr is None:
                 # decline (NIE): no native lowering for this predicate; no pandas bridge.

graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py

@@ -706,9 +706,13 @@ def order_by_polars(g: Plottable, keys: Sequence[Any]) -> Optional[Plottable]:
     if lowered is None:
         return None
     exprs, descending = lowered
-    # cypher ORDER BY puts NULLs last; polars defaults to nulls_last=False (pandas sort_values
-    # default puts NaN last only for asc), so set nulls_last=True to match pandas na_position='last'.
-    return _rewrap(g, table.sort(exprs, descending=descending, nulls_last=True))
+    # openCypher orders NULL as the LARGEST value: ASC -> nulls last, DESC -> nulls FIRST.
+    # (Previously hardcoded nulls_last=True, which mis-ordered DESC keys and silently returned
+    # the wrong `... DESC LIMIT k` top-k over a column containing NULLs.) Normalize `descending`
+    # to a per-key list (it may arrive as a scalar bool) so `nulls_last` mirrors it per key.
+    descending_list = descending if isinstance(descending, list) else [descending] * len(exprs)
+    nulls_last = [not d for d in descending_list]
+    return _rewrap(g, table.sort(exprs, descending=descending_list, nulls_last=nulls_last))
 
 
 # Native aggs: count/sum/avg/min/max/count_distinct/collect/collect_distinct; stdev/percentile

graphistry/compute/gfql/row/pipeline.py

@@ -240,6 +240,22 @@ def _gfql_fresh_col_name(columns: Any, prefix: str) -> str:
             col = f"{col}_x"
         return col
 
+    @staticmethod
+    def _gfql_coerce_case_branch_dtypes(a: Any, b: Any) -> "Tuple[Any, Any]":  # pragma: no cover - only reached from the cuDF CASE-mixed-dtype retry above; no cuDF coverage lane (validated on dgx)
+        """Cast two CASE branches to a common dtype for engines (cuDF) whose ``.where`` rejects
+        mismatched dtypes, where pandas coerces. Prefer a numeric common type (covers int / float /
+        null — the shortestPath hops branch); fall back to string; last resort return unchanged so
+        the caller re-raises the original error rather than masking an unexpected failure."""
+        for target in ("float64",):
+            try:
+                return a.astype(target), b.astype(target)
+            except Exception:
+                continue
+        try:
+            return a.astype("str"), b.astype("str")
+        except Exception:
+            return a, b
+
     @staticmethod
     def _gfql_mask_fill(series: Any, mask: Any, value: Any) -> Any:
         out = series.copy()
@@ -1648,7 +1664,17 @@ def _floor_series(s: Any) -> Any:
             cond_mask = self._gfql_bool_mask(table_df, cond_value)
             cond_null = self._gfql_null_mask(table_df, cond_value)
             cond_true = cond_mask & ~cond_null
-            return True, true_value.where(cond_true, false_value)
+            try:
+                return True, true_value.where(cond_true, false_value)
+            except TypeError:  # pragma: no cover - cuDF-only (.where mixed-dtype); no cuDF lane in the coverage gate (validated on dgx)
+                # cuDF rejects `.where` across mismatched branch dtypes ("cudf does not support
+                # mixed types"), where pandas coerces to a common type. This surfaced as a hard
+                # GFQLTypeError for e.g. `CASE WHEN path IS NULL THEN -1 ELSE length(path) END`
+                # over an UNREACHABLE shortestPath (the hops branch is an object/null column while
+                # the other branch is int -1). Unify the two branches to a common dtype and retry so
+                # CASE is engine-consistent (cuDF returns -1 like pandas instead of raising).
+                tv, fv = RowPipelineMixin._gfql_coerce_case_branch_dtypes(true_value, false_value)
+                return True, tv.where(cond_true, fv)
 
         if isinstance(node, QuantifierExpr):
             source_ok, list_value = self._gfql_eval_expr_ast(table_df, node.source)
@@ -4744,8 +4770,25 @@ def order_by(self, keys: List[Any]) -> "Plottable":
 
         if sort_cols:
             try:
-                effective_sort_cols = sort_cols + [row_order_col]
-                effective_ascending = ascending + [True]
+                # openCypher orders NULL as the LARGEST value: ASC -> nulls last, DESC -> nulls
+                # FIRST. pandas/cuDF sort_values `na_position` is a SINGLE scalar (cannot be
+                # per-key) and cuDF has no stable sort, so use a per-key null-indicator column
+                # sorted in the SAME direction as its key: ind = col.isna(); for an ASC key
+                # (ascending=True) True (null) sorts LAST, for a DESC key (ascending=False) True
+                # sorts FIRST. Matches the OLAP fast-path fix and order_by_polars. (Previously
+                # sort_values defaulted na_position='last' for every key, silently returning the
+                # wrong `... DESC LIMIT k` top-k over a column containing NULLs.)
+                interleaved_cols: List[Any] = []
+                interleaved_ascending: List[bool] = []
+                for _i, (_sc, _asc) in enumerate(zip(sort_cols, ascending)):
+                    _ind_col = RowPipelineMixin._gfql_fresh_col_name(
+                        work_df.columns, f"__gfql_sort_nullrank_{_i}__"
+                    )
+                    work_df = work_df.assign(**{_ind_col: work_df[_sc].isna()})
+                    interleaved_cols.extend([_ind_col, _sc])
+                    interleaved_ascending.extend([_asc, _asc])
+                effective_sort_cols = interleaved_cols + [row_order_col]
+                effective_ascending = interleaved_ascending + [True]
                 out_df = work_df.sort_values(by=effective_sort_cols, ascending=effective_ascending)
             except Exception as exc:
                 raise ValueError(

graphistry/compute/predicates/from_json.py

@@ -4,7 +4,12 @@
 from graphistry.compute.predicates.categorical import Duplicated
 from graphistry.compute.predicates.is_in import IsIn
 from graphistry.compute.predicates.logical import AllOf
-from graphistry.compute.predicates.numeric import GT, LT, GE, LE, EQ, NE, Between, IsNA, NotNA
+# Register the comparison.py versions (numeric + temporal + string), NOT numeric.py's numeric-only
+# classes: they share __name__ ("EQ", "GT", ...) so the from_json type registry would otherwise
+# downgrade a live comparison predicate to numeric-only on a JSON round-trip — raising "val must be
+# numeric" for a string/temporal equality or temporal comparison built by Cypher lowering / the
+# public predicate API (both of which use comparison.py). See from_json round-trip regression tests.
+from graphistry.compute.predicates.comparison import GT, LT, GE, LE, EQ, NE, Between, IsNA, NotNA
 from graphistry.compute.predicates.str import (
     Contains, Startswith, Endswith, Match, Fullmatch, IsNumeric, IsAlpha, IsDecimal, IsDigit, IsLower, IsUpper,
     IsSpace, IsAlnum, IsTitle, IsNull, NotNull

graphistry/compute/predicates/str.py

@@ -7,6 +7,44 @@
 from graphistry.compute.typing import SeriesT
 
 
+def _series_supports_str_ops(s: Any) -> bool:
+    """True if ``s`` has a usable string accessor (object / pandas ``string`` / categorical-of-str).
+
+    A numeric, temporal, or boolean column does NOT: pandas and cuDF both raise on ``s.str``
+    attribute access for those dtypes. Used to make the string predicates value-safe instead of
+    surfacing an opaque ``AttributeError: Can only use .str accessor with string values!``.
+    """
+    try:
+        s.str  # the accessor validates dtype on attribute access
+    except (AttributeError, TypeError):
+        return False
+    return True
+
+
+def _nonstring_null_result(s: Any, na: Optional[bool]) -> Any:
+    """Result of a string predicate applied to a NON-string column.
+
+    openCypher treats a string operation over a non-string value as null (a non-string value is
+    not a string, so the predicate is unknown → null → excluded from a ``WHERE``). This mirrors the
+    established per-cell behavior on an *object* column holding non-strings (those cells already
+    become null), and — crucially — NEVER stringifies the column (which would diverge pandas↔cuDF
+    and wrongly match, e.g. ``5 CONTAINS '5'``). ``na`` overrides the fill only when the caller
+    pinned it (``na=True``/``na=False``).
+    """
+    fill = None if na is None else na
+    n = len(s)
+    is_cudf = hasattr(s, '__module__') and 'cudf' in s.__module__
+    if is_cudf:  # pragma: no cover - cuDF-only; the changed-line-coverage gate has no cuDF lane (validated on dgx)
+        import cudf
+        out = cudf.Series([fill] * n)
+        try:
+            out.index = s.index
+        except Exception:
+            pass
+        return out
+    return pd.Series([fill] * n, index=getattr(s, 'index', None))
+
+
 def _cudf_mask_value(result: Any, mask: Any, value: Any) -> Any:
     try:
         return result.mask(mask, value)
@@ -91,6 +129,8 @@ def __init__(
         self.regex = regex
 
     def __call__(self, s: SeriesT) -> SeriesT:
+        if not _series_supports_str_ops(s):
+            return _nonstring_null_result(s, self.na)
         is_cudf = hasattr(s, '__module__') and 'cudf' in s.__module__
 
         # workaround cuDF not supporting 'case' and 'na' parameters
@@ -260,6 +300,8 @@ def _compute_result(self, s: SeriesT, is_cudf: bool) -> SeriesT:
         return self._match_boundary(s, self.pat)
 
     def __call__(self, s: SeriesT) -> SeriesT:
+        if not _series_supports_str_ops(s):
+            return _nonstring_null_result(s, self.na)
         is_cudf = hasattr(s, '__module__') and 'cudf' in s.__module__
         result = self._compute_result(s, is_cudf)
         if is_cudf:
@@ -381,6 +423,8 @@ def _compute_result(self, s: SeriesT, is_cudf: bool) -> SeriesT:
         raise NotImplementedError
 
     def __call__(self, s: SeriesT) -> SeriesT:
+        if not _series_supports_str_ops(s):
+            return _nonstring_null_result(s, self.na)
         is_cudf = hasattr(s, '__module__') and 'cudf' in s.__module__
         result = self._compute_result(s, is_cudf)
         if is_cudf: