Skip to content

Commit f45469c

Browse files
authored
Merge pull request #2056 from graphistry/fix/gfql-2050-binding-alias-validation
fix(gfql): chain result contract: binding-column aliases decline, duplicate ids answer once, no internal columns (#2050, #2051, #2067)
2 parents 60bb843 + 32ea472 commit f45469c

12 files changed

Lines changed: 231 additions & 72 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
1818

1919
### Fixed
2020

21+
* GFQL: a native chain whose edge alias is named like the source, destination or edge-ID binding column silently overwrote that binding on pandas and cuDF (the seed's edges vanished, or the edge ids became `True`) and raised a raw polars `SchemaError`; it is now the same typed decline (E108) the node-ID collision already gets, on every engine, before execution — the rule the Cypher route already applied (#2050).
22+
* GFQL polars: a colliding alias no longer leaves the internal `__gfql_shadow_restore__<alias>__` column on op-list results; the chain keeps the shadowed values under that name only while a compiled Cypher pipeline runs (its row pipeline reads them back), and on `gfql([...])` / `chain([...])` the marker simply shadows the column as on pandas and cuDF. Nothing is stripped from results: user-defined columns and pipelines composed from successive `gfql` calls are untouched.
23+
2124
* GFQL: a multi-hop step (`hops=2`, `to_fixed_point`) whose edge alias shares its name with the column its own filter uses raised `incompatible-column-type` on pandas and cuDF; the backward re-execution now reads the graph's edge columns for that step, so the eager engines serve it like the single-hop form and return the same rows as polars (#2049).
2225
* GFQL: one alias-shadowing contract on every engine for op-list chains: an alias named like a column of the frame it marks becomes the boolean marker under that name (pandas, cuDF and now polars, which used to keep the user values and leak a `_right` join suffix). The shadowed values ride under the internal restore column the Cypher row pipeline already resolves, so Cypher keeps reading the user value through the variable (`MATCH (a)-[type:K]->(b) RETURN type.type`) on both engines; the polars residual decline for that shape is gone.
26+
* GFQL polars: an unnamed, untyped single-hop chain kept duplicate node rows (a node table carrying the same id twice) where pandas, cuDF and every other polars shape collapse them; the plain single-hop branch now returns one row per node id (#2051).
2327
- **cuDF 26.2 compatibility: `cudf.from_pandas` replaces the removed `cudf.DataFrame.from_pandas` at the five cuDF-only product sites (`ai_utils`, `umap_utils`, `feature_utils`) and in the test fixtures (#2043)**; the three cuDF chain differential cases that disagree with the full path on cuDF 26.2 are marked expected-failure on that line with the tracking issue, so the GPU lane reports them instead of crashing before them.
2428
- **GFQL pandas/cuDF: several single-alias `IN` (and other pushed-down) predicates across a hop no longer raise `Unalignable boolean Series` (#2020)**: the predicate pushdown filtered an alias frame by label after an earlier pushdown had already narrowed it, while the mask it evaluated carried a fresh positional index. Rows are now kept by position, which is the contract of a mask computed on the same rows; results equal the polars engine and the scalar `=` form.
2529
- **GFQL polars: a native chain whose edge alias shares its name with the column that step filters on is served instead of raising `incompatible-column-type` (#2039)**: the backward pass and the pruned re-execution now run each step on the graph's original edge columns, and a node step whose alias names its own filtered column filters the graph's node values rather than the marker of an earlier pass, so a stamped alias marker is never re-filtered as that column; pandas and polars return the same rows. A cross-engine collision matrix pins the single-hop shapes and records the multi-hop (#2049) and binding-column (#2050) forms as expected failures. The Cypher rows-route projection of such an alias on polars still declines with a typed error (pinned) and stays tracked.

bin/test-polars.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ POLARS_TEST_FILES=(
122122
graphistry/tests/compute/gfql/lazy/engine/polars/test_chain_duplicate_node_rows_2051.py
123123
graphistry/tests/compute/gfql/cypher/test_variable_column_collision.py
124124
graphistry/tests/compute/test_chain_alias_column_collision.py
125+
graphistry/tests/compute/test_gfql_op_list_hides_internal_columns.py
125126
graphistry/tests/compute/gfql/test_engine_polars_semi_key_dedup.py
126127
graphistry/tests/compute/gfql/test_engine_polars_call_modality.py
127128
graphistry/tests/compute/gfql/test_engine_polars_gpu.py

docs/source/gfql/spec/language.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,7 @@ For Python accessor details (including row-pipeline result materialization), see
615615

616616
### Named Results
617617

618-
Operations with `name` parameter add boolean columns to mark matched entities:
618+
Operations with `name` parameter add boolean columns to mark matched entities. The marker takes the name: an existing column of that name on the same frame is replaced (later definitions win, as with dataframe assignment). A `name` equal to that frame's binding column (the node id, or the edge source, destination or id) is rejected with a validation error, since the binding is structural and cannot be shadowed.
619619

620620
```python
621621
result = g.gfql([

graphistry/compute/chain.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,6 +1027,54 @@ def _materialize_fast_path_graph() -> Plottable:
10271027
g.nodes(nodes).edges(edges), alias_n0, alias_e1, alias_n2, src, dst, node, direction)
10281028

10291029

1030+
def reject_alias_named_like_binding(
1031+
g: Plottable, chain_obj: "Chain", *, include_edge_endpoint_aliases: bool = False
1032+
) -> None:
1033+
"""Typed decline for an alias named after a binding column: a node alias equal to the
1034+
node-ID binding, and (native chains and Cypher alike) an edge alias equal to the source,
1035+
destination or edge-ID binding.
1036+
1037+
The alias marker is stamped as ``<alias> = True``, so an alias equal to the node-id
1038+
column overwrites the ids themselves: pandas then died with a raw
1039+
``ValueError: The column label 'id' is not unique`` from the chain's own merge while
1040+
polars answered ``True``. Neither is a usable result; decline the same way on both.
1041+
"""
1042+
from graphistry.compute.exceptions import ErrorCode, GFQLValidationError
1043+
node_id = getattr(g, "_node", None)
1044+
endpoint_cols = {
1045+
col for col in (getattr(g, "_source", None), getattr(g, "_destination", None), getattr(g, "_edge", None))
1046+
if isinstance(col, str)
1047+
}
1048+
for op in chain_obj.chain:
1049+
if isinstance(node_id, str) and isinstance(op, ASTNode) and getattr(op, "_name", None) == node_id:
1050+
raise GFQLValidationError(
1051+
ErrorCode.E108,
1052+
"A node alias cannot be named after the node-ID binding column",
1053+
field="chain.name",
1054+
value=node_id,
1055+
suggestion=(
1056+
f"The alias flag is materialized as a column named '{node_id}', which would "
1057+
f"overwrite the node-ID binding. Rename the alias."
1058+
),
1059+
)
1060+
if (
1061+
include_edge_endpoint_aliases
1062+
and isinstance(op, ASTEdge)
1063+
and getattr(op, "_name", None) in endpoint_cols
1064+
):
1065+
raise GFQLValidationError(
1066+
ErrorCode.E108,
1067+
"An edge alias cannot be named after an edge endpoint binding column",
1068+
field="chain.name",
1069+
value=getattr(op, "_name", None),
1070+
suggestion=(
1071+
"The alias flag is materialized as a column named like the edge "
1072+
"source, destination or edge-ID binding, which would overwrite it. "
1073+
"Rename the alias."
1074+
),
1075+
)
1076+
1077+
10301078
@otel_traced("gfql.chain", attrs_fn=_chain_otel_attrs)
10311079
def chain(
10321080
self: Plottable,
@@ -1088,6 +1136,7 @@ def _chain_with_strictness(
10881136
# _coerce_input_formats then converts input formats (polars, arrow, spark, dask) to that engine.
10891137
if isinstance(engine, str):
10901138
engine = EngineAbstract(engine)
1139+
reject_alias_named_like_binding(self, ops if isinstance(ops, Chain) else Chain(ops), include_edge_endpoint_aliases=True)
10911140
from graphistry.compute.ComputeMixin import _coerce_input_formats # lazy — avoids circular import
10921141
engine_concrete_early = resolve_engine(engine, self)
10931142
if engine_concrete_early in (Engine.POLARS, Engine.POLARS_GPU):

graphistry/compute/gfql/identifiers.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
from contextlib import contextmanager
2+
from contextvars import ContextVar
3+
from typing import Iterator
14
"""GFQL reserved identifiers and validation."""
25

36
import re
@@ -152,3 +155,21 @@ def validate_column_references(
152155
'dst', 'dest', 'destination', 'to', 'target',
153156
'type', 'label', 'name'
154157
}
158+
159+
160+
_SHADOW_RESTORE_WANTED: "ContextVar[bool]" = ContextVar("gfql_shadow_restore_wanted", default=False)
161+
162+
163+
def shadow_restore_wanted() -> bool:
164+
"""Whether the executing pipeline will read shadowed alias values back (the Cypher row pipeline does; the op-list surface does not)."""
165+
return _SHADOW_RESTORE_WANTED.get()
166+
167+
168+
@contextmanager
169+
def cypher_pipeline() -> Iterator[None]:
170+
"""Mark a compiled Cypher execution: chains keep shadowed alias values under the restore column for the row pipeline."""
171+
token = _SHADOW_RESTORE_WANTED.set(True)
172+
try:
173+
yield
174+
finally:
175+
_SHADOW_RESTORE_WANTED.reset(token)

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
from .degrees import get_degrees_polars, get_indegrees_polars, get_outdegrees_polars
3030
from .predicates import filter_by_dict_polars
3131
from .reserved_columns import CHAIN_NODE_HOP
32-
from graphistry.compute.gfql.identifiers import shadow_restore_column
32+
from graphistry.compute.gfql.identifiers import shadow_restore_column, shadow_restore_wanted
3333

3434

3535
def _polars_error_types() -> Tuple[Type[BaseException], ...]:
@@ -370,9 +370,12 @@ def _combine_edges(g: "_LazyShim",
370370
for op, g_step in label_steps:
371371
if op._name is not None and isinstance(op, ASTEdge) and g_step._edges is not None and op._name in colnames(g_step._edges):
372372
named = g_step._edges.filter(pl.col(op._name)).select(pl.col(edge_id)).with_columns(pl.lit(True).alias(op._name))
373-
if op._name in colnames(out): # the alias marker shadows the user column; its values stay under the restore name for Cypher scoping
374-
restore = shadow_restore_column(op._name)
375-
out = out.drop([c for c in (restore,) if c in colnames(out)]).rename({op._name: restore})
373+
if op._name in colnames(out): # the alias marker shadows the user column; only a Cypher pipeline keeps its values, under the restore name
374+
if shadow_restore_wanted():
375+
restore = shadow_restore_column(op._name)
376+
out = out.drop([c for c in (restore,) if c in colnames(out)]).rename({op._name: restore})
377+
else:
378+
out = out.drop(op._name)
376379
out = out.join(named, on=edge_id, how="left").with_columns(pl.col(op._name).fill_null(False))
377380
return out
378381

@@ -1072,6 +1075,7 @@ def _filter_ids(node_op: ASTNode) -> "Optional[PolarsFrame]":
10721075
endpoint_ids(edges, scol, dcol, ncol), on=ncol, how="semi")
10731076
else:
10741077
nodes = gf._nodes.join(endpoints, on=ncol, how="semi")
1078+
nodes = nodes.unique(subset=[ncol], maintain_order=True) # one row per node id, as the full chain and pandas collapse
10751079
return gf.nodes(nodes, ncol).edges(_restore_edge_dtypes(edges, scol, dcol, restore), scol, dcol)
10761080

10771081
if start_nodes is not None:

graphistry/compute/gfql_fast_paths.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3576,7 +3576,8 @@ def _execute_seeded_node_lookup_fast_path(
35763576
if edges is not None:
35773577
out._edges = edges.head(0) if is_polars else edges.head(0).reset_index(drop=True)
35783578
if suffix_ops:
3579-
return chain_impl(out, suffix_ops, engine=engine, policy=policy, context=context)
3579+
tail_ops: List[ASTObject] = list(suffix_ops)
3580+
return chain_impl(out, tail_ops, engine=engine, policy=policy, context=context)
35803581
return out
35813582
assert projection is not None
35823583
if is_polars:
@@ -3819,7 +3820,8 @@ def _execute_seeded_typed_hop_fast_path(
38193820
out._nodes = out_frame
38203821
out._edges = _empty_edges_with_alias_marker(_edges, e1._name, is_polars)
38213822
if suffix_ops:
3822-
return chain_impl(out, suffix_ops, engine=engine, policy=policy, context=context)
3823+
tail_ops: List[ASTObject] = list(suffix_ops)
3824+
return chain_impl(out, tail_ops, engine=engine, policy=policy, context=context)
38233825
return out
38243826
if bag_rows:
38253827
p_rows = _seeded_typed_hop_bag_rows(
@@ -3854,9 +3856,10 @@ def _execute_seeded_typed_hop_fast_path(
38543856
# edges are the matched hop edges, so take their zero-row head.
38553857
out._edges = _empty_edges_with_alias_marker(_edges, e1._name, is_polars)
38563858
if suffix_ops:
3859+
tail_ops_indexed: List[ASTObject] = list(suffix_ops)
38573860
return chain_impl(
38583861
out,
3859-
suffix_ops,
3862+
tail_ops_indexed,
38603863
engine=engine,
38613864
policy=policy,
38623865
context=context,

graphistry/compute/gfql_unified.py

Lines changed: 18 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from .ast import ASTObject, ASTLet, ASTNode, ASTEdge, ASTCall
1515
from .chain import Chain, chain as chain_impl
1616
from .gfql.query_types import GFQLQuery
17+
from .chain import reject_alias_named_like_binding
1718
from .chain_let import chain_let as chain_let_impl
1819
from .execution_context import ExecutionContext
1920
from .gfql.policy import (
@@ -25,7 +26,7 @@
2526
QueryType,
2627
expand_policy
2728
)
28-
from graphistry.compute.gfql.identifiers import TRAIL_ARM_EDGE_ALIAS_PREFIX
29+
from graphistry.compute.gfql.identifiers import cypher_pipeline, TRAIL_ARM_EDGE_ALIAS_PREFIX
2930
from graphistry.compute.gfql.same_path_types import (
3031
EDGE_IDENTITY_COLUMN,
3132
NODE_IDENTITY_COLUMN,
@@ -763,7 +764,7 @@ def _apply_connected_match_join(
763764
cache_store: Dict[str, Any] = {}
764765

765766
for pattern_chain in plan.pattern_chains:
766-
_reject_node_alias_shadowing_id_binding(
767+
reject_alias_named_like_binding(
767768
base_graph, pattern_chain, include_edge_endpoint_aliases=True
768769
)
769770

@@ -1591,7 +1592,7 @@ def _execute_compiled_query_chain_non_union(
15911592
compiled_query=compiled_query,
15921593
engine=engine,
15931594
)
1594-
_reject_node_alias_shadowing_id_binding(
1595+
reject_alias_named_like_binding(
15951596
base_graph, compiled_query.chain, include_edge_endpoint_aliases=True
15961597
)
15971598

@@ -1688,6 +1689,19 @@ def _execute_compiled_query_with_reentry(
16881689
engine: Union[EngineAbstract, str],
16891690
policy: Optional[PolicyDict],
16901691
context: ExecutionContext,
1692+
) -> Plottable:
1693+
with cypher_pipeline():
1694+
return _execute_compiled_query_with_reentry_impl(
1695+
base_graph, compiled_query=compiled_query, engine=engine, policy=policy, context=context)
1696+
1697+
1698+
def _execute_compiled_query_with_reentry_impl(
1699+
base_graph: Plottable,
1700+
*,
1701+
compiled_query: Union[CompiledCypherQuery, CompiledCypherUnionQuery],
1702+
engine: Union[EngineAbstract, str],
1703+
policy: Optional[PolicyDict],
1704+
context: ExecutionContext,
16911705
) -> Plottable:
16921706
if isinstance(compiled_query, CompiledCypherUnionQuery):
16931707
return _execute_compiled_query(
@@ -2765,51 +2779,6 @@ def _gfql_with_strictness(
27652779
context.policy_depth = policy_depth
27662780

27672781

2768-
def _reject_node_alias_shadowing_id_binding(
2769-
g: Plottable, chain_obj: Chain, *, include_edge_endpoint_aliases: bool = False
2770-
) -> None:
2771-
"""Typed decline for a node alias named after the node-ID binding column.
2772-
2773-
The alias marker is stamped as ``<alias> = True``, so an alias equal to the node-id
2774-
column overwrites the ids themselves: pandas then died with a raw
2775-
``ValueError: The column label 'id' is not unique`` from the chain's own merge while
2776-
polars answered ``True``. Neither is a usable result; decline the same way on both.
2777-
"""
2778-
node_id = getattr(g, "_node", None)
2779-
endpoint_cols = {
2780-
col for col in (getattr(g, "_source", None), getattr(g, "_destination", None))
2781-
if isinstance(col, str)
2782-
}
2783-
for op in chain_obj.chain:
2784-
if isinstance(node_id, str) and isinstance(op, ASTNode) and getattr(op, "_name", None) == node_id:
2785-
raise GFQLValidationError(
2786-
ErrorCode.E108,
2787-
"A node alias cannot be named after the node-ID binding column",
2788-
field="chain.name",
2789-
value=node_id,
2790-
suggestion=(
2791-
f"The alias flag is materialized as a column named '{node_id}', which would "
2792-
f"overwrite the node-ID binding. Rename the alias."
2793-
),
2794-
)
2795-
# Cypher-only decline; raw GFQL chains keep their documented overwrite parity.
2796-
if (
2797-
include_edge_endpoint_aliases
2798-
and isinstance(op, ASTEdge)
2799-
and getattr(op, "_name", None) in endpoint_cols
2800-
):
2801-
raise GFQLValidationError(
2802-
ErrorCode.E108,
2803-
"An edge alias cannot be named after an edge endpoint binding column",
2804-
field="chain.name",
2805-
value=getattr(op, "_name", None),
2806-
suggestion=(
2807-
"The alias flag is materialized as a column named like the edge "
2808-
"source/destination binding, which would overwrite the endpoints. "
2809-
"Rename the alias."
2810-
),
2811-
)
2812-
28132782

28142783
def _chain_dispatch(
28152784
g: Plottable,
@@ -2819,7 +2788,7 @@ def _chain_dispatch(
28192788
context: ExecutionContext,
28202789
start_nodes: Optional[DataFrameT] = None,
28212790
) -> Plottable:
2822-
_reject_node_alias_shadowing_id_binding(g, chain_obj)
2791+
reject_alias_named_like_binding(g, chain_obj, include_edge_endpoint_aliases=True)
28232792
engine_name = engine.value if hasattr(engine, "value") else str(engine)
28242793
if chain_obj.where and engine_name in (Engine.POLARS.value, Engine.POLARS_GPU.value):
28252794
# Cross-entity / same-path WHERE routes through DFSamePathExecutor

graphistry/tests/compute/gfql/lazy/engine/polars/test_chain_duplicate_node_rows_2051.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
Sibling of #1993 (polars ``hop()`` de-dups its node table): the unnamed, untyped single-hop
44
chain shape still keeps the duplicate (#2051, strict expected failure); the named, typed,
55
multi-hop and undirected shapes and ``hop()`` already collapse it and are pinned green.
6+
chain shape kept the duplicate (#2051); every shape now collapses it, pinned against pandas.
67
"""
78
import pandas as pd
89
import pytest
@@ -47,7 +48,6 @@ def test_hop_collapses_the_duplicate():
4748

4849

4950
@pytest.mark.parametrize("shape", ["unnamed untyped single hop", "unnamed single hop with destination filter"])
50-
@pytest.mark.xfail(strict=True, reason="graphistry/pygraphistry#2051")
5151
def test_unnamed_untyped_single_hop_collapses_the_duplicate(shape):
5252
g_pd, g_pl = _graph()
5353
ops = [n({"key": 1}), e_forward(), n()] if shape == "unnamed untyped single hop" else [n({"key": 1}), e_forward(), n({"id": 20})]

0 commit comments

Comments
 (0)