fix(cypher): make #1273 whole-row boundary route-independent (#1391)

Author: lmeyerov

graphistry/compute/gfql/cypher/lowering.py

@@ -3202,6 +3202,36 @@ def _query_requires_general_lowering_for_connected_join(
     )
 
 
+def _reject_unsupported_multi_alias_whole_row_cross_alias_where(
+    query: CypherQuery,
+    *,
+    merged_match: Optional[MatchClause],
+    alias_targets: Mapping[str, ASTObject],
+) -> None:
+    if (
+        merged_match is None
+        or query.where is None
+        or _cartesian_node_only_patterns(merged_match) is not None
+        or _match_relationship_count(merged_match) == 0
+        or len({item.expression.text for item in query.return_.items if item.expression.text in alias_targets}) <= 1
+    ):
+        return
+    if any(
+        isinstance(predicate.left, PropertyRef)
+        and isinstance(predicate.right, PropertyRef)
+        and predicate.left.alias != predicate.right.alias
+        and predicate.left.alias in alias_targets
+        and predicate.right.alias in alias_targets
+        for predicate in query.where.predicates
+        if not isinstance(predicate, WherePatternPredicate)
+    ):
+        raise _unsupported(
+            "Cypher row lowering currently supports one MATCH source alias at a time; for remaining multi-source residuals see issue #1273",
+            field=query.return_.kind,
+            value=[item.expression.text for item in query.return_.items],
+            line=query.return_.span.line,
+            column=query.return_.span.column,
+        )
 def _query_has_aggregate_stage(
     query: CypherQuery,
     *,
@@ -3215,8 +3245,6 @@ def _query_has_aggregate_stage(
             if _post_aggregate_expr_plan(item, params=params, alias_targets={}) is not None:
                 return True
     return False
-
-
 def _binding_row_aliases_for_match(
     clause: Optional[MatchClause],
     *,
@@ -6481,10 +6509,6 @@ def _lower_general_row_projection(
     alias_targets = _alias_target(lowered.query) if query.match is not None else {}
     merged_match = _merged_match_clause(query)
     relationship_count = _match_relationship_count(merged_match) if merged_match is not None else 0
-    if query.match is not None and _cartesian_node_only_patterns(query.match) is None and query.where is not None and relationship_count > 0:
-        whole_row_return_aliases = sum(1 for item in query.return_.items if item.expression.text in alias_targets)
-        if whole_row_return_aliases > 1 and any(not isinstance(predicate, WherePatternPredicate) and isinstance(predicate.right, PropertyRef) for predicate in query.where.predicates):
-            raise _unsupported("Cypher row lowering currently supports one MATCH source alias at a time; for remaining multi-source residuals see issue #1273", field=query.return_.kind, value=[item.expression.text for item in query.return_.items], line=query.return_.span.line, column=query.return_.span.column)
     aggregate_specs: List[_AggregateSpec] = []
     non_aggregate_items: List[ReturnItem] = []
     post_aggregate_items: List[_PostAggregateExprPlan] = []
@@ -7377,8 +7401,7 @@ def rewrite_text(expr: ExpressionText, field: str) -> ExpressionText:
         bare_collected.update(bare)
         return rewritten
 
-    # Rewrite trailing MATCH expressions (node/edge property maps via WHERE
-    # comes through reentry_wheres / clause.where).
+    # Rewrite trailing MATCH/WHERE expressions.
     rewritten_reentry_matches = tuple(
         _rewrite_reentry_match_clause(clause, rewrite_expr=rewrite_text)
         for clause in query.reentry_matches
@@ -7387,13 +7410,7 @@ def rewrite_text(expr: ExpressionText, field: str) -> ExpressionText:
         where_clause if where_clause is None else _rewrite_where_clause_and_resync(where_clause, rewrite_text, "where")
         for where_clause in query.reentry_wheres
     )
-    # Slice 4.3d.1 (#1256): Drop bare-identifier projection items that simply
-    # forward a secondary alias (`WITH a, x, y, collect(...)` pattern in IC3
-    # multi-stage chains). Their property carries already live as hidden columns
-    # on the reentry-source's row table; the bare items are pure forwarding
-    # noise. Without this pre-clean the bare-ref scanner inside
-    # `_collect_secondary_property_refs` would fail-fast on what is in fact a
-    # forwarding pattern, blocking IC3 even after #1248 admits the prefix WITH.
+    # Drop bare secondary-alias forwarding items before property-ref rewriting.
     secondary_forwarding_re = re.compile(r"[A-Za-z_][A-Za-z0-9_]*")
     from graphistry.compute.gfql.cypher.reentry import compiletime as _reentry_compiletime
 
@@ -7432,8 +7449,7 @@ def rewrite_text(expr: ExpressionText, field: str) -> ExpressionText:
             span=query.return_.span,
         )
 
-    # Synthesize prefix WITH items: drop the secondaries, append S.X AS hidden
-    # for each unique referenced (S, X) pair.
+    # Synthesize prefix WITH items: drop secondary whole-row carries, append hidden S.X.
     new_items: List[ReturnItem] = []
     secondary_drop_indices = {idx for idx, _item in secondary_items}
     for idx, item in enumerate(prefix_stage.clause.items):
@@ -7455,28 +7471,8 @@ def rewrite_text(expr: ExpressionText, field: str) -> ExpressionText:
         clause=replace(prefix_stage.clause, items=tuple(new_items)),
     )
 
-    # Slice 4.3d.2 (#1256): forward hidden carry columns through every downstream
-    # WITH stage so each recursive bounded-reentry compile sees them as scalar
-    # carries. Without this, the hidden column survives the first boundary (it
-    # is in the prefix) but the subsequent WITH/RETURN scope-narrowing drops it,
-    # and references like `RETURN x.id AS xid` (rewritten to a bare hidden
-    # identifier) fail at the inner compile's alias resolution.
-    #
-    # Interaction with DISTINCT in downstream stages: appending the carry as a
-    # bare item makes it a participant in DISTINCT key sets. For multi-alias
-    # carry semantics this is what callers want — DISTINCT over `(friend, x.id)`
-    # is the desired behavior when `x.id` is referenced downstream. Multi-row
-    # `x` cases that could observably mutate row count are blocked upstream by
-    # the pre-existing `unique carried node rows` failfast in `gfql_unified.py`.
-    #
-    # Aggregate guard (W2-IMPORTANT-1): if any downstream WITH stage contains
-    # an aggregate function call, refuse to forward the carry through it. The
-    # alternative — silently appending the hidden alias next to `count(*)` — can
-    # produce a wrong NULL value in the projected column when the trailing MATCH
-    # has no relationship to trigger the existing aggregate failfast. Better to
-    # raise a scoped #1256 error pointing at the gap than to risk silent wrong
-    # results. The relationship-pattern aggregate path is also covered by an
-    # earlier failfast; this guard is a single tighter rule that subsumes both.
+    # Forward hidden carry columns through downstream WITH stages, but refuse
+    # aggregate stages to avoid wrong NULL carry behavior (#1256).
     if refs_collected and rewritten_with_stages_tail:
         for stage in rewritten_with_stages_tail:
             for item in stage.clause.items:
@@ -8535,6 +8531,12 @@ def _attach_graph_context(result: CompiledCypherQuery) -> CompiledCypherQuery:
         if merged_match is not None
         else LoweredCypherMatch(query=[], where=[])
     )
+    alias_targets = _alias_target(lowered.query) if query.match is not None else {}
+    _reject_unsupported_multi_alias_whole_row_cross_alias_where(
+        query,
+        merged_match=merged_match,
+        alias_targets=alias_targets,
+    )
 
     def _lower_general() -> CompiledCypherQuery:
         return _lower_general_row_projection(
@@ -8547,7 +8549,6 @@ def _lower_general() -> CompiledCypherQuery:
         )
 
     if query.with_stages:
-        alias_targets = _alias_target(lowered.query)
         binding_row_aliases = _binding_row_aliases_for_match(query.match, alias_targets=alias_targets)
         binding_row_aliases = _apply_bound_scope_membership(
             binding_row_aliases,
@@ -8625,7 +8626,6 @@ def _lower_general() -> CompiledCypherQuery:
         ))
 
     if merged_match is not None and not query.unwinds:
-        alias_targets = _alias_target(lowered.query)
         binding_row_aliases = _binding_row_aliases_for_match(query.match, alias_targets=alias_targets)
         binding_row_aliases = _apply_bound_scope_membership(
             binding_row_aliases,

graphistry/tests/compute/gfql/cypher/test_lowering.py

@@ -11381,6 +11381,34 @@ def test_multi_alias_connected_whole_row_return_with_cross_alias_where_still_fai
     assert "#1273" in exc_info.value.message
 
 
+def test_multi_alias_connected_cross_alias_where_scalar_projection_remains_supported() -> None:
+    g = _mk_graph(
+        pd.DataFrame(
+            {
+                "id": ["n1", "n2", "x1", "x2"],
+                "animal": ["cat", "dog", "cat", "wolf"],
+            }
+        ),
+        pd.DataFrame({"s": ["n1", "n2"], "d": ["x1", "x2"], "type": ["R", "R"]}),
+    )
+    result = g.gfql("MATCH (n)-[rel]->(x) WHERE n.animal = x.animal RETURN n.id AS n_id, x.id AS x_id")
+    assert result._nodes.to_dict(orient="records") == [{"n_id": "n1", "x_id": "x1"}]
+
+
+def test_multi_alias_connected_cross_alias_where_single_whole_row_projection_remains_supported() -> None:
+    g = _mk_graph(
+        pd.DataFrame(
+            {
+                "id": ["n1", "n2", "x1", "x2"],
+                "animal": ["cat", "dog", "cat", "wolf"],
+            }
+        ),
+        pd.DataFrame({"s": ["n1", "n2"], "d": ["x1", "x2"], "type": ["R", "R"]}),
+    )
+    result = g.gfql("MATCH (n)-[rel]->(x) WHERE n.animal = x.animal RETURN n, x.id AS x_id")
+    assert result._nodes.to_dict(orient="records") == [{"n": "({animal: 'cat'})", "x_id": "x1"}]
+
+
 def test_compile_cypher_tracks_seeded_top_level_row_query() -> None:
     compiled = _compile_query("UNWIND [1, 2, 3] AS x RETURN x ORDER BY x DESC LIMIT 2")