diff --git a/bin/ci_comment_density_baseline.json b/bin/ci_comment_density_baseline.json index c2172b72e7..88f6e8e868 100644 --- a/bin/ci_comment_density_baseline.json +++ b/bin/ci_comment_density_baseline.json @@ -14,14 +14,14 @@ "graphistry/compute/chain.py": 31, "graphistry/compute/chain_fast_paths.py": 10, "graphistry/compute/chain_lean_combine.py": 4, - "graphistry/compute/chain_let.py": 15, + "graphistry/compute/chain_let.py": 9, "graphistry/compute/chain_remote.py": 2, "graphistry/compute/cluster.py": 2, "graphistry/compute/collapse.py": 4, "graphistry/compute/conditional.py": 1, "graphistry/compute/dataframe/join.py": 1, "graphistry/compute/engine_coercion.py": 2, - "graphistry/compute/filter_by_dict.py": 6, + "graphistry/compute/filter_by_dict.py": 5, "graphistry/compute/gfql/agg_types.py": 1, "graphistry/compute/gfql/call/executor.py": 7, "graphistry/compute/gfql/call/support.py": 1, @@ -29,7 +29,7 @@ "graphistry/compute/gfql/cypher/_boolean_expr_text.py": 1, "graphistry/compute/gfql/cypher/ast.py": 5, "graphistry/compute/gfql/cypher/ast_normalizer.py": 1, - "graphistry/compute/gfql/cypher/lowering.py": 50, + "graphistry/compute/gfql/cypher/lowering.py": 49, "graphistry/compute/gfql/cypher/parser.py": 24, "graphistry/compute/gfql/cypher/reentry/compiletime.py": 2, "graphistry/compute/gfql/cypher/reentry/execution.py": 5, @@ -49,24 +49,24 @@ "graphistry/compute/gfql/index/explain.py": 1, "graphistry/compute/gfql/index/lookup.py": 1, "graphistry/compute/gfql/index/registry.py": 4, - "graphistry/compute/gfql/index/traverse.py": 16, + "graphistry/compute/gfql/index/traverse.py": 15, "graphistry/compute/gfql/index/types.py": 3, "graphistry/compute/gfql/index/wire.py": 2, "graphistry/compute/gfql/ir/pushdown_safety.py": 5, "graphistry/compute/gfql/ir/query_graph.py": 10, "graphistry/compute/gfql/ir/verifier.py": 3, "graphistry/compute/gfql/lazy/__init__.py": 4, - "graphistry/compute/gfql/lazy/engine/polars/chain.py": 48, + "graphistry/compute/gfql/lazy/engine/polars/chain.py": 47, "graphistry/compute/gfql/lazy/engine/polars/degrees.py": 3, - "graphistry/compute/gfql/lazy/engine/polars/dtypes.py": 6, + "graphistry/compute/gfql/lazy/engine/polars/dtypes.py": 3, "graphistry/compute/gfql/lazy/engine/polars/hop.py": 2, "graphistry/compute/gfql/lazy/engine/polars/hop_eager.py": 19, "graphistry/compute/gfql/lazy/engine/polars/lowering_context.py": 2, "graphistry/compute/gfql/lazy/engine/polars/nan_clean.py": 2, "graphistry/compute/gfql/lazy/engine/polars/pattern_apply.py": 9, - "graphistry/compute/gfql/lazy/engine/polars/predicates.py": 19, - "graphistry/compute/gfql/lazy/engine/polars/projection.py": 6, - "graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py": 86, + "graphistry/compute/gfql/lazy/engine/polars/predicates.py": 18, + "graphistry/compute/gfql/lazy/engine/polars/projection.py": 5, + "graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py": 84, "graphistry/compute/gfql/lazy/engine/polars/search.py": 3, "graphistry/compute/gfql/lazy/engine/polars/varlen_rows.py": 1, "graphistry/compute/gfql/logical_planner.py": 1, @@ -77,7 +77,7 @@ "graphistry/compute/gfql/row/entity_props.py": 1, "graphistry/compute/gfql/row/frame_ops.py": 6, "graphistry/compute/gfql/row/ordering.py": 1, - "graphistry/compute/gfql/row/pipeline.py": 44, + "graphistry/compute/gfql/row/pipeline.py": 40, "graphistry/compute/gfql/same_path/multihop.py": 1, "graphistry/compute/gfql/same_path/native_shortest_path.py": 3, "graphistry/compute/gfql/search_any.py": 4, @@ -85,7 +85,7 @@ "graphistry/compute/gfql/temporal/constructors.py": 2, "graphistry/compute/gfql/temporal_text.py": 1, "graphistry/compute/gfql/validate.py": 1, - "graphistry/compute/gfql_fast_paths.py": 65, + "graphistry/compute/gfql_fast_paths.py": 64, "graphistry/compute/gfql_unified.py": 21, "graphistry/compute/gfql_validate.py": 3, "graphistry/compute/graph_operation.py": 1, @@ -96,7 +96,7 @@ "graphistry/compute/predicates/numeric.py": 1, "graphistry/compute/predicates/str.py": 8, "graphistry/compute/python_remote.py": 1, - "graphistry/compute/typing.py": 4, + "graphistry/compute/typing.py": 3, "graphistry/compute/validate/validate_schema.py": 2, "graphistry/compute/validate_schema.py": 1, "graphistry/constants.py": 5, @@ -111,7 +111,7 @@ "graphistry/layout/circle.py": 17, "graphistry/layout/gib/_squarify.py": 3, "graphistry/layout/gib/gib.py": 1, - "graphistry/layout/gib/partitioned_layout.py": 2, + "graphistry/layout/gib/partitioned_layout.py": 1, "graphistry/layout/graph/graph.py": 18, "graphistry/layout/graph/graphBase.py": 5, "graphistry/layout/graph/vertex.py": 1, @@ -150,7 +150,6 @@ "graphistry/compute/chain_lean_combine.py": 2, "graphistry/compute/gfql/agg_types.py": 1, "graphistry/compute/gfql/call/executor.py": 3, - "graphistry/compute/gfql/cypher/lowering.py": 1, "graphistry/compute/gfql/cypher/parser.py": 1, "graphistry/compute/gfql/cypher/row_pushdown.py": 1, "graphistry/compute/gfql/expr_parser.py": 2, @@ -165,17 +164,17 @@ "graphistry/compute/gfql/index/registry.py": 5, "graphistry/compute/gfql/index/traverse.py": 9, "graphistry/compute/gfql/lazy/__init__.py": 7, - "graphistry/compute/gfql/lazy/engine/polars/chain.py": 8, + "graphistry/compute/gfql/lazy/engine/polars/chain.py": 7, "graphistry/compute/gfql/lazy/engine/polars/degrees.py": 1, - "graphistry/compute/gfql/lazy/engine/polars/hop_eager.py": 3, - "graphistry/compute/gfql/lazy/engine/polars/nan_clean.py": 3, + "graphistry/compute/gfql/lazy/engine/polars/hop_eager.py": 2, + "graphistry/compute/gfql/lazy/engine/polars/nan_clean.py": 2, "graphistry/compute/gfql/lazy/engine/polars/pattern_apply.py": 1, "graphistry/compute/gfql/lazy/engine/polars/predicates.py": 1, "graphistry/compute/gfql/lazy/engine/polars/projection.py": 1, "graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py": 4, - "graphistry/compute/gfql/lazy/engine/polars/varlen_rows.py": 4, + "graphistry/compute/gfql/lazy/engine/polars/varlen_rows.py": 3, "graphistry/compute/gfql/row/ordering.py": 1, - "graphistry/compute/gfql/row/pipeline.py": 9, + "graphistry/compute/gfql/row/pipeline.py": 8, "graphistry/compute/gfql/same_path/native_shortest_path.py": 1, "graphistry/compute/gfql/temporal/constructors.py": 1, "graphistry/compute/gfql_fast_paths.py": 17, @@ -183,7 +182,7 @@ "graphistry/compute/hop.py": 4, "graphistry/feature_utils.py": 1, "graphistry/layout/gib/gib.py": 1, - "graphistry/layout/gib/partitioned_layout.py": 2, + "graphistry/layout/gib/partitioned_layout.py": 1, "graphistry/layout/graph/__init__.py": 1, "graphistry/layout/graph/edge.py": 1, "graphistry/layout/graph/edgeBase.py": 1, @@ -243,7 +242,7 @@ "graphistry/compute/gfql/cypher/_boolean_expr_text.py": 1, "graphistry/compute/gfql/cypher/ast.py": 3, "graphistry/compute/gfql/cypher/ast_normalizer.py": 1, - "graphistry/compute/gfql/cypher/lowering.py": 18, + "graphistry/compute/gfql/cypher/lowering.py": 10, "graphistry/compute/gfql/cypher/parser.py": 7, "graphistry/compute/gfql/cypher/reentry/execution.py": 2, "graphistry/compute/gfql/cypher/reentry/flatten.py": 1, @@ -255,21 +254,20 @@ "graphistry/compute/gfql/index/engine_arrays.py": 1, "graphistry/compute/gfql/ir/arrow_bridge.py": 1, "graphistry/compute/gfql/ir/metadata.py": 1, - "graphistry/compute/gfql/lazy/engine/polars/chain.py": 12, + "graphistry/compute/gfql/lazy/engine/polars/chain.py": 10, "graphistry/compute/gfql/lazy/engine/polars/degrees.py": 1, - "graphistry/compute/gfql/lazy/engine/polars/hop.py": 2, + "graphistry/compute/gfql/lazy/engine/polars/hop.py": 1, "graphistry/compute/gfql/lazy/engine/polars/hop_eager.py": 4, - "graphistry/compute/gfql/lazy/engine/polars/nan_clean.py": 1, "graphistry/compute/gfql/lazy/engine/polars/pattern_apply.py": 1, "graphistry/compute/gfql/lazy/engine/polars/projection.py": 6, "graphistry/compute/gfql/lazy/engine/polars/reserved_columns.py": 1, - "graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py": 12, + "graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py": 10, "graphistry/compute/gfql/lazy/engine/polars/varlen_rows.py": 1, "graphistry/compute/gfql/passes/predicate_pushdown.py": 1, "graphistry/compute/gfql/rollout.py": 1, "graphistry/compute/gfql/row/entity_props.py": 1, "graphistry/compute/gfql/row/ordering.py": 2, - "graphistry/compute/gfql/row/pipeline.py": 9, + "graphistry/compute/gfql/row/pipeline.py": 2, "graphistry/compute/gfql/series_str_compat.py": 2, "graphistry/compute/gfql_fast_paths.py": 9, "graphistry/compute/gfql_unified.py": 11, diff --git a/bin/ci_cypher_surface_guard_baseline.json b/bin/ci_cypher_surface_guard_baseline.json index 4fd4802055..3c155b9644 100644 --- a/bin/ci_cypher_surface_guard_baseline.json +++ b/bin/ci_cypher_surface_guard_baseline.json @@ -13,5 +13,5 @@ "max_properties": 0 } }, - "lowering_py_max_lines": 9645 + "lowering_py_max_lines": 9710 } diff --git a/bin/test-polars.sh b/bin/test-polars.sh index c370667ab7..0b353c39ee 100755 --- a/bin/test-polars.sh +++ b/bin/test-polars.sh @@ -50,6 +50,8 @@ POLARS_TEST_FILES=( graphistry/tests/compute/gfql/test_optional_match_with_pipeline_boundaries.py graphistry/tests/compute/gfql/test_row_multiplicity_semantics.py graphistry/tests/compute/gfql/test_numeric_conformance_semantics.py + graphistry/tests/compute/gfql/test_path_trail_semantics.py + graphistry/tests/compute/gfql/row/test_row_pipeline_boundaries.py graphistry/tests/compute/gfql/test_unary_op_surface.py graphistry/tests/compute/gfql/test_hop_boundary_matrix.py graphistry/tests/compute/gfql/test_hop_semantics_pins.py diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index 585686eb5e..73a5aab078 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -149,6 +149,7 @@ from graphistry.compute.gfql.same_path_types import NODE_IDENTITY_COLUMN, WhereComparison, col, compare, where_to_row_expr from graphistry.compute.gfql.cypher.reentry import naming as _reentry_naming, scope as _reentry_scope from graphistry.compute.gfql.cypher.ast import CypherParams +from graphistry.compute.gfql.identifiers import shortest_path_hops_column @dataclass(frozen=True) @@ -2604,10 +2605,10 @@ def _forces_relationship_multiplicity_projection_bindings( for pattern in clause.patterns: for element in pattern: if isinstance(element, RelationshipPattern) and ( - element.min_hops is not None - or element.max_hops is not None - or getattr(element, "to_fixed_point", False) + getattr(element, "to_fixed_point", False) + and element.direction == "undirected" ): + # An undirected unbounded fixed point is not reconstructible from binding rows. return False texts = [item.expression.text for item in items] if order_by is not None: @@ -2640,6 +2641,9 @@ def _forces_relationship_multiplicity_projection_bindings( len(pattern) == 3 and isinstance(pattern[0], NodePattern) and isinstance(pattern[1], RelationshipPattern) + and pattern[1].min_hops is None + and pattern[1].max_hops is None + and not getattr(pattern[1], "to_fixed_point", False) and isinstance(pattern[2], NodePattern) ): seed_alias = pattern[0].variable @@ -4356,6 +4360,36 @@ def _sub(m: "re.Match[str]") -> str: return new_text, calls +def _append_shortest_path_plain_match_filter( + row_steps: List[ASTObject], + *, + query: CypherQuery, + params: Optional[Mapping[str, Any]], # hygiene-ok: explicit-any -- Cypher params mapping, module-wide idiom + binding_rows_active: bool, +) -> None: + """openCypher: a PLAIN-MATCH shortestPath with no path drops the row; only + OPTIONAL MATCH null-extends. The SP binding runtime left-joins (per-pair + null hops), so plain MATCH filters null-hop rows out here.""" + if not binding_rows_active or not _query_has_shortest_path_patterns(query): + return + if any(clause.optional for clause in query.matches): + return + for spec in _shortest_path_alias_specs(query).values(): + if spec.end_alias is None: + continue + expr_text = f"{spec.end_alias}.{spec.hop_column} IS NOT NULL" + row_steps.append( + where_rows( + expr=_row_expr_arg( + ExpressionText(text=expr_text, span=query.return_.span), + params=params, + alias_targets={}, + field="where", + ) + ) + ) + + def _append_match_row_where( row_steps: List[ASTObject], *, @@ -4481,6 +4515,12 @@ def _lower_projection_chain( allowed_match_aliases=(allowed_match_aliases | pre_scope_binding_row_aliases) or None, params=params, ) + _append_shortest_path_plain_match_filter( + row_steps, + query=query, + params=params, + binding_rows_active=bool(binding_row_aliases), + ) if not plan.whole_row_output_names: projection_fn = with_ if plan.clause_kind == "with" else return_ @@ -4524,7 +4564,7 @@ def _build_initial_row_scope( params=params, alias_targets=alias_targets, ) - # Admit first-stage multi-alias non-aggregate WITH projections (shape A, #1273) + # Admit first-stage multi-alias non-aggregate WITH projections (shape A) # by routing through the bindings-row path when multiple MATCH node aliases are # referenced together in scalar expressions. stage_has_aggregates = bool(stage_aggregate_specs) @@ -4557,7 +4597,7 @@ def _build_initial_row_scope( ): binding_row_aliases.update(stage_non_aggregate_refs) # For connected non-cartesian MATCH, allow first-stage multi-whole-row node - # projections to use bindings rows (#880 / #1393). + # projections to use bindings rows. if not binding_row_aliases: binding_row_aliases.update( _binding_row_aliases_for_multi_alias_whole_row_node_projection( @@ -4671,6 +4711,12 @@ def _build_initial_row_scope( allowed_match_aliases=binding_row_aliases or None, params=params, ) + _append_shortest_path_plain_match_filter( + row_steps, + query=query, + params=params, + binding_rows_active=bool(binding_row_aliases), + ) unwind_aliases: Set[str] = set() for unwind_clause in query.unwinds: @@ -4791,7 +4837,7 @@ def _lower_match_alias_stage( elif scope.allowed_match_aliases and plan.projection_items: # Mixed case: whole-row aliases + scalar items on a bindings-row table. # Use extend mode to add scalar columns without dropping the existing - # alias-prefixed bindings columns (#880). + # alias-prefixed bindings columns. row_steps.append(with_(plan.projection_items, extend=True)) if stage.clause.distinct: row_steps.append(distinct()) @@ -4916,7 +4962,7 @@ def _lower_match_alias_aggregate_stage( projection_fn = with_ if stage.clause.kind == "with" else return_ # On the bindings-row path (allowed_match_aliases populated), the row # table preserves per-row multiplicity from the MATCH, so relationship- - # count aggregation guards do not apply (#880). + # count aggregation guards do not apply. if not scope.allowed_match_aliases: _reject_unsound_relationship_multiplicity_aggregates_common( aggregate_specs=aggregate_specs, @@ -6168,7 +6214,7 @@ def _shortest_path_relationship_hop_columns(clause: MatchClause) -> Dict[Tuple[i if isinstance(element, RelationshipPattern): span = element.span out[(span.line, span.column, span.end_line, span.end_column, span.start_pos, span.end_pos)] = ( - f"__cypher_shortest_path_hops__{alias}" + shortest_path_hops_column(alias) ) return out @@ -6996,13 +7042,12 @@ def _lower_general_row_projection( continue if len(refs) > 1 or (len(refs) == 1 and base_active_alias not in refs): # An aggregate over a pattern alias other than the projection's - # active alias. Two sound, benchmark-relevant shapes are routed to - # the bindings-row table (which materializes every alias, one row - # per matched path); everything else keeps the conservative - # fail-fast (the misleading "one MATCH" error is the residual). - # (a) #1708: `count()` — "matched paths binding + # active alias. Two sound shapes route to the bindings-row table + # (which materializes every alias, one row per matched path); + # everything else keeps the conservative fail-fast. + # (a): `count()` — "matched paths binding # this node per group" (graph-bench q1 top-k in-degree). - # (b) #1273: a CLEAN grouped aggregate `func(.)` + # (b): a CLEAN grouped aggregate `func(.)` # (avg/sum/min/max/count) grouped by another alias's property # (graph-bench q3/q4: `RETURN c.city, avg(p.age)`) — a # standard GROUP BY, sound on the per-path bindings rows. @@ -7010,7 +7055,7 @@ def _lower_general_row_projection( # item MIXES a non-aggregate ref with an aggregate in one # expression (`me.age + count(you.age)`), the cross-source # multiplicity is ambiguous — keep the fail-fast (the - # rejects-unsound-multi-source-overlap contract, #1273 tests). + # rejects-unsound-multi-source-overlap contract tests). agg_arg = (agg_spec.expr_text or "").strip() prop_ref = _projection_ref_from_expr_safe(agg_arg, alias_targets) prop_alias = prop_ref[0] if prop_ref is not None else None @@ -7101,6 +7146,12 @@ def _lower_general_row_projection( allowed_match_aliases=binding_row_aliases or None, params=params, ) + _append_shortest_path_plain_match_filter( + row_steps, + query=query, + params=params, + binding_rows_active=bool(binding_row_aliases), + ) unwind_aliases: Set[str] = set() for unwind_clause in query.unwinds: @@ -7506,7 +7557,14 @@ def _lower_general_row_projection( if query.return_.distinct: row_steps.append(distinct()) - if empty_result_row is None and binding_row_aliases and _query_has_shortest_path_patterns(query): + if ( + empty_result_row is None + and binding_row_aliases + and _query_has_shortest_path_patterns(query) + and any(clause.optional for clause in query.matches) + ): + # openCypher: only OPTIONAL MATCH null-extends an unreachable + # shortestPath; a plain MATCH emits NO row. from graphistry.compute.gfql.row.pipeline import _RowPipelineAdapter shortest_specs = _shortest_path_alias_specs(query) @@ -9257,7 +9315,7 @@ def _attach_graph_context(result: CompiledCypherQuery) -> CompiledCypherQuery: # Re-bind after normalization so scope and semantic metadata reflect the # lowered query shape consumed by downstream lowering decisions. - # #1357: strict alias/name-resolution is now the runtime default for the + # Strict alias/name-resolution is the runtime default for the # post-normalize bind pass so alias-scope enforcement is centralized at # binder time (validator/runtime parity). bound_ir = FrontendBinder().bind(query, PlanContext(), strict_name_resolution=True) @@ -9433,7 +9491,14 @@ def _lower_general() -> CompiledCypherQuery: row_steps.extend(stage_steps) empty_result_row: Optional[Dict[str, Any]] = None - if binding_row_aliases and _query_has_shortest_path_patterns(query) and result_projection is None: + if ( + binding_row_aliases + and _query_has_shortest_path_patterns(query) + and result_projection is None + and any(clause.optional for clause in query.matches) + ): + # openCypher: unreachable shortestPath null-extends only under + # OPTIONAL MATCH; plain MATCH drops the row. empty_result_row = _shortest_path_empty_result_row_for_row_steps( row_steps=row_steps, specs=_shortest_path_alias_specs(query), diff --git a/graphistry/compute/gfql/cypher/parser.py b/graphistry/compute/gfql/cypher/parser.py index a2e4788591..1135eb646c 100644 --- a/graphistry/compute/gfql/cypher/parser.py +++ b/graphistry/compute/gfql/cypher/parser.py @@ -144,6 +144,7 @@ rel_types: ":" LABEL_NAME ("|" ":"? LABEL_NAME)* rel_range: "*" INT ".." INT -> rel_range_bounded | "*" INT ".." -> rel_range_open_max + | "*" ".." INT -> rel_range_open_min | "*" INT -> rel_range_exact | "*" -> rel_range_fixed @@ -943,6 +944,21 @@ def rel_range_bounded(self, meta: Any, items: Sequence[Any]) -> dict[str, Any]: ) return {"min_hops": min_hops, "max_hops": max_hops, "to_fixed_point": False} + def rel_range_open_min(self, meta: Any, items: Sequence[Any]) -> dict[str, Any]: # hygiene-ok: explicit-any -- lark transformer callback signature, module-wide idiom + # openCypher [*..M]: omitted lower bound defaults to 1 + if len(items) != 1: + raise _to_syntax_error("Invalid relationship range", line=meta.line, column=meta.column) + max_hops = self._rel_hops(meta, items[0]) + if max_hops < 1: + raise _to_unsupported( + "Cypher relationship ranges require lower bound <= upper bound", + line=meta.line, + column=meta.column, + field="match", + value=self._slice(_span_from_meta(meta)), + ) + return {"min_hops": 1, "max_hops": max_hops, "to_fixed_point": False} + def rel_range_open_max(self, meta: Any, items: Sequence[Any]) -> dict[str, Any]: if len(items) != 1: raise _to_syntax_error("Invalid relationship range", line=meta.line, column=meta.column) diff --git a/graphistry/compute/gfql/cypher/row_pushdown.py b/graphistry/compute/gfql/cypher/row_pushdown.py index cc80d6e4c5..e32f86ab98 100644 --- a/graphistry/compute/gfql/cypher/row_pushdown.py +++ b/graphistry/compute/gfql/cypher/row_pushdown.py @@ -26,6 +26,7 @@ AliasPrefilterSpec, MutableAliasPrefilters, is_alias_prefilters, ) from graphistry.utils.json import JSONVal +from graphistry.compute.gfql.identifiers import is_shortest_path_hops_column from typing_extensions import TypeGuard # Ops that consume/close the post-``rows`` filter region. Reaching any of these @@ -122,7 +123,7 @@ def _binding_alias_targets( # honor alias_prefilters — pushing there would silently drop the filter. # Bail out of the entire pushdown for any such pattern. lnh = op_json.get("label_node_hops") if isinstance(op_json, dict) else None - if isinstance(lnh, str) and lnh.startswith("__cypher_shortest_path_hops__"): + if is_shortest_path_hops_column(lnh): return None try: obj = ast_from_json(op_json, validate=False) diff --git a/graphistry/compute/gfql/cypher/shortest_path_aliases.py b/graphistry/compute/gfql/cypher/shortest_path_aliases.py index ade0d6f072..4a2fb3fd10 100644 --- a/graphistry/compute/gfql/cypher/shortest_path_aliases.py +++ b/graphistry/compute/gfql/cypher/shortest_path_aliases.py @@ -12,6 +12,7 @@ PatternElement, RelationshipPattern, ) +from graphistry.compute.gfql.identifiers import shortest_path_hops_column @dataclass(frozen=True) @@ -78,7 +79,7 @@ def _shortest_path_alias_specs(query: CypherQuery) -> Dict[str, _ShortestPathAli end_alias = pattern[-1].variable if isinstance(pattern[-1], NodePattern) else None out[alias] = _ShortestPathAliasSpec( alias=alias, - hop_column=f"__cypher_shortest_path_hops__{alias}", + hop_column=shortest_path_hops_column(alias), pattern=pattern, start_alias=start_alias, end_alias=end_alias, diff --git a/graphistry/compute/gfql/identifiers.py b/graphistry/compute/gfql/identifiers.py index 44025be3ed..b0aa1220e8 100644 --- a/graphistry/compute/gfql/identifiers.py +++ b/graphistry/compute/gfql/identifiers.py @@ -1,7 +1,7 @@ """GFQL reserved identifiers and validation.""" import re -from typing import Optional, Dict, Any, Set +from typing import Optional, Dict, Any, Final, FrozenSet, Set #: The openCypher identifier form, shared by every surface that has to recognize one. IDENTIFIER_TOKEN = re.compile(r'[A-Za-z_][A-Za-z0-9_]*') @@ -25,6 +25,60 @@ def identifier_tokens(text: str) -> Set[str]: #: Prefix of the internal columns that carry an alias's value under a hidden name. HIDDEN_ALIAS_COLUMN_PREFIX: str = '__gfql_hidden_' +#: Source endpoint of an ORIENTED edge row (``EdgeSemantics.orient_edges`` output). +WALK_FROM_COL: Final[str] = '__from__' + +#: Destination endpoint of an ORIENTED edge row. +WALK_TO_COL: Final[str] = '__to__' + +#: The node a row of the path bag currently stands on. +WALK_CURRENT_COL: Final[str] = '__current__' + +#: The node a path-bag row just left, so a hop can drop an immediate backtrack. +WALK_PREV_COL: Final[str] = '__gfql_prev__' + +#: Stable per-edge identity: openCypher TRAIL semantics bind a relationship at most once per path. +TRAIL_EDGE_IDENT_COL: Final[str] = '__gfql_edge_ident__' + +#: Prefix of the per-hop column recording WHICH relationship that hop bound. +TRAIL_COLUMN_PREFIX: Final[str] = '__gfql_trail_' + +#: Prefix of the hidden hop-count column a ``shortestPath`` pattern binds for its path alias. +SHORTEST_PATH_HOPS_COLUMN_PREFIX: Final[str] = '__cypher_shortest_path_hops__' + +#: Every scratch column the row-binding walk may introduce; none may survive into a result. +WALK_SCRATCH_COLUMNS: FrozenSet[str] = frozenset({ + WALK_FROM_COL, WALK_TO_COL, WALK_CURRENT_COL, WALK_PREV_COL, TRAIL_EDGE_IDENT_COL, +}) + + +def trail_column_name(hop_index: int) -> str: + """Name of the trail column recording the relationship bound at ``hop_index``.""" + return f'{TRAIL_COLUMN_PREFIX}{hop_index}{INTERNAL_COLUMN_SUFFIX}' + + +def is_trail_column(name: str) -> bool: + """Whether ``name`` is a per-hop trail column produced by :func:`trail_column_name`.""" + return (isinstance(name, str) + and name.startswith(TRAIL_COLUMN_PREFIX) + and name.endswith(INTERNAL_COLUMN_SUFFIX) + and name[len(TRAIL_COLUMN_PREFIX):-len(INTERNAL_COLUMN_SUFFIX)].isdigit()) + + +def shortest_path_hops_column(alias: str) -> str: + """Name of the hidden hop-count column bound by the ``shortestPath`` path alias ``alias``.""" + return f'{SHORTEST_PATH_HOPS_COLUMN_PREFIX}{alias}' + + +def is_shortest_path_hops_column(name: object) -> bool: + """Whether ``name`` is a ``shortestPath`` hop-count column, i.e. selects shortestPath mode.""" + return isinstance(name, str) and name.startswith(SHORTEST_PATH_HOPS_COLUMN_PREFIX) + + +def is_walk_scratch_column(name: str) -> bool: + """Whether ``name`` is any walk scratch column (fixed vocabulary or a trail column).""" + return name in WALK_SCRATCH_COLUMNS or is_trail_column(name) + def is_internal_column(name: str) -> bool: """Check if name matches internal column pattern __gfql_*__.""" diff --git a/graphistry/compute/gfql/index/bindings.py b/graphistry/compute/gfql/index/bindings.py index 7466666718..8102e86154 100644 --- a/graphistry/compute/gfql/index/bindings.py +++ b/graphistry/compute/gfql/index/bindings.py @@ -15,6 +15,7 @@ from graphistry.Engine import Engine, df_concat from graphistry.Plottable import Plottable from graphistry.compute.typing import DataFrameT +from graphistry.compute.gfql.identifiers import WALK_CURRENT_COL from .api import ( _record_indexed_traversal, @@ -50,7 +51,7 @@ from .traverse import _indices_for_direction -_CURRENT = "__current__" +_CURRENT = WALK_CURRENT_COL _FROM = "__gfql_ib_from__" _TO = "__gfql_ib_to__" _PATH_ORD = "__gfql_ib_path_ord__" @@ -232,7 +233,24 @@ def one(from_col: str, to_col: str, orient: int) -> DataFrameT: reverse = one(dst, src, 1) if engine == Engine.POLARS: reverse = reverse.select(forward.columns) # type: ignore[operator] - return cast(DataFrameT, df_concat(engine)([forward, reverse], ignore_index=True)) + combined = cast(DataFrameT, df_concat(engine)([forward, reverse], ignore_index=True)) + # A self-loop's two undirected orientations are the SAME binding: drop the per-ordinal twin. + if _EDGE_ORD in combined.columns: + if engine == Engine.POLARS: + combined = cast( # hygiene-ok: explicit-cast -- DataFrameT narrowing, module-wide idiom + DataFrameT, + combined.unique( # type: ignore[operator] + subset=[_FROM, _TO, _EDGE_ORD], keep="first", maintain_order=True + ), + ) + else: + combined = cast( # hygiene-ok: explicit-cast -- DataFrameT narrowing, module-wide idiom + DataFrameT, + combined.drop_duplicates( + subset=[_FROM, _TO, _EDGE_ORD], keep="first", ignore_index=True + ), + ) + return combined if direction == "reverse": return one(dst, src, 0) return one(src, dst, 0) diff --git a/graphistry/compute/gfql/lazy/engine/polars/predicates.py b/graphistry/compute/gfql/lazy/engine/polars/predicates.py index 713686a6d0..ae3c72dfcf 100644 --- a/graphistry/compute/gfql/lazy/engine/polars/predicates.py +++ b/graphistry/compute/gfql/lazy/engine/polars/predicates.py @@ -110,12 +110,7 @@ def _cmp_expr( "DateTimeValue", "TimeValue", "DateValue", ): return None - # Narrow residual: no IEEE NaN mask here (unlike the WHERE/row-pipeline _nan_guard) — on a - # GENUINE polars NaN, `col > x` keeps the row (NaN = largest) where pandas drops it. - # Unreachable on standard ingestion: from_pandas/df_to_engine convert NaN→null (nan_to_null) - # and filter_by_dict runs on INGESTED columns (no in-query float math — that's the WHERE - # path). Only a natively-built polars frame with raw NaN diverges; documented, not guarded, - # to keep the lowering simple. (Mirrors the documented integer 0/0 column-compare residual.) + # Documented residual: only a natively-built polars frame holding a RAW NaN diverges from pandas. if _orders_boolean_column_against_number(op, val, dtype): import polars as pl return pl.lit(False) diff --git a/graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py b/graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py index b6885e4ba6..3d9df1f82e 100644 --- a/graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py +++ b/graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py @@ -64,6 +64,13 @@ COLUMNS_NAN_FREE as _COLUMNS_NAN_FREE, ) from graphistry.compute.gfql.same_path_types import NODE_IDENTITY_COLUMN as _NODE_ID_TOKEN +from graphistry.compute.gfql.identifiers import ( + TRAIL_EDGE_IDENT_COL, + WALK_CURRENT_COL, + WALK_FROM_COL, + WALK_TO_COL, + trail_column_name, +) # Ops needing the NaN guard: polars treats NaN as the LARGEST value (>/>=/== TRUE), but # IEEE/Python/pandas/Cypher compare NaN as FALSE (!= TRUE; Neo4j TCK agrees). Float operands @@ -982,7 +989,7 @@ def names(frame: "PolarsFrameT") -> List[str]: state = state.join( lookup, left_on=alias, right_on=node_id, how="left", ) - state = state.drop("__current__") + state = state.drop(WALK_CURRENT_COL) out_df = ( _lazy_collect(state) if isinstance(state, pl.LazyFrame) @@ -1542,7 +1549,7 @@ def _names(lf: pl.LazyFrame) -> List[str]: seed_ids_lf: Optional[Any] = None # LazyFrame; Any avoids the union-typed seed_nodes.join mismatch start_nodes = g._gfql_start_nodes if start_nodes is not None: - # Bounded WITH->MATCH re-entry (#1273): the carried WITH rows seed the first + # Bounded WITH->MATCH re-entry: the carried WITH rows seed the first # alias. Constrain the first node alias to the carried ids via a semi-join — # the native twin of the pandas wavefront seed. Support only UNIQUE carried # ids: then the semi-join contributes each seed node exactly once, matching the @@ -1572,7 +1579,7 @@ def _names(lf: pl.LazyFrame) -> List[str]: # (wrong cross-product). Decline honestly (the alternating-path seed is # applied at seed_nodes; node-cartesian re-entry stays pandas-only). return None - # MATCH (a), (b), ... disconnected node aliases: native cross-product (#1273). + # MATCH (a), (b), ... disconnected node aliases: native cross-product. return _cartesian_node_bindings_polars(g, ops, node_id) if RowPipelineMixin._gfql_is_shortest_path_scalar_binding_ops(ops): return None # shortestPath scalar contract: BFS/native backends, pandas-only @@ -1627,41 +1634,7 @@ def _names(lf: pl.LazyFrame) -> List[str]: return None sem = EdgeSemantics.from_edge(op) if sem.is_multihop: - # Bounded directed var-length (`-[*1..k]->`, graph-bench q3) is - # supported via iterative pair joins. Bounded UNDIRECTED var-length - # with min_hops == 1 (`-[*1..k]-`, the LDBC IC11/IC6 shape) is now - # also supported via a doubled-pair join with immediate-backtrack - # avoidance (see the execution branch below). UNBOUNDED DIRECTED - # fixed-point (`-[*0..]->` / `-[*]->`, the LDBC IS6 REPLY_OF ancestor - # walk, #1709) runs the same pair join to exhaustion — see - # `_directed_fixed_point_binding_rows_polars` for the parity argument. - # Everything else declines: - # - aliased var-length edges (pandas rejects those outright); - # - undirected var-length with min_hops != 1 (`-[*0..k]-` / - # `-[*2..k]-`): pandas' step_pairs come from the var-length - # `edge_op.execute` hop, whose backward hop-window pruning / - # zero-hop handling changes the edge multiplicity in a way this - # raw-edge reconstruction only reproduces for min_hops == 1 (every - # edge is trivially a length-1 path, so no pruning occurs) — - # fuzz-verified vs the pandas oracle; - # - undirected UNBOUNDED (`-[*]-`): would need both the multiplicity - # reconstruction above and backtrack-aware termination; - # - unbounded WITHOUT to_fixed_point (`min_hops=2` and no max): pandas - # silently truncates at `len(step_pairs) + 1` iterations instead of - # erroring, and its step_pairs row count is not reconstructible here, - # so the truncation depth (hence the answer) is not reproducible. - # - UNBOUNDED with min_hops >= 2 (`-[*2..]->`): same multiplicity hazard as - # the undirected min_hops != 1 case above, and it is Cypher-reachable. - # Pandas' step_pairs come from the var-length `edge_op.execute` hop, which - # returns an EMPTY frame when its `max_reached_hop < min_hops` - # (compute/hop.py) and otherwise drops edges labelled below min_hops. - # `max_reached_hop` is a dedup-by-node BFS eccentricity, NOT a longest-walk - # length, so the raw-edge reconstruction below expands a DIFFERENT edge - # multiset and disagrees SILENTLY — on a 7-node acyclic graph - # `MATCH (a)-[*3..]->(b) RETURN count(*)` gives pandas 0 vs polars 30. - # `RETURN count(*)` lowers to a pure-CALL chain, so `_is_native_multihop` - # in `_chain_traversal_polars` never runs: this gate is the only gate. - # Decline the rest honestly rather than risk silent-wrong multiplicities. + # Served: bounded fwd/rev/undirected windows and the unbounded DIRECTED fixed point. if isinstance(op._name, str): return None _resolved_max = op.max_hops if op.max_hops is not None else op.hops @@ -1680,8 +1653,7 @@ def _names(lf: pl.LazyFrame) -> List[str]: if _resolved_max is None: if not bool(op.to_fixed_point) or op.direction == "undirected": return None - # min_hops 0 and 1 are the fuzz-verified shapes (`-[*]->`, `-[*0..]->`, - # the #1709 IS6 walk); >= 2 is the divergence above. + # Unbounded serves min_hops 0 and 1 only; >= 2 is the divergence above. _resolved_min_unbounded = op.min_hops if op.min_hops is not None else ( op.hops if op.hops is not None else 1 ) @@ -1693,41 +1665,20 @@ def _names(lf: pl.LazyFrame) -> List[str]: ) if _resolved_min != 1: return None - # #1787, same root-cause family as the unbounded shapes #1781 declined: pandas' - # step_pairs come from the var-length `edge_op.execute` hop, whose hop-window - # pruning -- and, when seeded, its per-seed BFS -- changes an edge multiplicity - # this raw-edge rebuild cannot reproduce. Declining is a DELIBERATE divergence - # from master, which served these: parity-or-NIE means a loud error, never a - # different number. Shrink the gate again once the multiplicity is reconstructible. - # WHICH shapes, why each boundary sits where it does, and the counts that prove - # each one are executable rather than prose -- every claim that used to be written - # out here is now a named test in: - # graphistry/tests/compute/gfql/test_varlen_bounded_engine_parity_1787.py - # - # Keyed on an EXPLICIT window, NOT on `sem.is_multihop`: `-[*1..1]-` resolves to - # min == max == 1, is therefore not multihop, and pandas still routes it here. + # Residual decline: pandas' `max_reached_hop` is a dedup-by-node eccentricity, not a + # longest-trail length, so a DIRECTED min_hops window under-reports on the pandas lane. if op.min_hops is not None or op.max_hops is not None: - _vl_max = op.max_hops if op.max_hops is not None else op.hops _vl_min = op.min_hops if op.min_hops is not None else ( op.hops if op.hops is not None else 1 ) - # a seed is anything that starts the segment from less than the whole node - # set: a filtered start alias, or a re-entry / `WITH` seed frame _prev_op = ops[idx - 1] if idx >= 1 else None _seeded_start = start_nodes is not None or ( isinstance(_prev_op, ASTNode) and bool(_prev_op.filter_dict) ) - if _vl_max is not None: - if op.direction == "undirected": - if _vl_max == 1: # the degenerate window `-[*1..1]-` / `-[*1]-` - return None - if _seeded_start or idx > 1: # doubled-pair expansion over-counts - return None - # `max_reached_hop` (compute/hop.py) is a dedup-by-node BFS eccentricity, - # not a longest-walk length, so pandas prunes where this rebuild expands - # a different edge multiset - elif _vl_min >= 3 or (_vl_min >= 2 and _seeded_start): - return None + if op.direction != "undirected" and ( + _vl_min >= 3 or (_vl_min >= 2 and _seeded_start) + ): + return None if op.direction not in ("forward", "reverse", "undirected"): return None if any( @@ -1749,7 +1700,7 @@ def _names(lf: pl.LazyFrame) -> List[str]: try: # Build the WHOLE binding table as ONE deferred pl.LazyFrame and collect - # ONCE on the active target (#1709 laziness): under engine='polars-gpu' the + # ONCE on the active target: under engine='polars-gpu' the # entire join chain + property attach runs on cudf_polars in a single GPU # collect (~4-5× vs CPU on the join phase — de-risk probe 2026-07-06); # under 'polars' it collects on CPU (parity-identical). NO-CHEATING: a @@ -1780,6 +1731,11 @@ def _names(lf: pl.LazyFrame) -> List[str]: _endpoint_casts.append(pl.col(_endpoint).cast(_node_dtype)) if _endpoint_casts: edges_lf = edges_lf.with_columns(_endpoint_casts) + # openCypher trail semantics: stable per-edge identity for the + # at-most-once-per-path relationship constraint (pandas twin: + # _gfql_connected_bindings_state's __gfql_edge_ident__). + edges_lf = edges_lf.with_row_index(TRAIL_EDGE_IDENT_COL) + trail_cols_pl: List[str] = [] first_op = ops[0] if not isinstance(first_op, ASTNode): return None @@ -1791,12 +1747,12 @@ def _names(lf: pl.LazyFrame) -> List[str]: # `filter_by_dict_polars` is frame-polymorphic at runtime but declares the eager # type, so pin the path bag lazy here instead of leaving every downstream lazy # op to fight an eager inference. - state: pl.LazyFrame = seed_nodes.select(pl.col(node_id).alias("__current__")) # type: ignore[assignment] + state: pl.LazyFrame = seed_nodes.select(pl.col(node_id).alias(WALK_CURRENT_COL)) # type: ignore[assignment] alias_frames: Dict[str, pl.LazyFrame] = {} node_aliases: List[str] = [] first_alias = first_op._name if isinstance(first_alias, str): - state = state.with_columns(pl.col("__current__").alias(first_alias)) + state = state.with_columns(pl.col(WALK_CURRENT_COL).alias(first_alias)) alias_frames[first_alias] = seed_nodes node_aliases.append(first_alias) @@ -1811,20 +1767,27 @@ def _names(lf: pl.LazyFrame) -> List[str]: payload_renames = { col: f"{edge_alias}.{col}" for col in _names(edges_f) - if col not in (src, dst) + if col not in (src, dst, TRAIL_EDGE_IDENT_COL) } else: # Unaliased edge payload is unaddressable downstream; carrying it # unprefixed (as pandas does) only risks column collisions. - edges_f = edges_f.select([src, dst]) + edges_f = edges_f.select([src, dst, TRAIL_EDGE_IDENT_COL]) payload_renames = {} if sem.is_undirected: - fwd = edges_f.rename({src: "__from__", dst: "__to__"}) - rev = edges_f.rename({dst: "__from__", src: "__to__"}) + fwd = edges_f.rename({src: WALK_FROM_COL, dst: WALK_TO_COL}) + rev = edges_f.rename({dst: WALK_FROM_COL, src: WALK_TO_COL}) oriented = pl.concat([fwd, rev.select(_names(fwd))], how="vertical") + # A self-loop's two undirected orientations are the SAME binding: + # dedupe the flip twin. + oriented = oriented.unique( + subset=[WALK_FROM_COL, WALK_TO_COL, TRAIL_EDGE_IDENT_COL], + keep="first", + maintain_order=True, + ) else: join_col, result_col = (dst, src) if edge_op.direction == "reverse" else (src, dst) - oriented = edges_f.rename({join_col: "__from__", result_col: "__to__"}) + oriented = edges_f.rename({join_col: WALK_FROM_COL, result_col: WALK_TO_COL}) if payload_renames: oriented = oriented.rename(payload_renames) @@ -1839,23 +1802,18 @@ def _names(lf: pl.LazyFrame) -> List[str]: # this turn an all-edges scan into a small-domain edge semi-join. oriented = oriented.join( next_node_ids, - left_on="__to__", + left_on=WALK_TO_COL, right_on=node_id, how="semi", ) # Column collision between edge payload and accumulated state → decline # (pandas resolves via merge suffixes; unreferenced-by-queries either way). - overlap = (set(_names(oriented)) - {"__from__"}) & set(_names(state)) + overlap = (set(_names(oriented)) - {WALK_FROM_COL}) & set(_names(state)) if overlap: return None if sem.is_multihop: - # Bounded directed var-length: iterative pair joins, one row per - # distinct edge sequence (Cypher path multiplicity — pairs NOT - # deduped, so parallel edges multiply per hop, matching pandas - # `_gfql_multihop_binding_rows`). Zero-hop rows (min 0) keep the - # seed row (endpoint == start), also matching pandas. - # Same defaults as the pandas builder: bare hops=k means exactly-k. + # Same defaults as the pandas builder: a bare hops=k means exactly k. min_hops_value = edge_op.min_hops if edge_op.min_hops is not None else ( edge_op.hops if edge_op.hops is not None else 1 ) @@ -1867,79 +1825,71 @@ def _names(lf: pl.LazyFrame) -> List[str]: # above to to_fixed_point=True and a directed edge. Termination is # data-dependent, so unlike the bounded branch this one cannot stay # fully lazy — it collects one frontier per hop. - state = _directed_fixed_point_binding_rows_polars( + state, _fp_trail_cols = _directed_fixed_point_binding_rows_polars( state, - oriented.select(["__from__", "__to__"]), + oriented.select([WALK_FROM_COL, WALK_TO_COL]), state_cols, min_hops=min_hops, ) elif sem.is_undirected: - # Bounded UNDIRECTED var-length, min_hops == 1 (gated above): the - # LDBC IC11/IC6 `-[*1..k]-` shape. Mirror the pandas oracle - # (`_gfql_multihop_binding_rows`, avoid_immediate_backtrack=True) - # EXACTLY, including its edge multiplicity: pandas' `step_pairs` - # come from the undirected var-length hop + `orient_edges`, which - # emits each NON-loop edge as (u,v)x2 AND (v,u)x2, and each - # SELF-loop as (u,u)x2 (loops are not double-counted). Reconstruct - # that here: `exec_rows` = both directions of non-loops + one row - # per self-loop; the final `pairs` doubles `exec_rows` - # (fuzz-verified vs pandas over random graphs incl. self-loops, - # parallel + antiparallel edges). A `__prev__` column (seeded null) - # carries the just-left node so each hop can drop immediate - # backtracks (`__to__ == __prev__`), matching pandas' Kleene mask - # (null prev -> kept). + # One row per orientation (a self-loop just one); a relationship binds once per + # path, so a same-edge backtrack dies while a PARALLEL-edge return trip is legal. max_hops = int(max_hops_value) normal = edges_f.filter(pl.col(src) != pl.col(dst)) loops = edges_f.filter(pl.col(src) == pl.col(dst)) - fwd = normal.select([pl.col(src).alias("__from__"), pl.col(dst).alias("__to__")]) - rev = normal.select([pl.col(dst).alias("__from__"), pl.col(src).alias("__to__")]) - loop = loops.select([pl.col(src).alias("__from__"), pl.col(dst).alias("__to__")]) - exec_rows = pl.concat([fwd, rev, loop], how="vertical") - pairs = pl.concat([exec_rows, exec_rows], how="vertical") - prev_col = "__prev__" + ident = pl.col(TRAIL_EDGE_IDENT_COL) + fwd = normal.select([pl.col(src).alias(WALK_FROM_COL), pl.col(dst).alias(WALK_TO_COL), ident]) + rev = normal.select([pl.col(dst).alias(WALK_FROM_COL), pl.col(src).alias(WALK_TO_COL), ident]) + loop = loops.select([pl.col(src).alias(WALK_FROM_COL), pl.col(dst).alias(WALK_TO_COL), ident]) + pairs = pl.concat([fwd, rev, loop], how="vertical") reachable = [state.select(state_cols)] if min_hops == 0 else [] - # Seed the backtrack marker with the SAME dtype as __current__ so a - # non-Int64 node id (e.g. string ids) compares/concats cleanly. - current = state.with_columns( - pl.lit(None).cast(state.collect_schema()["__current__"]).alias(prev_col) - ) + current = state + _und_trail_cols: List[str] = [] for _hop in range(1, max_hops + 1): joined = current.join( - pairs, left_on="__current__", right_on="__from__", how="inner" - ) - joined = joined.filter( - pl.col(prev_col).is_null() | (pl.col("__to__") != pl.col(prev_col)) + pairs, left_on=WALK_CURRENT_COL, right_on=WALK_FROM_COL, how="inner" ) - # new prev = the node we are leaving (old __current__); new - # __current__ = __to__. Set prev BEFORE dropping __current__. - joined = ( - joined.with_columns(pl.col("__current__").alias(prev_col)) - .drop("__current__") - .rename({"__to__": "__current__"}) - ) - current = joined.select(state_cols + [prev_col]) + for _used in trail_cols_pl + _und_trail_cols: + joined = joined.filter( + (pl.col(TRAIL_EDGE_IDENT_COL) != pl.col(_used)) | pl.col(_used).is_null() + ) + _hop_trail = trail_column_name(len(trail_cols_pl) + len(_und_trail_cols)) + joined = joined.rename({TRAIL_EDGE_IDENT_COL: _hop_trail}) + _und_trail_cols.append(_hop_trail) + joined = joined.drop(WALK_CURRENT_COL).rename({WALK_TO_COL: WALK_CURRENT_COL}) + current = joined.select(state_cols + _und_trail_cols) if _hop >= min_hops: - reachable.append(current.select(state_cols)) - state = pl.concat(reachable, how="vertical") if reachable else state.limit(0) + reachable.append(current) + state = pl.concat(reachable, how="diagonal") if reachable else state.limit(0) + trail_cols_pl = trail_cols_pl + _und_trail_cols else: - # Bounded directed var-length (`-[*1..k]->`, graph-bench q3). - state = _directed_varlen_reachable_polars( + # Bounded directed var-length (`-[*1..k]->`), trail-tracked. + state, _seg_trail_cols = _directed_varlen_reachable_polars( state, - oriented.select(["__from__", "__to__"]), + oriented.select([WALK_FROM_COL, WALK_TO_COL, TRAIL_EDGE_IDENT_COL]), state_cols, min_hops=min_hops, max_hops=int(max_hops_value), + trail_cols_in=trail_cols_pl, ) + trail_cols_pl = trail_cols_pl + _seg_trail_cols else: state = ( - state.join(oriented, left_on="__current__", right_on="__from__", how="inner") - .drop("__current__") - .rename({"__to__": "__current__"}) + state.join(oriented, left_on=WALK_CURRENT_COL, right_on=WALK_FROM_COL, how="inner") +.drop(WALK_CURRENT_COL) +.rename({WALK_TO_COL: WALK_CURRENT_COL}) ) + for _used in trail_cols_pl: + state = state.filter( + (pl.col(TRAIL_EDGE_IDENT_COL) != pl.col(_used)) | pl.col(_used).is_null() + ) + _new_trail = trail_column_name(len(trail_cols_pl)) + state = state.rename({TRAIL_EDGE_IDENT_COL: _new_trail}) + trail_cols_pl = trail_cols_pl + [_new_trail] state = state.join( next_node_ids, - left_on="__current__", + left_on=WALK_CURRENT_COL, right_on=node_id, how="semi", ) @@ -1964,18 +1914,22 @@ def _names(lf: pl.LazyFrame) -> List[str]: return None _base_dup = bool( nodes.lazy() - .select(pl.col(node_id).is_duplicated().any()) - .collect() - .item() +.select(pl.col(node_id).is_duplicated().any()) +.collect() +.item() ) if _base_dup: return None next_alias = next_op._name if isinstance(next_alias, str): - state = state.with_columns(pl.col("__current__").alias(next_alias)) + state = state.with_columns(pl.col(WALK_CURRENT_COL).alias(next_alias)) alias_frames[next_alias] = next_nodes node_aliases.append(next_alias) + if trail_cols_pl: + _present = [c for c in trail_cols_pl if c in _names(state)] + if _present: + state = state.drop(_present) # The finisher's frame type is a constrained TypeVar so `state.join(lookup)` # type-checks. The GENERIC builder above mixes eager and lazy frames across # its (pre-existing) branches, so inference cannot pick one here; the diff --git a/graphistry/compute/gfql/lazy/engine/polars/varlen_rows.py b/graphistry/compute/gfql/lazy/engine/polars/varlen_rows.py index 9453c5dbd8..c67f5582b0 100644 --- a/graphistry/compute/gfql/lazy/engine/polars/varlen_rows.py +++ b/graphistry/compute/gfql/lazy/engine/polars/varlen_rows.py @@ -12,15 +12,29 @@ Only ``min_hops <= 1`` reaches the unbounded arm — the gate in ``row_pipeline.py`` declines ``-[*2..]->`` because pandas' ``step_pairs`` prune by min_hops against a dedup-by-node eccentricity that the raw-edge reconstruction here cannot reproduce. + +openCypher TRAIL semantics: when ``pairs`` carries the stable +``__gfql_edge_ident__`` column, each expansion hop filters the new edge against +every edge already bound on the path (this segment's and prior elements', via +``trail_cols_in``) and records it as a ``__gfql_trail_*`` column — mirroring the +pandas ``_gfql_multihop_binding_rows`` twin exactly. """ from __future__ import annotations -from typing import List, TYPE_CHECKING +from typing import List, Optional, Tuple, TYPE_CHECKING +from graphistry.compute.gfql.identifiers import ( + TRAIL_EDGE_IDENT_COL, + WALK_CURRENT_COL, + WALK_FROM_COL, + WALK_TO_COL, + trail_column_name, +) if TYPE_CHECKING: import polars as pl + def _directed_varlen_reachable_polars( state: "pl.LazyFrame", pairs: "pl.LazyFrame", @@ -28,32 +42,48 @@ def _directed_varlen_reachable_polars( *, min_hops: int, max_hops: int, -) -> "pl.LazyFrame": + trail_cols_in: Optional[List[str]] = None, +) -> Tuple["pl.LazyFrame", List[str]]: """Bounded DIRECTED variable-length expansion of a bindings path bag. - One row per distinct edge SEQUENCE: ``pairs`` is NOT deduped, so parallel edges - multiply per hop, matching pandas' ``_gfql_multihop_binding_rows`` merge. Zero-hop - rows (``min_hops == 0``) keep the seed row (endpoint == start) and come first, then - hop 1, 2, ... — the same ``reachable`` concat order pandas builds. + One row per distinct edge SEQUENCE under trail semantics: ``pairs`` is not + deduped (parallel edges multiply per hop), and when it carries + ``__gfql_edge_ident__`` a relationship binds at most once per path. Zero-hop + rows (``min_hops == 0``) keep the seed row (endpoint == start) and come first, + then hop 1, 2, ... — the same ``reachable`` concat order pandas builds. Stays fully lazy: all ``max_hops`` iterations are built without an eager ``.height`` early-break, because an empty intermediate lazily joins to empty and yields the identical result (pandas' break is an optimization, not semantics). + + Returns ``(frame, new_trail_cols)``; hop-k rows carry k trail columns, rows + from shallower hops null-fill the deeper ones (diagonal concat). """ import polars as pl - reachable: List["pl.LazyFrame"] = [state] if min_hops == 0 else [] + trail = TRAIL_EDGE_IDENT_COL in pairs.collect_schema().names() + outer_trail = list(trail_cols_in or []) + segment_trail_cols: List[str] = [] + + reachable: List["pl.LazyFrame"] = [state.select(state_cols)] if min_hops == 0 else [] current = state for hop in range(1, max_hops + 1): - current = ( - current.join(pairs, left_on="__current__", right_on="__from__", how="inner") - .drop("__current__") - .rename({"__to__": "__current__"}) - .select(state_cols) - ) + current = current.join(pairs, left_on=WALK_CURRENT_COL, right_on=WALK_FROM_COL, how="inner") + if trail: + for used_col in outer_trail + segment_trail_cols: + current = current.filter( + (pl.col(TRAIL_EDGE_IDENT_COL) != pl.col(used_col)) | pl.col(used_col).is_null() + ) + hop_trail_col = trail_column_name(len(outer_trail) + len(segment_trail_cols)) + current = current.rename({TRAIL_EDGE_IDENT_COL: hop_trail_col}) + segment_trail_cols.append(hop_trail_col) + current = current.drop(WALK_CURRENT_COL).rename({WALK_TO_COL: WALK_CURRENT_COL}) + current = current.select(state_cols + segment_trail_cols) if hop >= min_hops: reachable.append(current) - return pl.concat(reachable, how="vertical") if reachable else state.limit(0) + if not reachable: + return state.limit(0), segment_trail_cols + return pl.concat(reachable, how="diagonal"), segment_trail_cols def _directed_fixed_point_binding_rows_polars( @@ -62,37 +92,25 @@ def _directed_fixed_point_binding_rows_polars( state_cols: List[str], *, min_hops: int, -) -> "pl.LazyFrame": +) -> Tuple["pl.LazyFrame", List[str]]: """Unbounded DIRECTED variable-length binding rows (``-[*0..]->`` / ``-[*]->``), #1709. The native twin of the ``max_hops is None and to_fixed_point`` arm of pandas' ``RowPipelineMixin._gfql_multihop_binding_rows``. One row per distinct edge - SEQUENCE (Cypher path multiplicity): ``pairs`` is never deduped, so parallel + SEQUENCE (Cypher trail multiplicity): ``pairs`` is never deduped, so parallel edges multiply per hop exactly as the pandas merge does. ``min_hops == 0`` contributes the zero-hop rows (endpoint == start) first, then hop 1, 2, ... — the same ``reachable`` concat order pandas builds. Pandas discovers the traversal depth by expanding the PATH frontier until it is - empty, which materializes every partial path at every hop. This lowering splits - that into (a) a cheap dedup-by-node frontier walk that computes the exhaustion - depth ``D``, then (b) the SAME lazy bounded pair-join loop the ``-[*1..k]->`` arm - uses, with ``max_hops = D``. Step (a) is exact, not an approximation: the path - frontier at hop ``h`` is non-empty iff a walk of length ``h`` leaves some seed, - and deduping by endpoint node changes no walk's EXISTENCE — only its - multiplicity, which step (b) then reproduces in full. So the emitted rows are - identical to pandas' while the exponential path blow-up happens once, lazily. - - Non-termination: a cycle reachable from a seed makes the walk infinite, and - pandas raises ``E108`` ("require terminating variable-length segments"). We raise - the same error via the same helper, and we detect it strictly, by pigeonhole over - the REACHABLE set: a walk of ``h`` edges visits ``h + 1`` nodes, all of them seeds - or frontier members, so once ``h + 1`` exceeds the number of nodes seen the walk has - repeated a node — a reachable cycle, exactly the condition under which pandas' own - cap (``max(len(step_pairs), 1) + 1``) also fails to exhaust. Conversely an acyclic - reachable subgraph empties the frontier first, so both engines exhaust. Same outcome - on both sides of the branch, without pandas' cost of expanding paths into the cycle - before giving up — and the bound is the reachable node count, so an unreachable - remainder of the graph costs nothing (see the probe's own note). + empty. This lowering splits that into (a) a cheap dedup-by-node frontier walk + that computes the exhaustion depth ``D``, then (b) the SAME lazy bounded + pair-join loop the ``-[*1..k]->`` arm uses, with ``max_hops = D``. + + Cyclic reachability: the node-frontier probe cannot see trail exhaustion (a + cycle keeps the node frontier alive even though trails are finite), so a + reachable cycle still raises the terminating-segments error here — the pandas + twin now serves those via trail tracking; this decline is the #1903 residual. """ import polars as pl from graphistry.compute.gfql.lazy import collect as _lazy_collect @@ -100,6 +118,7 @@ def _directed_fixed_point_binding_rows_polars( pairs_df = _lazy_collect(pairs) pairs_lf = pairs_df.lazy() + pairs_step = pairs_lf.select([WALK_FROM_COL, WALK_TO_COL]) # (a) depth probe: dedup-by-node frontier, so each hop costs O(N) not O(paths). # @@ -107,14 +126,9 @@ def _directed_fixed_point_binding_rows_polars( # by the graph's node count. A walk of length ``hop`` visits ``hop + 1`` nodes, every # one of them a seed or a frontier member, i.e. all inside ``seen``; so the moment # ``hop + 1`` exceeds ``seen.height`` some node has repeated, and a repeat on a walk IS - # a reachable cycle. That is the same pigeonhole argument as a global-N bound but over - # the only set that matters, and it is still exact in both directions: an acyclic - # reachable subgraph empties the frontier before the bound, and a reachable cycle keeps - # it non-empty past it. Bounding by the global count instead makes a two-node cycle - # reachable from one seed cost O(graph) eager collects before raising — measured linear - # in the GLOBAL node count, which at LDBC SF1 scale is minutes of spinning to reach a - # validation error. - frontier = _lazy_collect(state.select(pl.col("__current__")).unique()) + # a reachable cycle. An acyclic reachable subgraph empties the frontier before the + # bound; a reachable cycle keeps it non-empty past it. + frontier = _lazy_collect(state.select(pl.col(WALK_CURRENT_COL)).unique()) seen = frontier frontier_lf = frontier.lazy() depth = 0 @@ -124,8 +138,8 @@ def _directed_fixed_point_binding_rows_polars( while not exhausted: hop += 1 frontier = _lazy_collect( - frontier_lf.join(pairs_lf, left_on="__current__", right_on="__from__", how="inner") - .select(pl.col("__to__").alias("__current__")) + frontier_lf.join(pairs_step, left_on=WALK_CURRENT_COL, right_on=WALK_FROM_COL, how="inner") + .select(pl.col(WALK_TO_COL).alias(WALK_CURRENT_COL)) .unique() ) if frontier.height == 0: diff --git a/graphistry/compute/gfql/row/pipeline.py b/graphistry/compute/gfql/row/pipeline.py index b4af56bc2e..f17a1fee70 100644 --- a/graphistry/compute/gfql/row/pipeline.py +++ b/graphistry/compute/gfql/row/pipeline.py @@ -71,6 +71,16 @@ is_entity_text_scalar, ) from graphistry.compute.gfql.same_path_types import NODE_IDENTITY_COLUMN +from graphistry.compute.gfql.identifiers import ( + SHORTEST_PATH_HOPS_COLUMN_PREFIX, + TRAIL_EDGE_IDENT_COL, + WALK_CURRENT_COL, + WALK_FROM_COL, + WALK_PREV_COL, + WALK_TO_COL, + is_shortest_path_hops_column, + trail_column_name, +) from graphistry.compute.gfql.cache_registry import register_process_singleton from graphistry.compute.gfql.series_str_compat import is_non_textual_scalar_dtype, series_sequence_len, series_str_match from graphistry.compute.gfql.row.ordering import ( @@ -403,7 +413,9 @@ def _gfql_nullable_structural_equal(left: Any, right: Any) -> Optional[bool]: return bool(equals) def _gfql_eval_comparison_op( - self, table_df: Any, left: Any, right: Any, op: str + self, table_df: Any, left: Any, right: Any, op: str, + left_null_mask_override: Optional[Any] = None, # hygiene-ok: explicit-any -- scalar-or-Series mask, evaluator-wide idiom + right_null_mask_override: Optional[Any] = None, # hygiene-ok: explicit-any -- scalar-or-Series mask, evaluator-wide idiom ) -> Optional[Any]: cmp_fn = GFQL_COMPARISON_BINARY_OPS.get(op) if cmp_fn is None: @@ -468,8 +480,17 @@ def _is_numeric_nonbool(value: Any) -> bool: # hygiene-ok: explicit-any -- hete self._gfql_broadcast_scalar(table_df, False).astype(bool), pd.NA ) - left_null_mask = self._gfql_null_mask(table_df, left) - right_null_mask = self._gfql_null_mask(table_df, right) + # A computed NaN (x % 0.0) is a VALUE to IEEE-compare; only an INPUT null nulls the result. + left_null_mask = ( + left_null_mask_override + if left_null_mask_override is not None + else self._gfql_null_mask(table_df, left) + ) + right_null_mask = ( + right_null_mask_override + if right_null_mask_override is not None + else self._gfql_null_mask(table_df, right) + ) if isinstance(left, float) and math.isnan(left): left_null_mask = self._gfql_broadcast_scalar(table_df, False).astype(bool) if isinstance(right, float) and math.isnan(right): @@ -521,7 +542,7 @@ def _gfql_raise_on_integer_zero_divisor(right: Any, op: str) -> None: # hygiene f"Cypher integer '{op}' by zero", field="expression", value=op, - suggestion="Guard the divisor (e.g. CASE WHEN d = 0 THEN null ELSE x / d END) or use a float divisor for IEEE semantics.", + suggestion="Guard the divisor (engine='pandas': CASE WHEN d = 0 THEN null ELSE x / d END; the CASE guard is not yet native on polars) or use a float divisor for IEEE semantics.", ) @staticmethod @@ -954,11 +975,7 @@ def _gfql_eval_expr_ast(self, table_df: Any, node: Any) -> Tuple[bool, Any]: if not any(hasattr(val, "astype") for val in item_values): return True, list(item_values) - # cuDF groupby-collect (.agg(list)) gives NO within-group row-order guarantee, so the - # melt+sort+groupby path below permutes list ELEMENTS vs construction order on cuDF - # (issue #1663 finding 1; pandas groupby(sort=False) is stable so it's correct there). - # Build the list column directly column-wise on cuDF — order-deterministic (same logic - # as the except-fallback below, which list-TYPED elements already use). + # cuDF groupby-collect has no within-group order guarantee; build the list column-wise. if resolve_engine(EngineAbstract.AUTO, table_df) == Engine.CUDF: _rc = len(table_df) _items: List[List[Any]] = [] @@ -1170,7 +1187,32 @@ def _gfql_eval_expr_ast(self, table_df: Any, node: Any) -> Tuple[bool, Any]: return False, None return True, bool_out - cmp_out = RowPipelineMixin._gfql_eval_comparison_op(self, table_df, left, right, op) + def _arith_input_null_mask(operand_node: Any) -> Optional[Any]: # hygiene-ok: explicit-any -- AST node + scalar-or-Series mask + """Input-null mask of an ARITHMETIC subtree; None -> caller falls back + to isna-of-result. Separates a genuine null input from a computed NaN.""" + if isinstance(operand_node, BinaryOp) and str(operand_node.op).lower() in {"+", "-", "*", "/", "%"}: + left_mask = _arith_input_null_mask(operand_node.left) + right_mask = _arith_input_null_mask(operand_node.right) + if left_mask is None or right_mask is None: + return None + return left_mask | right_mask + if isinstance(operand_node, UnaryOp) and str(operand_node.op) in {"+", "-"}: + return _arith_input_null_mask(operand_node.operand) + ok_operand, operand_value = self._gfql_eval_expr_ast(table_df, operand_node) + if not ok_operand: + return None + return self._gfql_null_mask(table_df, operand_value) + + def _is_arith_node(operand_node: Any) -> bool: # hygiene-ok: explicit-any -- AST node union + return isinstance(operand_node, BinaryOp) and str(operand_node.op).lower() in {"+", "-", "*", "/", "%"} + + left_mask_override = _arith_input_null_mask(node.left) if _is_arith_node(node.left) else None + right_mask_override = _arith_input_null_mask(node.right) if _is_arith_node(node.right) else None + cmp_out = RowPipelineMixin._gfql_eval_comparison_op( + self, table_df, left, right, op, + left_null_mask_override=left_mask_override, + right_null_mask_override=right_mask_override, + ) if cmp_out is not None: return True, cmp_out @@ -1549,13 +1591,7 @@ def _gfql_eval_expr_ast(self, table_df: Any, node: Any) -> Tuple[bool, Any]: return True, float(math.ceil(inner) if use_ceil else math.floor(inner)) if fn == "round" and len(values) in {1, 2}: - # neo4j tie-breaking (standards-vetted, #1673): precision 0 (or 1-arg) - # rounds ties toward +inf (round(-1.5) = -1.0); precision > 0 rounds ties - # away from zero (HALF_UP: round(-1.55, 1) = -1.6). numpy/pandas .round is - # half-to-even (round(2.5) -> 2.0) — a wrong answer vs the neo4j spec. - # Uses a floor+frac kernel, NOT floor(x+0.5): the +0.5 addition itself - # rounds up when x sits 1 ulp below a tie (JDK-6430675 class), e.g. - # round(0.49999999999999994) must be 0.0, round(0.0499…96, 1) → 0.0. + # floor+frac, NOT floor(x+0.5): the addition itself rounds up 1 ulp below a tie. inner = values[0] ndigits = int(values[1]) if len(values) == 2 else 0 if ndigits < 0: @@ -2524,9 +2560,7 @@ def _gfql_eval_string_predicate_expr( try: out = apply_string_predicate_series(left_txt, needle, op_name) except NotImplementedError: - # Honest engine decline (e.g. cuDF inline-flag/lookaround limits) — - # the blanket remap below destroyed the NIE class AND blamed the op - # name for what is a pattern/engine limit (#1675 wave-1). + # An engine decline is a pattern/engine limit, not a bad op name: keep its class. raise except re.error as exc: raise ValueError(f"invalid regex pattern in {expr!r}: {exc}") from exc @@ -2881,9 +2915,7 @@ def _gfql_resolve_token(self, table_df: Any, token: str) -> Any: if node_id is not None and node_id in table_df.columns: return table_df[node_id] return self._gfql_broadcast_scalar(table_df, pd.NA) - # Bare alias name on a bindings-row table: resolve to the alias's - # identity column (alias.{node_id_col}). This lets expressions like - # count(post) work when the table has post.id, post.name, etc. (#880) + # A bare alias on a bindings-row table resolves to its identity column, alias.{node_id}. if "." not in txt and RowPipelineMixin._gfql_has_bindings_alias_prefix(table_df, txt): edge_aliases = self._gfql_rows_edge_aliases if edge_aliases is not None and txt in edge_aliases: @@ -3532,6 +3564,29 @@ def _gfql_disambiguate_has_edge_destination_nodes( return candidate_nodes return candidate_nodes[candidate_nodes[label_col].fillna(False).astype(bool)].copy() + @staticmethod + def _gfql_drop_reused_relationship_rows(frame: DataFrameT, trail_cols: Sequence[str]) -> DataFrameT: + """Keep only rows whose newly bound relationship is not already on the path. + + One combined mask and ONE slice, so the frame is copied once per hop rather + than once per already-bound relationship. + """ + if not trail_cols or len(frame) == 0: + return frame + ident = frame[TRAIL_EDGE_IDENT_COL] + keep = None + for used_col in trail_cols: + used = frame[used_col] + unused = ident.ne(used) | used.isna() + keep = unused if keep is None else (keep & unused) + return frame if keep is None else frame[keep] + + @staticmethod + def _gfql_relabel_columns(frame: DataFrameT, mapping: Mapping[str, str]) -> DataFrameT: + """Metadata-only column relabel of a frame the walk owns -- never copies blocks.""" + frame.columns = [mapping.get(col, col) for col in frame.columns] + return frame + def _gfql_multihop_binding_rows( self, state_df: Any, @@ -3542,13 +3597,18 @@ def _gfql_multihop_binding_rows( to_fixed_point: bool, avoid_immediate_backtrack: bool = False, hop_column: Optional[str] = None, - ) -> Any: + trail_cols: Optional[List[str]] = None, + ) -> Tuple[Any, List[str]]: reachable: List[Any] = [] current = state_df.copy() - prev_col = "__gfql_prev__" - shortest_path_mode = bool( - hop_column is not None and str(hop_column).startswith("__cypher_shortest_path_hops__") + prev_col = WALK_PREV_COL + shortest_path_mode = is_shortest_path_hops_column(hop_column) + # A relationship binds at most once per path; shortestPath is exempt (BFS never reuses one). + trail_tracking = ( + not shortest_path_mode and TRAIL_EDGE_IDENT_COL in getattr(step_pairs, "columns", []) ) + outer_trail_cols = list(trail_cols or []) + segment_trail_cols: List[str] = [] def _state_key_cols(frame: Any) -> List[str]: excluded = {prev_col} @@ -3561,7 +3621,8 @@ def _state_key_cols(frame: Any) -> List[str]: current = current.assign(**{prev_col: None}) key_cols = _state_key_cols(current) if min_hops == 0: - zero_hop = current.drop(columns=[prev_col], errors="ignore").copy() + # drop() already returns a walk-owned frame; assigning below never aliases the input. + zero_hop = current.drop(columns=[prev_col], errors="ignore") if hop_column is not None: zero_hop[hop_column] = 0 reachable.append(zero_hop) @@ -3570,22 +3631,37 @@ def _state_key_cols(frame: Any) -> List[str]: max_iters = max_hops if max_hops is not None else max(len(step_pairs), 1) + 1 exhausted = False for hop in range(1, max_iters + 1): - current = current.merge(step_pairs, left_on="__current__", right_on="__from__", how="inner") + current = current.merge(step_pairs, left_on=WALK_CURRENT_COL, right_on=WALK_FROM_COL, how="inner") if len(current) == 0: exhausted = True break + if trail_tracking: + current = RowPipelineMixin._gfql_drop_reused_relationship_rows( + current, outer_trail_cols + segment_trail_cols + ) + if len(current) == 0: + exhausted = True + break + hop_trail_col = trail_column_name(len(outer_trail_cols) + len(segment_trail_cols)) + # The merge above made this hop's frame the walk's own: relabel in place. + current = RowPipelineMixin._gfql_relabel_columns(current, {TRAIL_EDGE_IDENT_COL: hop_trail_col}) + segment_trail_cols.append(hop_trail_col) if avoid_immediate_backtrack: prev_missing = current[prev_col].isna() - backtrack_mask = prev_missing | current["__to__"].ne(current[prev_col]).fillna(False) + backtrack_mask = prev_missing | current[WALK_TO_COL].ne(current[prev_col]).fillna(False) current = current[backtrack_mask] if len(current) == 0: exhausted = True break - current = current.drop(columns=["__current__", prev_col]).rename( - columns={"__from__": prev_col, "__to__": "__current__"} + current = RowPipelineMixin._gfql_relabel_columns( + current.drop(columns=[WALK_CURRENT_COL, prev_col]), + {WALK_FROM_COL: prev_col, WALK_TO_COL: WALK_CURRENT_COL}, ) else: - current = current.drop(columns=["__current__", "__from__"]).rename(columns={"__to__": "__current__"}) + current = RowPipelineMixin._gfql_relabel_columns( + current.drop(columns=[WALK_CURRENT_COL, WALK_FROM_COL]), + {WALK_TO_COL: WALK_CURRENT_COL}, + ) if shortest_path_mode: if len(current) > 0: current = current.drop_duplicates(subset=key_cols, keep="first") @@ -3611,7 +3687,7 @@ def _state_key_cols(frame: Any) -> List[str]: if combined is not None: seen_states = combined if hop >= min_hops: - reached = current.drop(columns=[prev_col], errors="ignore").copy() + reached = current.drop(columns=[prev_col], errors="ignore") if hop_column is not None: reached[hop_column] = hop reachable.append(reached) @@ -3621,12 +3697,23 @@ def _state_key_cols(frame: Any) -> List[str]: self._gfql_bindings_error( "Cypher multi-alias row bindings currently require terminating variable-length segments" ) + if segment_trail_cols and reachable and reachable[0].__class__.__module__.startswith("cudf"): + # cuDF aligns concat schemas strictly, so pad the shallower hops' absent trail + # columns there; pandas pd.concat aligns and NaN-fills mismatched schemas natively. + reachable = [ + frame.assign(**{ + col: float("nan") + for col in segment_trail_cols + if col not in frame.columns + }) + for frame in reachable + ] merged = concat_frames(reachable) if merged is None: - return state_df.iloc[0:0] + return state_df.iloc[0:0], segment_trail_cols if shortest_path_mode and len(merged) > 0: merged = merged.drop_duplicates(keep="first") - return merged + return merged, segment_trail_cols def _gfql_apply_alias_prefilter( self, @@ -3798,19 +3885,27 @@ def _gfql_connected_bindings_state( first_nodes = self._gfql_apply_alias_prefilter( first_nodes, first_alias, alias_prefilters ) - state_df = first_nodes[[node_id_col]].copy().rename(columns={node_id_col: "__current__"}) + state_df = first_nodes[[node_id_col]].copy().rename(columns={node_id_col: WALK_CURRENT_COL}) alias_frames: Dict[str, DataFrameT] = {} if isinstance(first_alias, str): - state_df[first_alias] = state_df["__current__"] + state_df[first_alias] = state_df[WALK_CURRENT_COL] alias_frames[first_alias] = first_nodes + # Positional, not index-based: per-hop frames reset their index so it cannot identify an edge. + trail_cols: List[str] = [] + base_edges_frame = base_graph._edges + if base_edges_frame is not None and TRAIL_EDGE_IDENT_COL not in base_edges_frame.columns: + base_graph = base_graph.edges( + base_edges_frame.assign(**{TRAIL_EDGE_IDENT_COL: range(len(base_edges_frame))}) + ) + for edge_idx in range(1, len(ops), 2): edge_op = ops[edge_idx] if not isinstance(edge_op, ASTEdge): self._gfql_bindings_error( "Cypher multi-alias row bindings currently require edge steps in odd positions" ) - current_nodes = base_nodes[base_nodes[node_id_col].isin(state_df["__current__"])].copy() + current_nodes = base_nodes[base_nodes[node_id_col].isin(state_df[WALK_CURRENT_COL])].copy() if len(current_nodes) == 0: return state_df.iloc[0:0], alias_frames edge_result = edge_op.execute( @@ -3844,13 +3939,17 @@ def _gfql_connected_bindings_state( rename_map = { col: f"{edge_alias}.{col}" for col in edges_df_step.columns - if col not in {src_col, dst_col} + if col not in {src_col, dst_col, TRAIL_EDGE_IDENT_COL} } + hop_column = getattr(edge_op, "label_node_hops", None) + shortest_path_mode = is_shortest_path_hops_column(hop_column) if edges_df_step is None or len(edges_df_step) == 0: oriented = self._gfql_empty_frame( edges_df_step if edges_df_step is not None else state_df, - columns=["__from__", "__to__"], + columns=[WALK_FROM_COL, WALK_TO_COL], ) + if TRAIL_EDGE_IDENT_COL not in oriented.columns: + oriented[TRAIL_EDGE_IDENT_COL] = self._gfql_broadcast_scalar(oriented, None) else: oriented = sem.orient_edges( edges_df_step, @@ -3858,27 +3957,47 @@ def _gfql_connected_bindings_state( dst_col, dedupe=False, ).rename(columns=rename_map) + if not shortest_path_mode and TRAIL_EDGE_IDENT_COL in oriented.columns: + # A self-loop's two undirected orientations are the SAME binding: drop the twin. + oriented = oriented.drop_duplicates( + subset=[WALK_FROM_COL, WALK_TO_COL, TRAIL_EDGE_IDENT_COL], keep="first" + ) if sem.is_multihop: - step_pairs = oriented[["__from__", "__to__"]] + step_cols = [WALK_FROM_COL, WALK_TO_COL] + ( + [TRAIL_EDGE_IDENT_COL] + if not shortest_path_mode and TRAIL_EDGE_IDENT_COL in oriented.columns + else [] + ) + step_pairs = oriented[step_cols] + if TRAIL_EDGE_IDENT_COL not in step_cols: + step_pairs = step_pairs.drop_duplicates(keep="first") + # else: already unique -- the self-loop-twin dedup above ran on exactly step_cols min_hops = edge_op.min_hops if edge_op.min_hops is not None else ( edge_op.hops if edge_op.hops is not None else 1 ) max_hops = edge_op.max_hops if edge_op.max_hops is not None else ( edge_op.hops if edge_op.hops is not None else None ) - state_df = self._gfql_multihop_binding_rows( + state_df, segment_trail_cols = self._gfql_multihop_binding_rows( state_df, step_pairs, min_hops=min_hops, max_hops=max_hops, to_fixed_point=bool(edge_op.to_fixed_point), - avoid_immediate_backtrack=sem.is_undirected, + avoid_immediate_backtrack=sem.is_undirected and shortest_path_mode, hop_column=edge_op.label_node_hops, + trail_cols=trail_cols, ) + trail_cols = trail_cols + segment_trail_cols else: - state_df = state_df.merge(oriented, left_on="__current__", right_on="__from__", how="inner") - state_df = state_df.drop(columns=["__current__", "__from__"]).rename(columns={"__to__": "__current__"}) + state_df = state_df.merge(oriented, left_on=WALK_CURRENT_COL, right_on=WALK_FROM_COL, how="inner") + state_df = state_df.drop(columns=[WALK_CURRENT_COL, WALK_FROM_COL]).rename(columns={WALK_TO_COL: WALK_CURRENT_COL}) + if not shortest_path_mode and TRAIL_EDGE_IDENT_COL in state_df.columns: + state_df = RowPipelineMixin._gfql_drop_reused_relationship_rows(state_df, trail_cols) + new_trail_col = trail_column_name(len(trail_cols)) + state_df = state_df.rename(columns={TRAIL_EDGE_IDENT_COL: new_trail_col}) + trail_cols = trail_cols + [new_trail_col] if len(state_df) == 0: return state_df, alias_frames @@ -3908,7 +4027,7 @@ def _gfql_connected_bindings_state( else base_nodes ) ) - candidate_nodes = candidate_source[candidate_source[node_id_col].isin(state_df["__current__"])].copy() + candidate_nodes = candidate_source[candidate_source[node_id_col].isin(state_df[WALK_CURRENT_COL])].copy() if not sem.is_multihop and edge_op.direction == "forward": candidate_nodes = self._gfql_disambiguate_has_edge_destination_nodes( candidate_nodes, @@ -3930,14 +4049,14 @@ def _gfql_connected_bindings_state( next_nodes = self._gfql_apply_alias_prefilter( next_nodes, node_alias, alias_prefilters ) - state_df = state_df[state_df["__current__"].isin(next_nodes[node_id_col])].copy() + state_df = state_df[state_df[WALK_CURRENT_COL].isin(next_nodes[node_id_col])].copy() if isinstance(node_alias, str): - state_df[node_alias] = state_df["__current__"] + state_df[node_alias] = state_df[WALK_CURRENT_COL] hop_column = edge_op.label_node_hops if hop_column is not None and hop_column in state_df.columns: hop_lookup = ( - state_df[["__current__", hop_column]] - .rename(columns={"__current__": node_id_col}) + state_df[[WALK_CURRENT_COL, hop_column]] + .rename(columns={WALK_CURRENT_COL: node_id_col}) .groupby(node_id_col, sort=False)[hop_column] .min() .reset_index() @@ -3950,6 +4069,8 @@ def _gfql_connected_bindings_state( ) alias_frames[node_alias] = next_nodes + if trail_cols: + state_df = state_df.drop(columns=[c for c in trail_cols if c in state_df.columns]) return state_df, alias_frames def _gfql_connected_bindings_row_table( @@ -3987,8 +4108,7 @@ def _gfql_is_shortest_path_scalar_binding_ops(ops: Sequence[Any]) -> bool: return ( isinstance(start_alias, str) and isinstance(end_alias, str) - and isinstance(hop_column, str) - and hop_column.startswith("__cypher_shortest_path_hops__") + and is_shortest_path_hops_column(hop_column) ) def _gfql_connected_bindings_row_table_from_ops( @@ -4103,11 +4223,7 @@ def _gfql_connected_bindings_row_frame_from_state( if base_nodes is not None and node_id in base_nodes.columns else self._gfql_empty_frame(base_nodes, columns=[node_id]) ) - # #1711 projection-pushdown: attach_prop_aliases (from the cypher lowering) - # names the node aliases whose PROPERTIES are referenced downstream. Aliases - # not listed skip the O(N) property left-join — their bare id column (already - # in state) is all the query needs (e.g. count(*) references nothing, - # count(a) references only the bare column). None = attach all (default). + # attach_prop_aliases names the aliases whose PROPERTIES are read downstream; None = all. attach_set = None if attach_prop_aliases is None else set(attach_prop_aliases) bindings = state_df.copy() @@ -4136,12 +4252,12 @@ def _gfql_connected_bindings_row_frame_from_state( dup_col = f"{node_id}__{alias}_join__" if dup_col in bindings.columns: bindings = bindings.drop(columns=[dup_col]) - for hop_col in [col for col in bindings.columns if str(col).startswith("__cypher_shortest_path_hops__")]: + for hop_col in [col for col in bindings.columns if is_shortest_path_hops_column(str(col))]: alias_hop_col = f"{alias}.{hop_col}" if alias_hop_col in bindings.columns: bindings[alias_hop_col] = bindings[hop_col] - drop_cols = ["__current__"] + drop_cols = [WALK_CURRENT_COL] bindings = bindings.drop(columns=[col for col in drop_cols if col in bindings.columns]) if len(bindings) == 0: bindings = self._gfql_add_missing_binding_columns(bindings, ops) @@ -4212,7 +4328,7 @@ def _gfql_shortest_path_scalar_native( null_val = self._gfql_broadcast_scalar(base, None) return base.assign(**{hop_column: null_val, f"{end_alias}.{hop_column}": null_val}) - step_pairs = sem.orient_edges(edges_df, src_col, dst_col, dedupe=True)[["__from__", "__to__"]] + step_pairs = sem.orient_edges(edges_df, src_col, dst_col, dedupe=True)[[WALK_FROM_COL, WALK_TO_COL]] sources = seed_table[start_alias] targets = seed_table[end_alias] @@ -4478,7 +4594,7 @@ def _gfql_project_items( if isinstance(expr, str): value = table_df[expr] if expr in table_df.columns else self._gfql_eval_string_expr(table_df, expr) is_series_value = isinstance(value, pd.Series) or value.__class__.__module__.startswith("cudf") - if normalize_shortest_path_hops and "__cypher_shortest_path_hops__" in expr and is_series_value: + if normalize_shortest_path_hops and SHORTEST_PATH_HOPS_COLUMN_PREFIX in expr and is_series_value: to_numeric = None try: to_numeric = s_to_numeric(resolve_engine(EngineAbstract.AUTO, table_df)) @@ -5188,12 +5304,7 @@ def group_by( table_df = table_df.assign(**{group_order_col: range(len(table_df))}) def _make_grouped(df: Any, value_cols: Any = ()) -> Any: - # Group over ONLY the key columns + the value columns THIS call aggregates. Carrying - # unrelated non-key/non-agg columns (e.g. an object 'name' or float 'f') into the - # groupby pushes cuDF onto a Series-truthiness path that raises "The truth value of a - # Series is ambiguous" (issue #1663 finding 4); pandas/polars tolerate the extra cols. - # Projecting first yields an IDENTICAL result on every engine (selecting value columns - # before grouping cannot change group sizes or per-column reductions) and sidesteps it. + # Group over ONLY key + this call's value columns; a spare column trips cuDF's groupby. keep_cols = list(dict.fromkeys([*key_cols, *(c for c in value_cols if c in df.columns)])) df = df[keep_cols] diff --git a/graphistry/compute/gfql/same_path/bfs.py b/graphistry/compute/gfql/same_path/bfs.py index 0872415837..42f56a5d46 100644 --- a/graphistry/compute/gfql/same_path/bfs.py +++ b/graphistry/compute/gfql/same_path/bfs.py @@ -1,6 +1,7 @@ from typing import Any, Dict, Optional, Sequence, TYPE_CHECKING, Union from graphistry.compute.ast import ASTEdge from graphistry.compute.typing import DataFrameT, DomainT +from graphistry.compute.gfql.identifiers import WALK_CURRENT_COL, WALK_FROM_COL, WALK_TO_COL from .edge_semantics import EdgeSemantics from .df_utils import ( concat_frames, @@ -24,14 +25,14 @@ def bfs_reachability(edge_pairs: DataFrameT, start_nodes: Union[Sequence[Any], D result = domain_to_frame(edge_pairs, start_domain, '__node__') result[hop_col] = 0 visited_idx = start_domain - frontier = result[['__node__']].rename(columns={'__node__': '__from__'}) + frontier = result[['__node__']].rename(columns={'__node__': WALK_FROM_COL}) for hop in range(1, max_hops + 1): if len(frontier) == 0: break next_df = ( - edge_pairs.merge(frontier, on='__from__', how='inner')[['__to__']] - .rename(columns={'__to__': '__node__'}) + edge_pairs.merge(frontier, on=WALK_FROM_COL, how='inner')[[WALK_TO_COL]] + .rename(columns={WALK_TO_COL: '__node__'}) .drop_duplicates() ) candidate_nodes = series_values(next_df['__node__']) @@ -41,7 +42,7 @@ def bfs_reachability(edge_pairs: DataFrameT, start_nodes: Union[Sequence[Any], D new_nodes = domain_to_frame(edge_pairs, new_node_ids, '__node__') new_nodes[hop_col] = hop visited_idx = domain_union(visited_idx, new_node_ids) - frontier = new_nodes[['__node__']].rename(columns={'__node__': '__from__'}) + frontier = new_nodes[['__node__']].rename(columns={'__node__': WALK_FROM_COL}) merged = concat_frames([result, new_nodes]) if merged is None: @@ -73,11 +74,11 @@ def walk_edge_state(executor: "DFSamePathExecutor", edge_indices: Sequence[int], next_state = ( edge_pairs.merge( current_state, - left_on="__from__", - right_on="__current__", + left_on=WALK_FROM_COL, + right_on=WALK_CURRENT_COL, how="inner", - )[["__to__"] + label_list] - .rename(columns={"__to__": "__current__"}) + )[[WALK_TO_COL] + label_list] + .rename(columns={WALK_TO_COL: WALK_CURRENT_COL}) .drop_duplicates() ) if len(next_state) == 0: @@ -99,14 +100,14 @@ def walk_edge_state(executor: "DFSamePathExecutor", edge_indices: Sequence[int], join_col, result_col = sem.join_cols(src_col, dst_col) if sem.is_undirected: next1 = ( - edges_df.merge(state_df, left_on=src_col, right_on="__current__", how="inner") + edges_df.merge(state_df, left_on=src_col, right_on=WALK_CURRENT_COL, how="inner") [[dst_col] + label_list] - .rename(columns={dst_col: "__current__"}) + .rename(columns={dst_col: WALK_CURRENT_COL}) ) next2 = ( - edges_df.merge(state_df, left_on=dst_col, right_on="__current__", how="inner") + edges_df.merge(state_df, left_on=dst_col, right_on=WALK_CURRENT_COL, how="inner") [[src_col] + label_list] - .rename(columns={src_col: "__current__"}) + .rename(columns={src_col: WALK_CURRENT_COL}) ) merged = concat_frames([next1, next2]) if merged is None: @@ -116,9 +117,9 @@ def walk_edge_state(executor: "DFSamePathExecutor", edge_indices: Sequence[int], continue state_df = ( - edges_df.merge(state_df, left_on=join_col, right_on="__current__", how="inner") + edges_df.merge(state_df, left_on=join_col, right_on=WALK_CURRENT_COL, how="inner") [[result_col] + label_list] - .rename(columns={result_col: "__current__"}) + .rename(columns={result_col: WALK_CURRENT_COL}) .drop_duplicates() ) return state_df diff --git a/graphistry/compute/gfql/same_path/edge_semantics.py b/graphistry/compute/gfql/same_path/edge_semantics.py index cd0c7dd06e..e4ea220d5b 100644 --- a/graphistry/compute/gfql/same_path/edge_semantics.py +++ b/graphistry/compute/gfql/same_path/edge_semantics.py @@ -2,6 +2,7 @@ from typing import Tuple from graphistry.compute.ast import ASTEdge from graphistry.compute.typing import DataFrameT, DomainT +from graphistry.compute.gfql.identifiers import WALK_FROM_COL, WALK_TO_COL from .df_utils import concat_frames, series_values, domain_union @@ -33,7 +34,7 @@ def start_nodes(self, edges_df: DataFrameT, src_col: str, dst_col: str) -> Domai return domain_union(series_values(edges_df[src_col]), series_values(edges_df[dst_col])) return series_values(edges_df[dst_col] if self.is_reverse else edges_df[src_col]) - def orient_edges(self, edges_df: DataFrameT, src_col: str, dst_col: str, *, from_col: str = "__from__", to_col: str = "__to__", dedupe: bool = False) -> DataFrameT: + def orient_edges(self, edges_df: DataFrameT, src_col: str, dst_col: str, *, from_col: str = WALK_FROM_COL, to_col: str = WALK_TO_COL, dedupe: bool = False) -> DataFrameT: if self.is_undirected: fwd = edges_df.rename(columns={src_col: from_col, dst_col: to_col}) rev = edges_df.rename(columns={dst_col: from_col, src_col: to_col}) diff --git a/graphistry/compute/gfql/same_path/multihop.py b/graphistry/compute/gfql/same_path/multihop.py index 88acb355fb..6eed0f80af 100644 --- a/graphistry/compute/gfql/same_path/multihop.py +++ b/graphistry/compute/gfql/same_path/multihop.py @@ -3,6 +3,7 @@ from graphistry.compute.ast import ASTEdge from graphistry.compute.dataframe import project_node_attrs, semijoin_eval_pairs from graphistry.compute.typing import DataFrameT, DomainT +from graphistry.compute.gfql.identifiers import WALK_CURRENT_COL, WALK_FROM_COL, WALK_TO_COL from graphistry.compute.gfql.same_path_types import ( EQ_NEQ_WHERE_OPS, INEQ_WHERE_OPS, @@ -32,7 +33,7 @@ def filter_multihop_edges_by_endpoints(edges_df: DataFrameT, left_allowed: Optio left_domain = domain_from_values(left_allowed, edge_pairs) right_domain = domain_from_values(right_allowed, edge_pairs) fwd_df = bfs_reachability(edge_pairs, left_domain, sem.max_hops, "__fwd_hop__") - rev_edge_pairs = edge_pairs.rename(columns={"__from__": "__to__", "__to__": "__from__"}) + rev_edge_pairs = edge_pairs.rename(columns={WALK_FROM_COL: WALK_TO_COL, WALK_TO_COL: WALK_FROM_COL}) bwd_df = bfs_reachability(rev_edge_pairs, right_domain, sem.max_hops, "__bwd_hop__") if len(fwd_df) == 0 or len(bwd_df) == 0: return edges_df.iloc[:0] @@ -162,7 +163,7 @@ def _empty_pair(left_df: DataFrameT, right_df: DataFrameT, start_idx: int, end_i def _edge_pairs_cached(edge_idx: int, sem: EdgeSemantics, allowed_edges: Optional[DomainT]) -> DataFrameT: edges_df = executor.forward_steps[edge_idx]._edges if edges_df is None or len(edges_df) == 0: - return df_cons(nodes_df, {"__from__": [], "__to__": []}) + return df_cons(nodes_df, {WALK_FROM_COL: [], WALK_TO_COL: []}) if allowed_edges is None: cached = edge_pairs_cache.get(edge_idx) if cached is None: @@ -173,14 +174,14 @@ def _edge_pairs_cached(edge_idx: int, sem: EdgeSemantics, allowed_edges: Optiona edges_df = edges_df[edges_df[edge_id_col].isin(allowed_edges)] return build_edge_pairs(edges_df, src, dst, sem) - def _pairs_from_endpoints(pairs_left: DataFrameT, pairs_right: DataFrameT, start_df: DataFrameT, end_df: DataFrameT, left_cols: Sequence[str], right_cols: Sequence[str], *, left_id: str = "__start__", right_id: str = "__current__") -> Tuple[DataFrameT, DataFrameT]: - start_vals = start_df[[left_id] + list(left_cols)].rename(columns={left_id: "__from__"}).drop_duplicates() - end_vals = end_df[[right_id] + list(right_cols)].rename(columns={right_id: "__to__"}).drop_duplicates() - left_pairs = pairs_left.merge(start_vals, on="__from__", how="inner").rename(columns={"__from__": "__start__", "__to__": "__mid__"})[ + def _pairs_from_endpoints(pairs_left: DataFrameT, pairs_right: DataFrameT, start_df: DataFrameT, end_df: DataFrameT, left_cols: Sequence[str], right_cols: Sequence[str], *, left_id: str = "__start__", right_id: str = WALK_CURRENT_COL) -> Tuple[DataFrameT, DataFrameT]: + start_vals = start_df[[left_id] + list(left_cols)].rename(columns={left_id: WALK_FROM_COL}).drop_duplicates() + end_vals = end_df[[right_id] + list(right_cols)].rename(columns={right_id: WALK_TO_COL}).drop_duplicates() + left_pairs = pairs_left.merge(start_vals, on=WALK_FROM_COL, how="inner").rename(columns={WALK_FROM_COL: "__start__", WALK_TO_COL: "__mid__"})[ ["__start__", "__mid__"] + list(left_cols) ] - right_pairs = pairs_right.merge(end_vals, on="__to__", how="inner").rename(columns={"__from__": "__mid__", "__to__": "__current__"})[ - ["__mid__", "__current__"] + list(right_cols) + right_pairs = pairs_right.merge(end_vals, on=WALK_TO_COL, how="inner").rename(columns={WALK_FROM_COL: "__mid__", WALK_TO_COL: WALK_CURRENT_COL})[ + ["__mid__", WALK_CURRENT_COL] + list(right_cols) ] return left_pairs, right_pairs @@ -208,7 +209,7 @@ def _over_pair_limit(pair_est: float, edge_pair_est: Optional[float], limit: Opt end_cols = [end_col for _, end_col, _ in eq_entries] label_cols = [f"__label{idx}__" for idx in range(len(start_cols))] start_df = _attr_frame(start_nodes, start_cols, "__start__", label_cols) - end_df = _attr_frame(end_nodes, end_cols, "__current__", label_cols) + end_df = _attr_frame(end_nodes, end_cols, WALK_CURRENT_COL, label_cols) if start_df is None or end_df is None: continue if _empty_pair(start_df, end_df, start_node_idx, end_node_idx): @@ -216,12 +217,12 @@ def _over_pair_limit(pair_est: float, edge_pair_est: Optional[float], limit: Opt label_cardinality = max(len(start_df[label_cols].drop_duplicates()), len(end_df[label_cols].drop_duplicates())) if value_card_max is None or label_cardinality <= value_card_max: processed_clause_ids.update(group_clause_ids) - state_df = start_df[["__start__"] + label_cols].rename(columns={"__start__": "__current__"}).drop_duplicates() + state_df = start_df[["__start__"] + label_cols].rename(columns={"__start__": WALK_CURRENT_COL}).drop_duplicates() state_df = walk_edge_state(executor, relevant_edge_indices, state_df, label_cols, local_allowed_edges, edge_id_col, src_col, dst_col) - state_df = state_df[state_df["__current__"].isin(end_nodes)] + state_df = state_df[state_df[WALK_CURRENT_COL].isin(end_nodes)] if _empty_pair(state_df, state_df, start_node_idx, end_node_idx): continue - valid_labels = state_df.merge(end_df, on=["__current__"] + label_cols, how="inner")[label_cols].drop_duplicates() + valid_labels = state_df.merge(end_df, on=[WALK_CURRENT_COL] + label_cols, how="inner")[label_cols].drop_duplicates() if len(valid_labels) == 0: _set_empty_nodes(start_node_idx, end_node_idx) continue @@ -229,7 +230,7 @@ def _over_pair_limit(pair_est: float, edge_pair_est: Optional[float], limit: Opt valid_ends_df = end_df.merge(valid_labels, on=label_cols, how="inner") if _empty_pair(valid_starts_df, valid_ends_df, start_node_idx, end_node_idx): continue - _apply_pairs_and_backprop(start_node_idx, end_node_idx, valid_starts_df["__start__"], valid_ends_df["__current__"]) + _apply_pairs_and_backprop(start_node_idx, end_node_idx, valid_starts_df["__start__"], valid_ends_df[WALK_CURRENT_COL]) for clause_entries in endpoint_clauses.values(): endpoint_clause_count = len(clause_entries) for clause, start_idx, end_idx, _, _ in clause_entries: @@ -243,7 +244,7 @@ def _over_pair_limit(pair_est: float, edge_pair_est: Optional[float], limit: Opt if start_nodes is None or end_nodes is None: continue left_vals = _attr_frame(start_nodes, [clause.left.column], "__start__", ["__start_val__"]) - right_vals = _attr_frame(end_nodes, [clause.right.column], "__current__", ["__end_val__"]) + right_vals = _attr_frame(end_nodes, [clause.right.column], WALK_CURRENT_COL, ["__end_val__"]) if left_vals is None or right_vals is None: continue if _empty_pair(left_vals, right_vals, start_idx, end_idx): @@ -283,7 +284,7 @@ def _over_pair_limit(pair_est: float, edge_pair_est: Optional[float], limit: Opt if len(left_vals_df) == 0: _set_empty_nodes(start_idx, end_idx) continue - _apply_pairs_and_backprop(start_idx, end_idx, left_vals_df["__start__"], right_vals_df["__current__"], backprop=False) + _apply_pairs_and_backprop(start_idx, end_idx, left_vals_df["__start__"], right_vals_df[WALK_CURRENT_COL], backprop=False) left_domain = series_values(left_vals_df["__start_val__"]) right_domain = series_values(right_vals_df["__end_val__"]) if bounds_enabled and clause.op in INEQ_WHERE_OPS: @@ -298,7 +299,7 @@ def _over_pair_limit(pair_est: float, edge_pair_est: Optional[float], limit: Opt if _empty_pair(left_vals_df, right_vals_df, start_idx, end_idx): continue start_nodes = series_values(left_vals_df["__start__"]) - end_nodes = series_values(right_vals_df["__current__"]) + end_nodes = series_values(right_vals_df[WALK_CURRENT_COL]) _update_allowed(start_idx, start_nodes) _update_allowed(end_idx, end_nodes) left_domain = series_values(left_vals_df["__start_val__"]) @@ -342,11 +343,11 @@ def _over_pair_limit(pair_est: float, edge_pair_est: Optional[float], limit: Opt ) if start_nodes is not None and not domain_is_empty(start_nodes): pairs_left = pairs_left[ - pairs_left["__from__"].isin(start_nodes) + pairs_left[WALK_FROM_COL].isin(start_nodes) ] if end_nodes is not None and not domain_is_empty(end_nodes): pairs_right = pairs_right[ - pairs_right["__to__"].isin(end_nodes) + pairs_right[WALK_TO_COL].isin(end_nodes) ] force_semijoin = ( (not domain_semijoin_enabled) @@ -363,7 +364,7 @@ def _over_pair_limit(pair_est: float, edge_pair_est: Optional[float], limit: Opt left_pairs, right_pairs = _pairs_from_endpoints(pairs_left, pairs_right, start_val_df, end_val_df, ["__value__"], ["__value__"]) if _empty_pair(left_pairs, right_pairs, start_idx, end_idx): continue - left_eval, right_eval, mid_values = semijoin_eval_pairs(left_pairs, right_pairs, clause.op, left_value="__value__", right_value="__value__", left_unique_col="__left_unique__", right_unique_col="__right_unique__", left_only_col="__left_only__", right_only_col="__right_only__", left_keep=["__start__"], right_keep=["__current__"]) + left_eval, right_eval, mid_values = semijoin_eval_pairs(left_pairs, right_pairs, clause.op, left_value="__value__", right_value="__value__", left_unique_col="__left_unique__", right_unique_col="__right_unique__", left_only_col="__left_only__", right_only_col="__right_only__", left_keep=["__start__"], right_keep=[WALK_CURRENT_COL]) if mid_values is not None: if left_eval is None or right_eval is None: _set_empty_nodes(start_idx, end_idx) @@ -371,18 +372,18 @@ def _over_pair_limit(pair_est: float, edge_pair_est: Optional[float], limit: Opt else: if left_eval is None or right_eval is None or _empty_pair(left_eval, right_eval, start_idx, end_idx): continue - _apply_pairs_and_backprop(start_idx, end_idx, left_eval["__start__"], right_eval["__current__"]) + _apply_pairs_and_backprop(start_idx, end_idx, left_eval["__start__"], right_eval[WALK_CURRENT_COL]) continue state_label_col = "__start_val__" if value_mode_enabled else "__start__" - state_df = left_vals[["__start__", state_label_col]].rename(columns={"__start__": "__current__"}).drop_duplicates() if value_mode_enabled else left_vals[["__start__"]].assign(__current__=left_vals["__start__"]) + state_df = left_vals[["__start__", state_label_col]].rename(columns={"__start__": WALK_CURRENT_COL}).drop_duplicates() if value_mode_enabled else left_vals[["__start__"]].assign(__current__=left_vals["__start__"]) state_df = walk_edge_state(executor, edge_idxs, state_df, [state_label_col], local_allowed_edges, edge_id_col, src_col, dst_col) if end_nodes is None: continue - state_df = state_df[state_df["__current__"].isin(end_nodes)] + state_df = state_df[state_df[WALK_CURRENT_COL].isin(end_nodes)] if len(state_df) == 0: _set_empty_nodes(start_idx, end_idx) continue - pairs_df = state_df.merge(right_vals, on="__current__", how="inner") if value_mode_enabled else state_df.merge(left_vals, on="__start__", how="inner").merge(right_vals, on="__current__", how="inner") + pairs_df = state_df.merge(right_vals, on=WALK_CURRENT_COL, how="inner") if value_mode_enabled else state_df.merge(left_vals, on="__start__", how="inner").merge(right_vals, on=WALK_CURRENT_COL, how="inner") left_col = state_label_col if value_mode_enabled else "__start_val__" mask = evaluate_clause(pairs_df[left_col], clause.op, pairs_df["__end_val__"], null_safe=True) valid_pairs = pairs_df[mask] @@ -390,6 +391,6 @@ def _over_pair_limit(pair_est: float, edge_pair_est: Optional[float], limit: Opt start_series = left_vals[left_vals["__start_val__"].isin(series_values(valid_pairs[left_col]))]["__start__"] else: start_series = valid_pairs["__start__"] - end_series = valid_pairs["__current__"] + end_series = valid_pairs[WALK_CURRENT_COL] _apply_pairs_and_backprop(start_idx, end_idx, start_series, end_series) return PathState.from_mutable(local_allowed_nodes, local_allowed_edges, local_pruned_edges) diff --git a/graphistry/compute/gfql/same_path/native_shortest_path.py b/graphistry/compute/gfql/same_path/native_shortest_path.py index cce9d2d54e..dba31b7172 100644 --- a/graphistry/compute/gfql/same_path/native_shortest_path.py +++ b/graphistry/compute/gfql/same_path/native_shortest_path.py @@ -27,6 +27,7 @@ from typing import Any, Dict, Hashable, Literal, Optional, Tuple import pandas as pd +from graphistry.compute.gfql.identifiers import WALK_FROM_COL, WALK_TO_COL logger = logging.getLogger(__name__) @@ -40,8 +41,8 @@ def _build_igraph_shortest_path_state(step_pairs: Any, *, directed: bool) -> IgraphShortestPathState: import igraph as ig # type: ignore[import] - sp_frm = list(step_pairs["__from__"]) - sp_to = list(step_pairs["__to__"]) + sp_frm = list(step_pairs[WALK_FROM_COL]) + sp_to = list(step_pairs[WALK_TO_COL]) all_nodes = list(dict.fromkeys(sp_frm + sp_to)) node_index = {n: i for i, n in enumerate(all_nodes)} edges = [(node_index[f], node_index[t]) for f, t in zip(sp_frm, sp_to)] @@ -166,8 +167,8 @@ def cugraph_shortest_path_distances( def _build_graph() -> Any: edges_gdf = cudf.DataFrame({ - "src": step_pairs["__from__"], - "dst": step_pairs["__to__"], + "src": step_pairs[WALK_FROM_COL], + "dst": step_pairs[WALK_TO_COL], }) graph = cugraph.Graph(directed=directed) graph.from_cudf_edgelist(edges_gdf, source="src", destination="dst") diff --git a/graphistry/compute/gfql/same_path/where_filter.py b/graphistry/compute/gfql/same_path/where_filter.py index 7e24fae7f4..e5862dcb5f 100644 --- a/graphistry/compute/gfql/same_path/where_filter.py +++ b/graphistry/compute/gfql/same_path/where_filter.py @@ -3,6 +3,7 @@ from graphistry.compute.ast import ASTEdge, ASTNode from graphistry.compute.dataframe import project_node_attrs, semijoin_eval_pairs from graphistry.compute.typing import DataFrameT, DomainT +from graphistry.compute.gfql.identifiers import WALK_FROM_COL, WALK_TO_COL from graphistry.compute.gfql.same_path_types import ( ComparisonOp, EQ_NEQ_WHERE_OPS, @@ -162,12 +163,12 @@ def _merge_edges_with_pairs(edges_df: DataFrameT, sem: EdgeSemantics, pairs_df: edges_with_id = edges_df.reset_index(drop=True) edges_with_id[edge_id_col] = edges_with_id.index oriented = sem.orient_edges(edges_with_id, src, dst, dedupe=sem.is_undirected) - rename_map = {left_label: "__from__", right_label: "__to__"} + rename_map = {left_label: WALK_FROM_COL, right_label: WALK_TO_COL} if value_label is not None and value_col is not None: rename_map[value_label] = value_col - on_cols = ["__from__", "__to__", value_col] + on_cols = [WALK_FROM_COL, WALK_TO_COL, value_col] else: - on_cols = ["__from__", "__to__"] + on_cols = [WALK_FROM_COL, WALK_TO_COL] merged = oriented.merge(pairs_df.rename(columns=rename_map), on=on_cols, how="inner") edge_ids = merged[edge_id_col].drop_duplicates() edges_out = edges_with_id[edges_with_id[edge_id_col].isin(edge_ids)].copy() @@ -179,7 +180,7 @@ def _edges_for_step(edge_idx: int) -> Optional[DataFrameT]: def _build_value_pairs(edges_df: DataFrameT, sem: EdgeSemantics, value_col: str, left_label: str, right_label: str, value_label: str) -> DataFrameT: pairs = sem.orient_edges( edges_df[[src, dst, value_col]], src, dst, dedupe=sem.is_undirected - ).rename(columns={"__from__": left_label, "__to__": right_label, value_col: value_label}).drop_duplicates() + ).rename(columns={WALK_FROM_COL: left_label, WALK_TO_COL: right_label, value_col: value_label}).drop_duplicates() return pairs[pairs[value_label].notna()] if edge_semijoin_enabled or edge_semijoin_auto: @@ -324,9 +325,9 @@ def _build_value_pairs(edges_df: DataFrameT, sem: EdgeSemantics, value_col: str, edges_subset = edges_df_step[cols].rename(columns={col: f"e{edge_idx}_{col}" for col in cols[2:]}) left_col = f"n{left_node_idx}" edges_oriented = sem.orient_edges(edges_subset, src_col, dst_col) - paths_df = paths_df.merge(edges_oriented, left_on=left_col, right_on="__from__", how="inner") - paths_df[f"n{right_node_idx}"] = paths_df["__to__"] - paths_df = paths_df.drop(columns=["__from__", "__to__", src_col, dst_col], errors="ignore") + paths_df = paths_df.merge(edges_oriented, left_on=left_col, right_on=WALK_FROM_COL, how="inner") + paths_df[f"n{right_node_idx}"] = paths_df[WALK_TO_COL] + paths_df = paths_df.drop(columns=[WALK_FROM_COL, WALK_TO_COL, src_col, dst_col], errors="ignore") right_allowed = local_allowed_nodes.get(right_node_idx) if right_allowed is not None and not domain_is_empty(right_allowed): paths_df = paths_df[paths_df[f"n{right_node_idx}"].isin(right_allowed)] diff --git a/graphistry/tests/compute/gfql/cypher/test_ast_normalizer.py b/graphistry/tests/compute/gfql/cypher/test_ast_normalizer.py index ca6f1f62d1..4786d1cd97 100644 --- a/graphistry/tests/compute/gfql/cypher/test_ast_normalizer.py +++ b/graphistry/tests/compute/gfql/cypher/test_ast_normalizer.py @@ -9,6 +9,7 @@ from graphistry.compute.gfql.cypher.ast_normalizer import ASTNormalizer from graphistry.compute.gfql.cypher.lowering import lower_match_query from graphistry.compute.gfql.cypher.parser import parse_cypher +from graphistry.compute.gfql.identifiers import shortest_path_hops_column def _normalize(query: str) -> CypherQuery: @@ -26,7 +27,7 @@ def test_normalizer_rewrites_shortest_path_seed_and_projection() -> None: assert len(normalized.matches) == 2 assert normalized.matches[0].pattern_alias_kinds == ("pattern",) assert normalized.matches[1].pattern_alias_kinds == ("shortestPath",) - assert "__cypher_shortest_path_hops__path" in normalized.return_.items[0].expression.text + assert shortest_path_hops_column("path") in normalized.return_.items[0].expression.text def test_normalizer_keeps_where_pattern_predicate_as_existence_check() -> None: diff --git a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py index 2974f8c512..80806b232c 100644 --- a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py +++ b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py @@ -60,6 +60,7 @@ "MATCH (a)-[:R*]->(b) RETURN b", "MATCH p = shortestPath((a:X)-[:R*1..4]->(b:Y)) RETURN p", "MATCH (a)-[:R*2..]->(b) RETURN b", + "MATCH (a)-[:R*..4]->(b) RETURN b", # open LOWER bound = 1..4 (#1903 item 8) "MATCH (a)-[:R]->(b), (b)-[:S]->(c) RETURN a, c", "OPTIONAL MATCH (n)-[r]->(m) RETURN n, m", "MATCH (n) MATCH (m) RETURN n, m", @@ -158,7 +159,6 @@ "MATCH (n) RETURN", "MATCH (n)-[r->-(m) RETURN n", "WHERE n.x = 1 RETURN n", # WHERE without MATCH (rejected post-parse) - "MATCH (a)-[:R*..4]->(b) RETURN b", # open LOWER hop bound is not in the grammar ] diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index b012c6345b..d3571e82a9 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -3000,8 +3000,12 @@ def test_issue_1411_connected_join_property_projection_shape() -> None: "ORDER BY friendId" ) + # openCypher relationship uniqueness (#1903 item 4): friend=p1 would have + # to REBIND person's own IS_LOCATED_IN edge across pattern elements, so + # only p2 (its own edge) matches -- the old expectation pinned the + # cross-element edge reuse. (LDBC's reference queries add friend <> person + # precisely because Neo4j excludes the same-edge rebind, not the node.) assert result._nodes.to_dict(orient="records") == [ - {"friendId": "p1", "friendFirstName": "Seed", "cityName": "City"}, {"friendId": "p2", "friendFirstName": "Friend", "cityName": "City"}, ] @@ -10642,12 +10646,6 @@ def test_string_cypher_executes_undirected_multihop_row_bindings_on_cudf() -> No None, "do not yet support variable-length relationship aliases", ), - ( - _mk_multihop_row_binding_cycle_graph, - "MATCH (a:A)-[:R*0..]->(b) RETURN a.id AS aid, b.id AS bid", - None, - "currently require terminating variable-length segments", - ), ], ) def test_string_cypher_failfast_rejects_remaining_unsupported_multihop_row_bindings( @@ -10660,6 +10658,19 @@ def test_string_cypher_failfast_rejects_remaining_unsupported_multihop_row_bindi graph_factory().gfql(query, params=params) +def test_string_cypher_executes_unbounded_cycle_trail_termination() -> None: + """#1903: trail semantics bound an unbounded walk on a cycle (each edge + binds once per path), so `[*0..]` on a 2-cycle now SERVES on pandas: + zero-hop (a,a); e0 -> (a,b); e0,e1 -> (a,a); a third hop would reuse e0. + (polars' node-frontier probe still declines the reachable cycle -- its + terminating-segments error is the pinned #1903 residual.)""" + result = _mk_multihop_row_binding_cycle_graph().gfql( + "MATCH (a:A)-[:R*0..]->(b) RETURN a.id AS aid, b.id AS bid" + ) + got = sorted((r["aid"], r["bid"]) for r in result._nodes.to_dict(orient="records")) + assert got == [("a", "a"), ("a", "a"), ("a", "b")] + + def test_compile_cypher_records_reentry_plan_for_multi_whole_row_prefix() -> None: """#989 slice 4.1: ``compiled_query.reentry_plan`` is populated with one CarriedAlias per prefix whole-row, exactly one marked as the reentry source. @@ -13229,8 +13240,11 @@ def test_multi_alias_undirected_self_loop() -> None: ) records = _to_pandas_df(result._nodes).to_dict(orient="records") # Self-loop in undirected traversal matches both directions → 2 rows for the self-loop. - # KNOWS edge: 1 row with fid=2. Total: 3 rows. - assert len(records) == 3, f"Expected 3 rows (self-loop×2 + KNOWS), got {records}" + # openCypher (#1903 addendum A-1): a self-loop's two undirected + # orientations are the SAME relationship binding -> ONE row for the + # self-loop (the old x2 pinned the orientation-flip double-count). + # KNOWS edge: 1 row with fid=2. Total: 2 rows. + assert len(records) == 2, f"Expected 2 rows (self-loop + KNOWS), got {records}" assert records[0]["w"] == 10 # KNOWS edge first (weight 10) assert records[0]["fid"] == 2 assert all(r["fid"] == 1 for r in records[1:]), f"Self-loop rows should reference self, got {records}" diff --git a/graphistry/tests/compute/gfql/cypher/test_shortest_path_parity.py b/graphistry/tests/compute/gfql/cypher/test_shortest_path_parity.py index ef2ab4da98..2f9fc13875 100644 --- a/graphistry/tests/compute/gfql/cypher/test_shortest_path_parity.py +++ b/graphistry/tests/compute/gfql/cypher/test_shortest_path_parity.py @@ -1,3 +1,10 @@ +"""shortestPath parity pins. + +Conformance note (#1903 item 6): openCypher drops the row when a PLAIN-MATCH +shortestPath finds no path; only OPTIONAL MATCH null-extends. The disconnected +cases here therefore spell the LDBC IC13 idiom with OPTIONAL MATCH (as the LDBC +reference query itself does), keeping the null-row plumbing these tests pin. +""" from __future__ import annotations from dataclasses import dataclass @@ -90,8 +97,8 @@ def test_string_cypher_executes_shortest_path_reverse_direction_length_projectio """ MATCH (person1:Person {id: 'p3'}), - (person2:Person {id: 'p1'}), - path = shortestPath((person1)<-[:KNOWS*]-(person2)) + (person2:Person {id: 'p1'}) + OPTIONAL MATCH path = shortestPath((person1)<-[:KNOWS*]-(person2)) RETURN length(path) AS shortestPathLength """, ) @@ -120,8 +127,8 @@ def test_string_cypher_executes_shortest_path_endpoint_projection_with_length() """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN person1.id AS person1Id, person2.id AS person2Id, length(path) AS shortestPathLength """, ) @@ -152,8 +159,8 @@ def test_string_cypher_executes_shortest_path_disconnected_endpoint_projection_w """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN person1.id AS person1Id, person2.id AS person2Id, length(path) AS shortestPathLength, path IS NULL AS noPath """, ) @@ -184,8 +191,8 @@ def test_string_cypher_executes_shortest_path_with_length_stage_and_order_by() - """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, length(path) AS shortestPathLength RETURN person1Id, person2Id, shortestPathLength ORDER BY shortestPathLength @@ -218,8 +225,8 @@ def test_string_cypher_executes_shortest_path_disconnected_with_is_null_stage() """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) WITH path IS NULL AS noPath RETURN noPath """, @@ -249,8 +256,8 @@ def test_string_cypher_executes_shortest_path_disconnected_with_length_stage() - """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) WITH length(path) AS shortestPathLength RETURN shortestPathLength """, @@ -280,8 +287,8 @@ def test_string_cypher_executes_shortest_path_disconnected_with_endpoint_project """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, length(path) AS shortestPathLength, path IS NULL AS noPath RETURN person1Id, person2Id, shortestPathLength, noPath """, @@ -313,8 +320,8 @@ def test_string_cypher_executes_bounded_shortest_path_disconnected_with_length_s """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*1..3]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*1..3]-(person2)) WITH length(path) AS shortestPathLength RETURN shortestPathLength """, @@ -344,8 +351,8 @@ def test_string_cypher_executes_shortest_path_disconnected_with_case_stage() -> """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) WITH CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN shortestPathLength """, @@ -375,8 +382,8 @@ def test_string_cypher_executes_reverse_shortest_path_disconnected_with_case_sta """ MATCH (person1:Person {id: 'p3'}), - (person2:Person {id: 'p1'}), - path = shortestPath((person1)<-[:KNOWS*]-(person2)) + (person2:Person {id: 'p1'}) + OPTIONAL MATCH path = shortestPath((person1)<-[:KNOWS*]-(person2)) WITH CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN shortestPathLength """, @@ -406,8 +413,8 @@ def test_string_cypher_executes_reverse_shortest_path_with_endpoint_and_case_sta """ MATCH (person1:Person {id: 'p3'}), - (person2:Person {id: 'p1'}), - path = shortestPath((person1)<-[:KNOWS*]-(person2)) + (person2:Person {id: 'p1'}) + OPTIONAL MATCH path = shortestPath((person1)<-[:KNOWS*]-(person2)) WITH person2.id AS person2Id, CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN person2Id, shortestPathLength """, @@ -439,8 +446,8 @@ def test_string_cypher_executes_shortest_path_zero_hop_length() -> None: """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p1'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p1'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN length(path) AS shortestPathLength """, ) @@ -469,8 +476,8 @@ def test_string_cypher_executes_shortest_path_zero_hop_case_and_is_null() -> Non """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p1'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p1'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN CASE path IS NULL WHEN true THEN -1 @@ -506,8 +513,8 @@ def test_string_cypher_executes_shortest_path_zero_hop_with_length_stage() -> No """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p1'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p1'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) WITH length(path) AS shortestPathLength RETURN shortestPathLength """, @@ -537,8 +544,8 @@ def test_string_cypher_executes_reverse_shortest_path_zero_hop_case_stage() -> N """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p1'}), - path = shortestPath((person1)<-[:KNOWS*]-(person2)) + (person2:Person {id: 'p1'}) + OPTIONAL MATCH path = shortestPath((person1)<-[:KNOWS*]-(person2)) WITH CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN shortestPathLength """, @@ -568,8 +575,8 @@ def test_string_cypher_executes_shortest_path_on_cyclic_graph_with_multiple_shor """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p4'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p4'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN length(path) AS shortestPathLength """, ) @@ -598,8 +605,8 @@ def test_string_cypher_executes_shortest_path_stage_on_cyclic_graph_with_multipl """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p4'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p4'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN person1Id, person2Id, shortestPathLength """, @@ -631,8 +638,8 @@ def test_string_cypher_executes_bounded_shortest_path_on_cyclic_graph_with_multi """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p4'}), - path = shortestPath((person1)-[:KNOWS*1..3]-(person2)) + (person2:Person {id: 'p4'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*1..3]-(person2)) RETURN length(path) AS shortestPathLength """, ) @@ -661,8 +668,8 @@ def test_string_cypher_executes_bounded_shortest_path_on_self_loop_without_dupli """ MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p1'}), - path = shortestPath((person1)-[:KNOWS*1..3]-(person2)) + (person2:Person {id: 'p1'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*1..3]-(person2)) RETURN CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength """, ) @@ -691,9 +698,9 @@ def test_string_cypher_executes_multirow_shortest_path_pairs_without_zero_hop_co """ MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p3', 'p4'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN person1Id, person2Id, shortestPathLength ORDER BY person1Id, person2Id @@ -729,9 +736,9 @@ def test_string_cypher_executes_multirow_bounded_shortest_path_pairs_without_end """ MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*1..2]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p3', 'p4'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*1..2]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN person1Id, person2Id, shortestPathLength ORDER BY person1Id, person2Id @@ -767,9 +774,9 @@ def test_string_cypher_executes_multirow_disconnected_shortest_path_endpoint_pro """ MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p5'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN person1.id AS person1Id, person2.id AS person2Id, length(path) AS shortestPathLength, path IS NULL AS noPath ORDER BY person1Id, person2Id """, @@ -802,9 +809,9 @@ def test_string_cypher_executes_multirow_disconnected_shortest_path_case_stage() """ MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p5'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN person1Id, person2Id, shortestPathLength ORDER BY person1Id, person2Id @@ -838,9 +845,9 @@ def test_string_cypher_executes_multirow_bounded_disconnected_shortest_path_endp """ MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*1..2]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p5'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*1..2]-(person2)) RETURN person1.id AS person1Id, person2.id AS person2Id, length(path) AS shortestPathLength, path IS NULL AS noPath ORDER BY person1Id, person2Id """, @@ -873,9 +880,9 @@ def test_string_cypher_executes_multirow_bounded_disconnected_shortest_path_case """ MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*1..2]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p5'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*1..2]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN person1Id, person2Id, shortestPathLength ORDER BY person1Id, person2Id @@ -909,9 +916,9 @@ def test_string_cypher_executes_multirow_disconnected_reverse_shortest_path_endp """ MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)<-[:KNOWS*]-(person2)) + (person2:Person) WHERE person1.id IN ['p5'] AND person2.id IN ['p1', 'p2'] + OPTIONAL MATCH path = shortestPath((person1)<-[:KNOWS*]-(person2)) RETURN person1.id AS person1Id, person2.id AS person2Id, length(path) AS shortestPathLength, path IS NULL AS noPath ORDER BY person1Id, person2Id """, @@ -944,9 +951,9 @@ def test_string_cypher_executes_multirow_disconnected_reverse_shortest_path_case """ MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)<-[:KNOWS*]-(person2)) + (person2:Person) WHERE person1.id IN ['p5'] AND person2.id IN ['p1', 'p2'] + OPTIONAL MATCH path = shortestPath((person1)<-[:KNOWS*]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN person1Id, person2Id, shortestPathLength ORDER BY person1Id, person2Id @@ -980,9 +987,9 @@ def test_string_cypher_executes_multirow_shortest_path_is_null_ordering() -> Non """ MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p3', 'p5'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN person1.id AS person1Id, person2.id AS person2Id, path IS NULL AS noPath ORDER BY noPath, person1Id, person2Id """, @@ -1017,9 +1024,9 @@ def test_string_cypher_executes_multirow_bounded_shortest_path_is_null_stage_ord """ MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*1..2]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p3', 'p4', 'p5'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*1..2]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, path IS NULL AS noPath RETURN person1Id, person2Id, noPath ORDER BY noPath, person1Id, person2Id @@ -1057,9 +1064,9 @@ def test_string_cypher_executes_multirow_reverse_shortest_path_is_null_stage_ord """ MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)<-[:KNOWS*]-(person2)) + (person2:Person) WHERE person1.id IN ['p3', 'p5'] AND person2.id IN ['p1', 'p2'] + OPTIONAL MATCH path = shortestPath((person1)<-[:KNOWS*]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, path IS NULL AS noPath RETURN person1Id, person2Id, noPath ORDER BY noPath, person1Id, person2Id @@ -1175,8 +1182,8 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person {id: $person1Id}), - (person2:Person {id: $person2Id}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: $person2Id}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN CASE path IS NULL WHEN true THEN -1 @@ -1192,8 +1199,8 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person {id: $person1Id}), - (person2:Person {id: $person2Id}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: $person2Id}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN CASE path IS NULL WHEN true THEN -1 @@ -1209,8 +1216,8 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*1..3]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*1..3]-(person2)) RETURN length(path) AS shortestPathLength """, expected_rows=[{"shortestPathLength": 2}], @@ -1221,8 +1228,8 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person {id: 'p3'}), - (person2:Person {id: 'p1'}), - path = shortestPath((person1)<-[:KNOWS*]-(person2)) + (person2:Person {id: 'p1'}) + OPTIONAL MATCH path = shortestPath((person1)<-[:KNOWS*]-(person2)) RETURN length(path) AS shortestPathLength """, expected_rows=[{"shortestPathLength": 2}], @@ -1233,9 +1240,9 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*1..2]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p3', 'p4'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*1..2]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN person1Id, person2Id, shortestPathLength ORDER BY person1Id, person2Id @@ -1253,8 +1260,8 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN path IS NULL AS noPath """, expected_rows=[{"noPath": True}], @@ -1265,8 +1272,8 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN path IS NULL AS noPath """, expected_rows=[{"noPath": False}], @@ -1277,8 +1284,8 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p3'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p3'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN length(path) AS shortestPathLength """, expected_rows=[{"shortestPathLength": None}], @@ -1289,8 +1296,8 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p1'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p1'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN CASE path IS NULL WHEN true THEN -1 @@ -1306,8 +1313,8 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p4'}), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person {id: 'p4'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN length(path) AS shortestPathLength """, expected_rows=[{"shortestPathLength": 2}], @@ -1318,8 +1325,8 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person {id: 'p1'}), - (person2:Person {id: 'p1'}), - path = shortestPath((person1)-[:KNOWS*1..3]-(person2)) + (person2:Person {id: 'p1'}) + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*1..3]-(person2)) RETURN CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength """, expected_rows=[{"shortestPathLength": 1}], @@ -1330,9 +1337,9 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p3', 'p4'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN person1Id, person2Id, shortestPathLength ORDER BY person1Id, person2Id @@ -1350,9 +1357,9 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p5'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN person1.id AS person1Id, person2.id AS person2Id, length(path) AS shortestPathLength, path IS NULL AS noPath ORDER BY person1Id, person2Id """, @@ -1367,9 +1374,9 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)<-[:KNOWS*]-(person2)) + (person2:Person) WHERE person1.id IN ['p5'] AND person2.id IN ['p1', 'p2'] + OPTIONAL MATCH path = shortestPath((person1)<-[:KNOWS*]-(person2)) WITH person1.id AS person1Id, person2.id AS person2Id, CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS shortestPathLength RETURN person1Id, person2Id, shortestPathLength ORDER BY person1Id, person2Id @@ -1385,9 +1392,9 @@ def _mk_person_self_loop_graph() -> _CypherTestGraph: query=""" MATCH (person1:Person), - (person2:Person), - path = shortestPath((person1)-[:KNOWS*]-(person2)) + (person2:Person) WHERE person1.id IN ['p1', 'p2'] AND person2.id IN ['p3', 'p5'] + OPTIONAL MATCH path = shortestPath((person1)-[:KNOWS*]-(person2)) RETURN person1.id AS person1Id, person2.id AS person2Id, path IS NULL AS noPath ORDER BY noPath, person1Id, person2Id """, diff --git a/graphistry/tests/compute/gfql/row/test_row_pipeline_boundaries.py b/graphistry/tests/compute/gfql/row/test_row_pipeline_boundaries.py new file mode 100644 index 0000000000..0cd1ff721f --- /dev/null +++ b/graphistry/tests/compute/gfql/row/test_row_pipeline_boundaries.py @@ -0,0 +1,353 @@ +"""Both-sides pins for every boundary ``compute/gfql/row/pipeline.py`` decides. + +Each boundary gets the case just INSIDE it and the case just OUTSIDE it, so a +gate that drifts in either direction goes red: + +============================== ================================= ========================== +boundary inside outside +============================== ================================= ========================== +trail tracking engages plain var-length MATCH ``shortestPath`` (BFS) +undirected flip-twin dedupe a self-loop is ONE binding a non-loop keeps BOTH +same-edge return trip parallel edge -> legal the same edge -> illegal +walk scratch columns live inside the pipeline never reach a result +arithmetic null mask computed NaN compares by IEEE a genuine null stays null +polars var-length gate served with pandas parity typed decline, named +============================== ================================= ========================== + +Every expectation is hand-computed from the fixture edge list under openCypher +trail semantics (a relationship binds at most once per path; nodes may repeat). +No expectation was produced by running the code, and no engine is used as +another engine's oracle -- pandas, polars and cuDF are each checked against the +same hand-written literal. + +A polars decline is never a silent pass: ``POLARS_DECLINES`` says which shapes +polars is allowed to decline and which words the message must contain, and +:func:`_assert_value_or_named_decline` fails a shape that declines off-table AND +a tabled shape that quietly starts serving. +""" +import os +from typing import Dict, Tuple + +import pandas as pd +import pytest + +import graphistry +from graphistry.compute.gfql.identifiers import ( + TRAIL_COLUMN_PREFIX, + WALK_CURRENT_COL, + WALK_SCRATCH_COLUMNS, + is_trail_column, + is_walk_scratch_column, +) + +try: + import polars as pl + HAS_POLARS = True +except ImportError: + HAS_POLARS = False + +polars_only = pytest.mark.skipif(not HAS_POLARS, reason="polars not installed") +cudf_only = pytest.mark.skipif( + "TEST_CUDF" not in os.environ, reason="cuDF lane: set TEST_CUDF=1 (e.g. dgx-spark)" +) + +ENGINES = [ + "pandas", + pytest.param("polars", marks=polars_only), + pytest.param("cudf", marks=cudf_only), +] + +# --- fixtures (the edge list IS the oracle's input; ``e`` names the i-th row) --- + +#: e1 a->b1, e2 a->b2, e3 b1->c, e4 b2->c, e5 c->d +DIAMOND = ( + pd.DataFrame({"id": ["a", "b1", "b2", "c", "d"]}), + pd.DataFrame({ + "s": ["a", "a", "b1", "b2", "c"], + "d": ["b1", "b2", "c", "c", "d"], + "type": ["KNOWS"] * 5, + }), +) + +#: e1 a->b only, so the sole undirected 2-hop candidate has to reuse e1. +SINGLE = ( + pd.DataFrame({"id": ["a", "b"]}), + pd.DataFrame({"s": ["a"], "d": ["b"], "type": ["KNOWS"]}), +) + +#: e1 a->b, e2 a->b -- a return trip over the OTHER parallel edge is a legal trail. +PARA2 = ( + pd.DataFrame({"id": ["a", "b"]}), + pd.DataFrame({"s": ["a", "a"], "d": ["b", "b"], "type": ["KNOWS"] * 2}), +) + +#: e1 0->0 (self-loop) and e2 0->1, so both sides of the flip-twin rule show at once. +LOOP_AND_EDGE = ( + pd.DataFrame({"id": [0, 1]}), + pd.DataFrame({"s": [0, 0], "d": [0, 1], "type": ["REL"] * 2}), +) + +#: p1->p2->p3->p4 plus a disconnected q1. +LINE = ( + pd.DataFrame({"id": ["p1", "p2", "p3", "p4", "q1"]}), + pd.DataFrame({"s": ["p1", "p2", "p3"], "d": ["p2", "p3", "p4"], "type": ["KNOWS"] * 3}), +) + +#: f = 5.0 / NULL / 0.5 -- the three arithmetic-null-mask cases in one table. +NUMS = ( + pd.DataFrame({"id": ["n1", "n2", "n3"], "f": [5.0, None, 0.5]}), + pd.DataFrame({"s": ["n1"], "d": ["n3"], "type": ["KNOWS"]}), +) + +_VARLEN_GAP = "polars chain engine supports single-hop and multi-hop edges" +_ROWS_GAP = "does not yet natively support cypher row op" + +#: query -> the words polars' decline MUST contain. Absent = polars must serve it. +POLARS_DECLINES: Dict[str, Tuple[str, ...]] = { + "MATCH (x {id:'a'})-[*2]-(y) RETURN y.id AS y": (_VARLEN_GAP, "undirected min_hops>1"), + "MATCH (x)-[*1..2]->(m)-[]->(y) RETURN x.id AS a, y.id AS b": (_VARLEN_GAP,), + "MATCH (x {id:'a'})-[*1..2]->(m)-[]->(y) RETURN y.id AS y": (_VARLEN_GAP,), + "MATCH (a {id:'p1'}), (b {id:'p4'}), p = shortestPath((a)-[*]-(b)) RETURN length(p) AS y": + (_ROWS_GAP, "engine='pandas'"), + "MATCH (x)-[*2..3]->(y) RETURN y.id AS y": ("#1748", "hop-gated"), + "MATCH (x {id:'a'})-[*2..3]->(y) RETURN y.id AS y": ("#1748", "hop-gated"), +} + + +def _graph(fixture, engine): + nodes, edges = fixture + if engine == "polars": + return graphistry.nodes(pl.from_pandas(nodes), "id").edges(pl.from_pandas(edges), "s", "d") + if engine == "cudf": + import cudf + return graphistry.nodes(cudf.from_pandas(nodes), "id").edges(cudf.from_pandas(edges), "s", "d") + return graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + +def _rows(fixture, query, engine) -> pd.DataFrame: + out = _graph(fixture, engine).gfql(query, engine=engine)._nodes + if hasattr(out, "to_pandas"): + out = out.to_pandas() + return out.reset_index(drop=True) + + +def _assert_value_or_named_decline(fixture, query, engine, check): + """Run ``check`` on the result, or -- only for a tabled polars shape -- require + a decline whose message NAMES the gap. Never a silent skip in either + direction: an off-table decline fails, and a tabled shape that starts + serving fails so the table cannot rot.""" + expected_tokens = POLARS_DECLINES.get(query) if engine == "polars" else None + try: + got = _rows(fixture, query, engine) + except NotImplementedError as exc: + assert expected_tokens is not None, f"unexpected {engine} decline for {query}: {exc}" + missing = [tok for tok in expected_tokens if tok not in str(exc)] + assert not missing, f"decline did not name {missing}: {exc}" + return + assert expected_tokens is None, f"{engine} now serves a tabled decline; update the table: {query}" + check(got) + + +def _bag_check(expected, col="y"): + def check(got): + assert sorted(str(v) for v in got[col]) == expected + return check + + +# =========================================================================== +# Boundary 1: trail tracking engages on a plain MATCH, NOT in shortestPath mode +# =========================================================================== + + +@pytest.mark.parametrize("engine", ENGINES) +def test_plain_varlen_binds_each_relationship_at_most_once(engine): + """INSIDE. PARA2 ``-[*2]-`` from a: hop 1 reaches b over e1 and over e2; hop 2 + may only leave b over the edge that branch has NOT bound, so each returns to + a. Bag is [a, a] -- walk semantics would also emit b (a->b->a->b) plus the + two same-edge returns.""" + _assert_value_or_named_decline( + PARA2, "MATCH (x {id:'a'})-[*2]-(y) RETURN y.id AS y", engine, _bag_check(["a", "a"])) + + +@pytest.mark.parametrize("engine", ENGINES) +def test_plain_varlen_same_edge_return_trip_is_not_a_trail(engine): + """OUTSIDE the parallel-edge case, same query shape. SINGLE ``-[*2]-`` from a: + the only 2-hop candidate a->b->a must reuse e1, so NO row survives.""" + _assert_value_or_named_decline( + SINGLE, "MATCH (x {id:'a'})-[*2]-(y) RETURN y.id AS y", engine, _bag_check([])) + + +@pytest.mark.parametrize("engine", ENGINES) +def test_shortest_path_mode_keeps_bfs_length_without_trail_tracking(engine): + """OUTSIDE. shortestPath does not take the trail lane; LINE p1..p4 is 3 hops.""" + _assert_value_or_named_decline( + LINE, + "MATCH (a {id:'p1'}), (b {id:'p4'}), p = shortestPath((a)-[*]-(b)) RETURN length(p) AS y", + engine, + lambda got: [int(v) for v in got["y"]] == [3], + ) + + +# =========================================================================== +# Boundary 2: the undirected flip-twin dedupe is a SELF-LOOP rule +# =========================================================================== + + +@pytest.mark.parametrize("engine", ENGINES) +def test_undirected_self_loop_is_one_binding_and_non_loop_keeps_both(engine): + """BOTH SIDES in one fixture. LOOP_AND_EDGE under ``(a)-[r]-(b)``: e1 (0->0) + yields ONE binding because its two orientations agree on (from, to, edge); + e2 (0->1) yields TWO, (0,1) and (1,0). Three rows -- not two, not four.""" + def check(got): + pairs = sorted((str(r["y"]), str(r["z"])) for r in got.to_dict("records")) + assert pairs == [("0", "0"), ("0", "1"), ("1", "0")] + _assert_value_or_named_decline( + LOOP_AND_EDGE, "MATCH (a)-[r]-(b) RETURN a.id AS y, b.id AS z", engine, check) + + +# =========================================================================== +# Boundary 3: walk scratch columns live INSIDE the pipeline, never in a result +# =========================================================================== + + +@pytest.mark.parametrize("engine", ENGINES) +def test_multi_element_varlen_result_carries_no_walk_scratch_column(engine): + """INSIDE the pipeline the pattern binds a ``__gfql_trail_*`` column per hop; + OUTSIDE (the returned frame) the columns are exactly the projected aliases. + + DIAMOND ``(x)-[*1..2]->(m)-[]->(y)`` enumerated over distinct-edge paths: + (a,b1|e1)+e3, (a,b2|e2)+e4, (b1,c|e3)+e5, (b2,c|e4)+e5, (a,c|e1,e3)+e5, + (a,c|e2,e4)+e5. The (c,d|e5) prefix dies because d has no out-edge.""" + def check(got): + assert sorted((str(r["a"]), str(r["b"])) for r in got.to_dict("records")) == [ + ("a", "c"), ("a", "c"), ("a", "d"), ("a", "d"), ("b1", "d"), ("b2", "d"), + ] + assert list(got.columns) == ["a", "b"] + _assert_value_or_named_decline( + DIAMOND, "MATCH (x)-[*1..2]->(m)-[]->(y) RETURN x.id AS a, y.id AS b", engine, check) + + +def test_bindings_state_returns_no_trail_column_but_keeps_the_walk_cursor(): + """The RETURN projection would hide a leak, so pin the boundary where it is + decided. ``_gfql_connected_bindings_state`` OWNS the drop: the per-hop + ``__gfql_trail_*`` columns are consumed there and must not escape, while + ``__current__`` is the cursor its caller still needs -- dropping everything + would break the join, dropping nothing leaks scratch into every downstream + op. Same six paths as the query pin above.""" + from graphistry.compute.ast import e_forward, n as node_op + from graphistry.compute.gfql.row.pipeline import _RowPipelineAdapter + + ops = [node_op(name="x"), e_forward(min_hops=1, max_hops=2), + node_op(name="m"), e_forward(), node_op(name="y")] + state, alias_frames = _RowPipelineAdapter(_graph(DIAMOND, "pandas"))._gfql_connected_bindings_state(ops) + + assert len(state) == 6 + assert [c for c in state.columns if is_trail_column(str(c))] == [] + assert WALK_CURRENT_COL in state.columns + assert sorted(alias_frames) == ["m", "x", "y"] + + +def test_walk_scratch_vocabulary_is_closed_over_its_predicate(): + """The externed vocabulary and its predicate cannot drift apart.""" + assert all(is_walk_scratch_column(col) for col in WALK_SCRATCH_COLUMNS) + assert is_walk_scratch_column(f"{TRAIL_COLUMN_PREFIX}0__") + assert is_walk_scratch_column(f"{TRAIL_COLUMN_PREFIX}17__") + assert not is_walk_scratch_column(f"{TRAIL_COLUMN_PREFIX}x__") + assert not is_walk_scratch_column("__gfql_trail__") + assert not is_walk_scratch_column("from") + + +# =========================================================================== +# Boundary 4: hop depths mix in one result (the pad-before-concat alignment) +# =========================================================================== + + +@cudf_only +@pytest.mark.parametrize("query,expected", [ + ("MATCH (x {id:'a'})-[*1..2]->(m)-[]->(y) RETURN y.id AS y", ["c", "c", "d", "d"]), + ("MATCH (x {id:'a'})-[*2]->(m)-[]->(y) RETURN y.id AS y", ["d", "d"]), +], ids=["mixed_depth", "single_depth"]) +def test_cudf_range_varlen_keeps_rows_from_every_hop_depth(query, expected): + """cuDF-only on purpose: its concat aligns schemas strictly, so hop frames of + DIFFERENT trail width need an explicit pad or the shallow rows vanish -- + pandas and polars null-fill and cannot see the bug (their values for the + mixed-depth query are already pinned in test_path_trail_semantics.py). + + INSIDE (mixed depth) DIAMOND from a: hop-1 prefixes (a,b1|e1) and (a,b2|e2) + reach c,c; hop-2 prefixes (a,c|e1,e3) and (a,c|e2,e4) reach d,d. + OUTSIDE (single depth) ``*2`` gives one frame width, so d,d with no pad.""" + got = _rows(DIAMOND, query, "cudf") + assert sorted(str(v) for v in got["y"]) == expected + + +# =========================================================================== +# Boundary 5: a computed NaN is a VALUE; a genuine null stays null +# =========================================================================== + + +@pytest.mark.parametrize("engine", ENGINES) +def test_computed_nan_comparison_is_false_not_null(engine): + """INSIDE the arithmetic-operand branch. ``n.f % 0.0`` is NaN for 5.0 and for + 0.5, and ``NaN > 1`` is FALSE, so ``NOT (...)`` is TRUE and both rows stay. + n2 (f IS NULL) propagates null through the arithmetic; ``NOT null`` is null + and WHERE drops it.""" + _assert_value_or_named_decline( + NUMS, "MATCH (n) WHERE NOT (n.f % 0.0 > 1) RETURN n.id AS y", engine, + _bag_check(["n1", "n3"])) + + +@pytest.mark.parametrize("engine", ENGINES) +def test_computed_nan_comparison_matches_nothing_in_positive_form(engine): + """The same expression un-negated: ``NaN > 1`` is false for n1/n3 and null for + n2, so NO row matches. Pins that the override makes the comparison FALSE + rather than merely non-null.""" + _assert_value_or_named_decline( + NUMS, "MATCH (n) WHERE n.f % 0.0 > 1 RETURN n.id AS y", engine, _bag_check([])) + + +@pytest.mark.parametrize("engine", ENGINES) +def test_computed_nan_override_applies_to_the_right_operand_too(engine): + """The override is per-operand, so the RIGHT side needs its own pin: with the + arithmetic moved across the comparison, ``1 < n.f % 0.0`` is still false for + n1/n3 (NaN) and null for n2, so ``NOT (...)`` keeps exactly n1 and n3.""" + _assert_value_or_named_decline( + NUMS, "MATCH (n) WHERE NOT (1 < n.f % 0.0) RETURN n.id AS y", engine, + _bag_check(["n1", "n3"])) + + +@pytest.mark.parametrize("engine", ENGINES) +@pytest.mark.parametrize("query,expected", [ + ("MATCH (n) WHERE NOT (n.f > 1) RETURN n.id AS y", ["n3"]), + ("MATCH (n) WHERE n.f > 1 RETURN n.id AS y", ["n1"]), +], ids=["negated", "plain"]) +def test_non_arithmetic_operand_keeps_plain_null_semantics(query, expected, engine): + """OUTSIDE the arithmetic branch. A bare property reference takes no override, + so n2's null drops under BOTH polarities. Green before this PR as well -- + kept as the fence that stops the override widening to non-arithmetic + operands, where it would turn null into false.""" + _assert_value_or_named_decline(NUMS, query, engine, _bag_check(expected)) + + +# =========================================================================== +# Boundary 6: the polars variable-length gate -- served vs TYPED decline +# =========================================================================== + + +@polars_only +def test_polars_serves_unseeded_min_one_varlen_with_pandas_parity(): + """INSIDE the gate. DIAMOND ``(x)-[*1..2]->(y)`` over all starts: length 1 + gives b1,b2,c,c,d and length 2 gives c,c (via b1/b2) and d,d (b1->c->d and + b2->c->d) -- nine rows, c four times and d three times.""" + expected = ["b1", "b2", "c", "c", "c", "c", "d", "d", "d"] + query = "MATCH (x)-[*1..2]->(y) RETURN y.id AS y" + _assert_value_or_named_decline(DIAMOND, query, "polars", _bag_check(expected)) + _assert_value_or_named_decline(DIAMOND, query, "pandas", _bag_check(expected)) + + +@polars_only +@pytest.mark.parametrize("query", sorted(q for q in POLARS_DECLINES if "#1748" in POLARS_DECLINES[q])) +def test_polars_declines_min_hops_above_one_by_name(query): + """OUTSIDE the gate. The decline must NAME the missing capability -- a bare + failure is indistinguishable from a crash, and a silent skip would make + every polars cell above vacuous.""" + _assert_value_or_named_decline(DIAMOND, query, "polars", _bag_check([])) diff --git a/graphistry/tests/compute/gfql/same_path/test_native_shortest_path.py b/graphistry/tests/compute/gfql/same_path/test_native_shortest_path.py index f0feb44f60..2ce17de5c8 100644 --- a/graphistry/tests/compute/gfql/same_path/test_native_shortest_path.py +++ b/graphistry/tests/compute/gfql/same_path/test_native_shortest_path.py @@ -24,6 +24,7 @@ try_native_shortest_path, ) from graphistry.Engine import Engine +from graphistry.compute.gfql.identifiers import WALK_FROM_COL, WALK_TO_COL from graphistry.tests.test_compute import CGFull igraph = pytest.importorskip("igraph", reason="igraph not installed") @@ -63,7 +64,7 @@ def _chain_graph() -> _TestGraph: # --------------------------------------------------------------------------- def _step_pairs(frm, to): - return pd.DataFrame({"__from__": frm, "__to__": to}) + return pd.DataFrame({WALK_FROM_COL: frm, WALK_TO_COL: to}) def _hops(result, src, tgt): @@ -264,9 +265,11 @@ def test_distinct_cache_keys_build_separate_graphs(self): # Integration: Cypher shortestPath via igraph backend # --------------------------------------------------------------------------- +# OPTIONAL MATCH spelling: plain-MATCH shortestPath drops unreachable rows +# (openCypher; #1903), so the -1 sentinel needs the null-extending clause. _SP_QUERY = ( - "MATCH (a:Person {id: $a}), (b:Person {id: $b}), " - "path = shortestPath((a)-[:KNOWS*]-(b)) " + "MATCH (a:Person {id: $a}), (b:Person {id: $b}) " + "OPTIONAL MATCH path = shortestPath((a)-[:KNOWS*]-(b)) " "RETURN CASE path IS NULL WHEN true THEN -1 ELSE length(path) END AS dist" ) diff --git a/graphistry/tests/compute/gfql/test_engine_polars_binding_rows.py b/graphistry/tests/compute/gfql/test_engine_polars_binding_rows.py index 5b2edfda2e..883429d615 100644 --- a/graphistry/tests/compute/gfql/test_engine_polars_binding_rows.py +++ b/graphistry/tests/compute/gfql/test_engine_polars_binding_rows.py @@ -278,27 +278,26 @@ def _undirected_chain_graph(): def test_polars_undirected_varlen_min1_backtrack_and_multiplicity_pins(): - """Undirected `-[*1..k]-` binding table (min_hops == 1) — pin the EXACT pandas - oracle semantics, not just parity, so a future drift on either engine is caught: + """Undirected `-[*1..k]-` binding table (min_hops == 1) — pin the openCypher + TRAIL oracle (#1903, matches the pandas twin exactly): - * immediate-backtrack avoidance (0->1->0 excluded, so `WHERE NOT a = b` on a chain - never re-reaches the start via a single edge), - * pandas' edge multiplicity (each non-loop edge contributes each directed - orientation TWICE), so a length-1 pair appears x2 and a length-2 pair appears x4. + * one row per distinct edge sequence (each undirected edge binds each + orientation ONCE — the old x2/x4 step_pairs doubling was a walk artifact), + * same-edge immediate backtrack (0-1-0 via one edge) is excluded because a + relationship binds at most once per path — not by a node heuristic. """ g = _undirected_chain_graph() - # length-1 pairs appear x2, length-2 pairs appear x4 (pandas step_pairs doubling). q = "MATCH (a)-[*1..2]-(b) WHERE NOT a = b RETURN a.id AS ai, b.id AS bi" rpl = g.gfql(q, engine="polars")._nodes.to_pandas() counts = rpl.groupby(["ai", "bi"]).size().to_dict() - # 1-hop neighbours (both orientations), each doubled - assert counts[(0, 1)] == 2 and counts[(1, 0)] == 2 - assert counts[(1, 2)] == 2 and counts[(2, 1)] == 2 - assert counts[(2, 3)] == 2 and counts[(3, 2)] == 2 - # 2-hop reaches (backtrack-free), each x4 - assert counts[(0, 2)] == 4 and counts[(2, 0)] == 4 - assert counts[(1, 3)] == 4 and counts[(3, 1)] == 4 - # backtrack pairs (a==b via 0->1->0) are excluded -> no (0,0)/(1,1)/... rows here + # 1-hop neighbours, once per orientation + assert counts[(0, 1)] == 1 and counts[(1, 0)] == 1 + assert counts[(1, 2)] == 1 and counts[(2, 1)] == 1 + assert counts[(2, 3)] == 1 and counts[(3, 2)] == 1 + # 2-hop reaches (distinct edges), once each + assert counts[(0, 2)] == 1 and counts[(2, 0)] == 1 + assert counts[(1, 3)] == 1 and counts[(3, 1)] == 1 + # same-edge backtrack pairs (a==b via one edge) are excluded assert (0, 0) not in counts and (1, 1) not in counts # and it exactly matches the pandas oracle _assert_parity(q) @@ -543,37 +542,27 @@ def test_polars_unbounded_varlen_path_multiplicity_matches_pandas(): ) -def test_polars_unbounded_varlen_cycle_raises_same_error_as_pandas(): - """A cycle reachable from the seed means infinitely many paths. Pandas raises - E108 ('require terminating variable-length segments'); polars must raise the - SAME diagnosis, never truncate to a subtly different answer. - - Asserts the CLASS and `.code` are engine-INDEPENDENT, not just that polars carries - some code of its own. An earlier version pinned `errs['polars'].code == E108` and so - passed while the two engines actually disagreed — pandas re-wraps the kernel error as - GFQLTypeError(E303) via `execute_call`, polars ran the native kernel before that - wrapper and leaked the raw GFQLValidationError(E108). Callers switch on `.code`, so - a per-engine code is a real divergence; the E108 text survives inside the message.""" +def test_polars_unbounded_varlen_cycle_pandas_serves_polars_declines(): + """A cycle reachable from the seed no longer means infinitely many TRAILS: + each relationship binds once per path (#1903), so pandas now SERVES the + unbounded walk on a 3-cycle -- 3 trails per seed (e.g. from 0: e0; e0e1; + e0e1e2), 9 total. polars' node-frontier depth probe cannot see trail + exhaustion yet, so it still raises the terminating-segments error -- the + pinned #1903 residual (parity-or-error, never a different number).""" from graphistry.compute.exceptions import GFQLValidationError nodes = pd.DataFrame({"id": [0, 1, 2]}) edges = pd.DataFrame({"s": [0, 1, 2], "d": [1, 2, 0], "type": ["R"] * 3}) # 3-cycle g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") q = "MATCH (a)-[*]->(b) RETURN count(*) AS c" - errs = {} - for engine in ("pandas", "polars"): - with pytest.raises(GFQLValidationError) as exc: - g.gfql(q, engine=engine) - errs[engine] = exc.value - assert type(errs["polars"]) is type(errs["pandas"]), ( - f"exception class differs by engine: {type(errs['polars'])} vs {type(errs['pandas'])}") - assert errs["polars"].code == errs["pandas"].code, ( - f"error code differs by engine: {errs['polars'].code} vs {errs['pandas'].code}") - for engine in ("pandas", "polars"): - assert ( - "Cypher multi-alias row bindings currently require terminating " - "variable-length segments" - ) in str(errs[engine]) + served = g.gfql(q, engine="pandas")._nodes + assert int(served["c"].iloc[0]) == 9 # hand-computed trail oracle + with pytest.raises(GFQLValidationError) as exc: + g.gfql(q, engine="polars") + assert ( + "Cypher multi-alias row bindings currently require terminating " + "variable-length segments" + ) in str(exc.value) def test_polars_unbounded_varlen_cycle_bound_is_the_reachable_set_not_the_graph(): diff --git a/graphistry/tests/compute/gfql/test_engine_polars_row_pipeline.py b/graphistry/tests/compute/gfql/test_engine_polars_row_pipeline.py index da51c9ccb7..1c5e94af3c 100644 --- a/graphistry/tests/compute/gfql/test_engine_polars_row_pipeline.py +++ b/graphistry/tests/compute/gfql/test_engine_polars_row_pipeline.py @@ -658,7 +658,8 @@ def test_group_by_polars_malformed_agg_declines_not_crashes(): def test_polars_rows_binding_ops_undirected_self_loop_multiplicity(): - """Raw undirected bindings preserve both orientations, including two self-loop paths.""" + """Raw undirected bindings preserve both orientations; a self-loop's flip + twin is the SAME binding and appears once (#1903 addendum A-1).""" from graphistry.compute.ast import e_undirected, n, rows, serialize_binding_ops g = graphistry.nodes(pd.DataFrame({"id": [0, 1]}), "id").edges( @@ -667,7 +668,7 @@ def test_polars_rows_binding_ops_undirected_self_loop_multiplicity(): binding_ops = serialize_binding_ops([n(name="a"), e_undirected(), n(name="b")]) out = g.gfql([rows(binding_ops=binding_ops)], engine="polars")._nodes - assert sorted(out.select(["a", "b"]).rows()) == [(0, 1), (1, 0), (1, 1), (1, 1)] + assert sorted(out.select(["a", "b"]).rows()) == [(0, 1), (1, 0), (1, 1)] def test_run_calls_polars_binding_ops_native(): diff --git a/graphistry/tests/compute/gfql/test_path_trail_semantics.py b/graphistry/tests/compute/gfql/test_path_trail_semantics.py new file mode 100644 index 0000000000..6bf74b4b87 --- /dev/null +++ b/graphistry/tests/compute/gfql/test_path_trail_semantics.py @@ -0,0 +1,362 @@ +"""Round-005 var-length path TRAIL semantics + lane-consistency pins (#1903). + +openCypher trail semantics, hand-computed per fixture: each distinct edge +SEQUENCE is one row; a relationship binds at most once per path (edges never +repeat, nodes may); one MATCH has ONE cardinality regardless of what the +RETURN projects; a plain-MATCH shortestPath with no path emits NO row (only +OPTIONAL MATCH null-extends). + +polars is parity-or-NIE: it matches the pandas oracle or declines honestly. +Fixtures: DIAMOND a->b1->c, a->b2->c, c->d; TRI directed 3-cycle; SELF +self-loop s->s plus s->t; PARA parallel a->b x2 plus b->c; LINE p1->p2->p3->p4 +with disconnected q1. +""" +import pandas as pd +import pytest + +import graphistry +from graphistry.compute.exceptions import GFQLValidationError + +try: + import polars as pl + HAS_POLARS = True +except ImportError: + HAS_POLARS = False + +polars_only = pytest.mark.skipif(not HAS_POLARS, reason="polars not installed") + +ENGINES = ["pandas", pytest.param("polars", marks=polars_only)] + +FIXTURES = { + "DIAMOND": ( + pd.DataFrame({"id": ["a", "b1", "b2", "c", "d"]}), + pd.DataFrame({ + "s": ["a", "a", "b1", "b2", "c"], + "d": ["b1", "b2", "c", "c", "d"], + "type": ["KNOWS"] * 5, + }), + ), + "TRI": ( + pd.DataFrame({"id": ["a", "b", "c"]}), + pd.DataFrame({"s": ["a", "b", "c"], "d": ["b", "c", "a"], "type": ["KNOWS"] * 3}), + ), + "SELF": ( + pd.DataFrame({"id": ["s", "t"]}), + pd.DataFrame({"s": ["s", "s"], "d": ["s", "t"], "type": ["KNOWS"] * 2}), + ), + "PARA": ( + pd.DataFrame({"id": ["a", "b", "c"]}), + pd.DataFrame({"s": ["a", "a", "b"], "d": ["b", "b", "c"], "type": ["KNOWS"] * 3}), + ), + "LINE": ( + pd.DataFrame({"id": ["p1", "p2", "p3", "p4", "q1"]}), + pd.DataFrame({"s": ["p1", "p2", "p3"], "d": ["p2", "p3", "p4"], "type": ["KNOWS"] * 3}), + ), +} + + +def _run(fixture: str, query: str, engine: str) -> pd.DataFrame: + nodes, edges = FIXTURES[fixture] + if engine == "polars": + g = graphistry.nodes(pl.from_pandas(nodes), "id").edges(pl.from_pandas(edges), "s", "d") + else: + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + out = g.gfql(query, engine=engine)._nodes + if hasattr(out, "to_pandas"): + out = out.to_pandas() + return out.reset_index(drop=True) + + +#: A polars decline must NAME the gap; these are the capabilities it may cite. +DECLINE_PHRASES = ( + "not yet hop-gated", # #1748 min_hops>1 node-alias window + "undirected min_hops>1", # var-length feature gate + "require terminating variable-length segments", # unbounded walk into a reachable cycle + "does not yet natively support cypher row op", # row-op surface gap +) + +#: Every (fixture, query) polars is currently allowed to decline -- keyed on the +#: pair because the same shape routes differently per graph (``-[*]->`` serves on +#: the acyclic DIAMOND and declines on the cyclic TRI). Membership is asserted in +#: BOTH directions, so a shape that starts declining -- or quietly starts serving +#: -- fails instead of passing silently. Without the table every polars cell here +#: is vacuous: 14 of the 28 (fixture, query) pairs below decline today. +POLARS_DECLINED = frozenset({ + ("DIAMOND", "MATCH (x {id:'a'})-[*1..2]->(m)-[]->(y) RETURN y.id AS y"), + ("DIAMOND", "MATCH (x {id:'a'})-[*2]-(y) RETURN y.id AS y"), + ("DIAMOND", "MATCH (x {id:'a'})-[*2]->(y {id:'c'}) RETURN count(*) AS y"), + ("DIAMOND", "MATCH (x {id:'a'})-[*2]->(y) RETURN x.id AS x, y.id AS y"), + ("DIAMOND", "MATCH (x {id:'a'})-[*2]->(y) RETURN y.id AS y"), + ("DIAMOND", "MATCH (x {id:'a'})-[*3]->(y) RETURN y.id AS y"), + ("DIAMOND", "MATCH (x {id:'c'})<-[*2]-(y) RETURN y.id AS y"), + ("PARA", "MATCH (x {id:'a'})-[*2]-(y) RETURN y.id AS y"), + ("PARA", "MATCH (x {id:'a'})-[*2]->(y) RETURN y.id AS y"), + ("SELF", "MATCH (x {id:'s'})-[*2]->(y) RETURN y.id AS y"), + ("TRI", "MATCH (x {id:'a'})-[*2]-(y) RETURN y.id AS y"), + ("TRI", "MATCH (x {id:'a'})-[*3]-(y) RETURN y.id AS y"), + ("TRI", "MATCH (x {id:'a'})-[*3]->(y) RETURN y.id AS y"), + ("TRI", "MATCH (x {id:'a'})-[*]->(y) RETURN y.id AS y"), +}) + + +def _bag(fixture: str, query: str, engine: str, col: str = "y"): + """Sorted value bag, or ``('DECLINED', message)`` when the engine declines.""" + try: + df = _run(fixture, query, engine) + except NotImplementedError as exc: + return ("DECLINED", str(exc)) + except GFQLValidationError as exc: + return ("DECLINED", str(exc)) + return sorted(str(v) for v in df[col]) + + +def _is_decline(got) -> bool: + return isinstance(got, tuple) and bool(got) and got[0] == "DECLINED" + + +def _assert_polars_routing(fixture, query, got) -> None: + """A decline passes only if it was TABLED and it SAYS what is missing.""" + assert _is_decline(got) == ((fixture, query) in POLARS_DECLINED), ( + f"polars routing changed for ({fixture}, {query}): " + f"{'declined but not tabled' if _is_decline(got) else 'tabled as declined but served'}" + ) + if _is_decline(got): + assert any(phrase in got[1] for phrase in DECLINE_PHRASES), \ + f"decline did not name the gap: {got[1]}" + + +def _assert_bag(fixture, query, engine, expected, col="y"): + got = _bag(fixture, query, engine, col) + if engine == "polars": + _assert_polars_routing(fixture, query, got) + if _is_decline(got): + return + assert got == sorted(str(v) for v in expected), f"{query}: got {got}" + + +# =========================================================================== +# 1+2. Lane consistency: single-alias endpoint projections match the row lane +# =========================================================================== + + +@pytest.mark.parametrize("engine", ENGINES) +@pytest.mark.parametrize("fixture,query,expected", [ + ("DIAMOND", "MATCH (x {id:'a'})-[*2]->(y) RETURN y.id AS y", ["c", "c"]), + ("DIAMOND", "MATCH (x {id:'a'})-[*1..2]->(y) RETURN y.id AS y", ["b1", "b2", "c", "c"]), + ("DIAMOND", "MATCH (x {id:'a'})-[*]->(y) RETURN y.id AS y", ["b1", "b2", "c", "c", "d", "d"]), + ("DIAMOND", "MATCH (x {id:'c'})<-[*2]-(y) RETURN y.id AS y", ["a", "a"]), + ("DIAMOND", "MATCH (x {id:'a'})-[*2]-(y) RETURN y.id AS y", ["c", "c"]), + ("DIAMOND", "MATCH (x {id:'a'})-[*0..2]->(y) RETURN y.id AS y", ["a", "b1", "b2", "c", "c"]), + ("PARA", "MATCH (x {id:'a'})-[*1]->(y) RETURN y.id AS y", ["b", "b"]), + ("PARA", "MATCH (x {id:'a'})-[*2]->(y) RETURN y.id AS y", ["c", "c"]), + ("TRI", "MATCH (x {id:'a'})-[*2]-(y) RETURN y.id AS y", ["b", "c"]), +], ids=["d_exact2", "d_range", "d_unbounded", "d_reverse", "d_undirected", + "d_zero_hop", "p_parallel1", "p_parallel2", "t_undirected_bfs_prune"]) +def test_single_alias_projection_keeps_path_multiplicity(fixture, query, expected, engine): + """#1903 items 1-2: the single-alias endpoint projection now rides the + binding-row lane -- path multiplicity preserved (D01 gave 1 row, oracle 2), + BFS visited-pruning gone (TRI undirected [*2] gave [], oracle [b,c]).""" + _assert_bag(fixture, query, engine, expected) + + +@pytest.mark.parametrize("engine", ENGINES) +def test_one_match_one_cardinality(engine): + """The headline invariant: RETURN y.id, count(*), and RETURN x.id, y.id + agree on cardinality for the same MATCH.""" + queries = ["MATCH (x {id:'a'})-[*2]->(y) RETURN y.id AS y", + "MATCH (x {id:'a'})-[*2]->(y {id:'c'}) RETURN count(*) AS y", + "MATCH (x {id:'a'})-[*2]->(y) RETURN x.id AS x, y.id AS y"] + bag, cnt, pair = [_bag("DIAMOND", q, engine) for q in queries] + if engine == "polars": + for query, got in zip(queries, (bag, cnt, pair)): + _assert_polars_routing("DIAMOND", query, got) + if any(_is_decline(got) for got in (bag, cnt, pair)): + return + assert bag == ["c", "c"] and cnt == ["2"] and pair == ["c", "c"] + + +# =========================================================================== +# 3-5. Trail semantics: edges never repeat within a path +# =========================================================================== + + +@pytest.mark.parametrize("engine", ENGINES) +@pytest.mark.parametrize("fixture,query,expected", [ + ("SELF", "MATCH (x {id:'s'})-[*2]->(y) RETURN y.id AS y", ["t"]), + ("SELF", "MATCH (x {id:'s'})-[*1..2]->(y) RETURN y.id AS y", ["s", "t", "t"]), + ("SELF", "MATCH (x {id:'s'})-[*1]-(y) RETURN y.id AS y", ["s", "t"]), + ("TRI", "MATCH (x {id:'a'})-[*1..4]->(y) RETURN y.id AS y", ["a", "b", "c"]), + ("TRI", "MATCH (x {id:'a'})-[*]->(y) RETURN y.id AS y", ["a", "b", "c"]), + ("TRI", "MATCH (x {id:'a'})-[]->(m)-[]->(y) RETURN y.id AS y", ["c"]), + ("PARA", "MATCH (x {id:'a'})-[]->(m)-[]-(y) RETURN y.id AS y", ["a", "a", "c", "c"]), + ("PARA", "MATCH (x {id:'a'})-[*2]-(y) RETURN y.id AS y", ["a", "a", "c", "c"]), + ("TRI", "MATCH (x {id:'a'})-[*3]-(y) RETURN y.id AS y", ["a", "a"]), + ("DIAMOND", "MATCH (x {id:'a'})-[*1..2]->(m)-[]->(y) RETURN y.id AS y", ["c", "c", "d", "d"]), +], ids=["selfloop_no_reuse", "selfloop_range", "selfloop_undirected_once", + "cycle_bounded_terminates", "cycle_unbounded_terminates", + "fixed_2hop_cross_unique", "parallel_return_trip_legal", + "undirected_parallel_backtrack", "undirected_cycle_closes", + "midpattern_varlen_then_hop"]) +def test_trail_relationship_uniqueness(fixture, query, expected, engine): + """#1903 items 3-5: an edge binds at most once per path (self-loop walk + fabrication gone; cross-element reuse gone; a return trip over a PARALLEL + edge stays legal; nodes may repeat).""" + _assert_bag(fixture, query, engine, expected) + + +@pytest.mark.parametrize("engine", ENGINES) +def test_undirected_selfloop_single_hop_binds_once(engine): + """Addendum A-1: both orientations of one self-loop are the SAME binding -- + one row, count 1 (was 2 on both engines).""" + nodes = pd.DataFrame({"id": [0, 1]}) + edges = pd.DataFrame({"s": [0], "d": [0], "type": ["REL"]}) + if engine == "polars": + g = graphistry.nodes(pl.from_pandas(nodes), "id").edges(pl.from_pandas(edges), "s", "d") + else: + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + try: + rows = g.gfql("MATCH (a)-[r]-(b) RETURN a.id AS y", engine=engine)._nodes + cnt = g.gfql("MATCH (a)-[r]-(b) RETURN count(*) AS y", engine=engine)._nodes + except NotImplementedError: + assert engine == "polars" + return + rows = rows.to_pandas() if hasattr(rows, "to_pandas") else rows + cnt = cnt.to_pandas() if hasattr(cnt, "to_pandas") else cnt + assert len(rows) == 1 and int(cnt["y"][0]) == 1 + + +# =========================================================================== +# 6. shortestPath: plain MATCH unreachable -> NO row; OPTIONAL null-extends +# =========================================================================== + + +@pytest.mark.parametrize("engine", ENGINES) +def test_shortest_path_plain_match_unreachable_no_row(engine): + q = ("MATCH (a {id:'p1'}), (b {id:'q1'}), p = shortestPath((a)-[*]-(b)) " + "RETURN length(p) AS y") + try: + df = _run("LINE", q, engine) + except NotImplementedError: + assert engine == "polars" + return + assert len(df) == 0 + + +@pytest.mark.parametrize("engine", ENGINES) +def test_shortest_path_bound_below_actual_no_row(engine): + q = ("MATCH (a {id:'p1'}), (b {id:'p4'}), p = shortestPath((a)-[*..2]-(b)) " + "RETURN length(p) AS y") + try: + df = _run("LINE", q, engine) + except NotImplementedError: + assert engine == "polars" + return + assert len(df) == 0 + + +@pytest.mark.parametrize("engine", ENGINES) +def test_shortest_path_optional_unreachable_null_row(engine): + q = ("MATCH (a {id:'p1'}), (b {id:'q1'}) " + "OPTIONAL MATCH p = shortestPath((a)-[*]-(b)) RETURN length(p) AS y") + try: + df = _run("LINE", q, engine) + except NotImplementedError: + assert engine == "polars" + return + assert len(df) == 1 and pd.isna(df["y"][0]) + + +# =========================================================================== +# 8. Grammar: [*..M] omitted lower bound (defaults to 1) +# =========================================================================== + + +@pytest.mark.parametrize("engine", ENGINES) +def test_open_min_bound_parses_and_serves(engine): + _assert_bag("DIAMOND", "MATCH (x {id:'a'})-[*..3]->(y) RETURN y.id AS y", + engine, ["b1", "b2", "c", "c", "d", "d"]) + + +@pytest.mark.parametrize("engine", ENGINES) +def test_open_min_bound_shortest_path(engine): + q = ("MATCH (a {id:'p1'}), (b {id:'p4'}), p = shortestPath((a)-[*..3]-(b)) " + "RETURN length(p) AS y") + try: + df = _run("LINE", q, engine) + except NotImplementedError: + assert engine == "polars" + return + assert [int(v) for v in df["y"]] == [3] + + +# =========================================================================== +# Correct-inventory regression fences (were green pre-#1903; must stay green) +# =========================================================================== + + +@pytest.mark.parametrize("engine", ENGINES) +@pytest.mark.parametrize("fixture,query,expected", [ + ("TRI", "MATCH (x {id:'a'})-[*3]->(y) RETURN y.id AS y", ["a"]), + ("DIAMOND", "MATCH (x {id:'a'})-[*3]->(y) RETURN y.id AS y", ["d", "d"]), + ("DIAMOND", "MATCH (x {id:'a'})-[*1]-(y) RETURN y.id AS y", ["b1", "b2"]), + ("LINE", "MATCH (x {id:'q1'})-[*1..3]-(y) RETURN y.id AS y", []), +], ids=["cycle_exact_closes", "diamond_exact3", "undirected_1_no_double", "component_isolation"]) +def test_correct_inventory_stays_green(fixture, query, expected, engine): + _assert_bag(fixture, query, engine, expected) + + +@pytest.mark.parametrize("engine", ENGINES) +def test_shortest_path_ties_and_typed_fences(engine): + for q, fixture, expected in [ + ("MATCH (a {id:'a'}), (b {id:'c'}), p = shortestPath((a)-[*]->(b)) RETURN length(p) AS y", "DIAMOND", [2]), + ("MATCH (a {id:'p1'}), (b {id:'p4'}), p = shortestPath((a)-[*]-(b)) RETURN length(p) AS y", "LINE", [3]), + ]: + try: + df = _run(fixture, q, engine) + except NotImplementedError: + assert engine == "polars" + continue + assert [int(v) for v in df["y"]] == expected + + +@pytest.mark.parametrize("engine", ENGINES) +def test_grouped_agg_lane_consistent(engine): + q = "MATCH (x {id:'a'})-[*1..3]->(y) RETURN y.id AS y, count(*) AS n" + try: + df = _run("DIAMOND", q, engine) + except NotImplementedError: + assert engine == "polars" + return + got = sorted((str(r["y"]), int(r["n"])) for r in df.to_dict("records")) + assert got == [("b1", 1), ("b2", 1), ("c", 2), ("d", 2)] + + +# =========================================================================== +# Residual pins (strict xfail, #1903) +# =========================================================================== + + +@pytest.mark.parametrize("engine", ENGINES) +@pytest.mark.xfail(strict=True, reason="#1903 addendum A-2 residual: the seeded typed-hop " + "lane (fast path AND its fallback) projects the destination NODE SET -- " + "parallel edges from a unique seed collapse ([1,2] vs bag [1,1,2])") +def test_seeded_parallel_edge_multiplicity_residual(engine): + nodes = pd.DataFrame({"id": [0, 1, 2], "kind": ["a", "b", "b"]}) + edges = pd.DataFrame({"s": [0, 0, 0], "d": [1, 1, 2], "type": ["KNOWS"] * 3}) + if engine == "polars": + g = graphistry.nodes(pl.from_pandas(nodes), "id").edges(pl.from_pandas(edges), "s", "d") + else: + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + df = g.gfql("MATCH (a {id:0})-[{type:'KNOWS'}]->(b) RETURN b.id AS y", engine=engine)._nodes + df = df.to_pandas() if hasattr(df, "to_pandas") else df + assert sorted(int(v) for v in df["y"]) == [1, 1, 2] + + +@pytest.mark.xfail(strict=True, reason="#1903 residual: polars unbounded fixed point on a " + "reachable cycle still raises the terminating-segments error (its " + "node-frontier probe cannot see trail exhaustion); pandas serves it") +@polars_only +def test_polars_unbounded_cycle_residual(): + _assert_bag("TRI", "MATCH (x {id:'a'})-[*]->(y) RETURN y.id AS y", "polars", ["a", "b", "c"]) + df = _run("TRI", "MATCH (x {id:'a'})-[*]->(y) RETURN y.id AS y", "polars") + assert sorted(df["y"]) == ["a", "b", "c"] diff --git a/graphistry/tests/compute/gfql/test_varlen_bounded_engine_parity_1787.py b/graphistry/tests/compute/gfql/test_varlen_bounded_engine_parity_1787.py index 0b25033303..1910a57d70 100644 --- a/graphistry/tests/compute/gfql/test_varlen_bounded_engine_parity_1787.py +++ b/graphistry/tests/compute/gfql/test_varlen_bounded_engine_parity_1787.py @@ -102,14 +102,18 @@ def _seeded(id_: str, query: str, oracle: int) -> Shape: # --- the contract, as data ----------------------------------------------------------------- #: Shapes the polars engines must DECLINE and the pandas-API engines must ANSWER correctly. +#: Oracle literals recomputed under openCypher TRAIL semantics (#1903, independent +#: brute-force enumerator): a relationship binds once per path, so the old walk +#: reconstruction's doubled undirected orientations and reused edges are gone. +#: (dir-min3-* keep 0: hop-window eccentricity pruning still under-reports the one +#: 3-trail on the pandas lane -- pre-existing, out of #1903's scope.) DECLINED_BY_POLARS: List[Shape] = [ _bounded("dir-min3-exact", "MATCH (a)-[*3..3]->(b) RETURN count(*) AS c", 0), _bounded("dir-min3-window", "MATCH (a)-[*3..4]->(b) RETURN count(*) AS c", 0), - _seeded("dir-min2-seeded", "MATCH (a {kind:'a'})-[*2..3]->(b) RETURN count(*) AS c", 32), - _undir("undir-degenerate-window", "MATCH (a)-[*1..1]-(b) RETURN count(*) AS c", 36), - _undir("undir-degenerate-exact", "MATCH (a)-[*1]-(b) RETURN count(*) AS c", 36), - _seeded("undir-seeded", "MATCH (a {kind:'a'})-[*1..2]-(b) RETURN count(*) AS c", 38), - _seeded("undir-non-first-segment", "MATCH (a)-[]->(b)-[*1..2]-(c) RETURN count(*) AS c", 316), + # seeded directed min>=2: pandas' per-seed hop window under-reports + # data-dependently (fuzz: 1 vs trail 2), so polars stays declined; the 28 + # here happens to be trail-exact on THIS fixture. + _seeded("dir-min2-seeded", "MATCH (a {kind:'a'})-[*2..3]->(b) RETURN count(*) AS c", 28), ] #: Neighbours of every declined shape, on BOTH sides of each gate boundary. These are what @@ -117,13 +121,18 @@ def _seeded(id_: str, query: str, oracle: int) -> Shape: #: length" would fail every one of them. SERVED_EVERYWHERE: List[Shape] = [ _bounded("dir-min1", "MATCH (a)-[*1..2]->(b) RETURN count(*) AS c", 12), + # migrated from DECLINED_BY_POLARS by the #1903 trail rework (gate shrink): + _undir("undir-degenerate-window", "MATCH (a)-[*1..1]-(b) RETURN count(*) AS c", 18), + _undir("undir-degenerate-exact", "MATCH (a)-[*1]-(b) RETURN count(*) AS c", 18), + _seeded("undir-seeded", "MATCH (a {kind:'a'})-[*1..2]-(b) RETURN count(*) AS c", 38), + _seeded("undir-non-first-segment", "MATCH (a)-[]->(b)-[*1..2]-(c) RETURN count(*) AS c", 134), _bounded("dir-min2-unseeded", "MATCH (a)-[*2..3]->(b) RETURN count(*) AS c", 6), _undir("undir-plain-edge", "MATCH (a)-[]-(b) RETURN count(*) AS c", 18), - _undir("undir-min1-max2", "MATCH (a)-[*1..2]-(b) RETURN count(*) AS c", 212), - _undir("undir-min1-max3", "MATCH (a)-[*1..3]-(b) RETURN count(*) AS c", 948), + _undir("undir-min1-max2", "MATCH (a)-[*1..2]-(b) RETURN count(*) AS c", 74), + _undir("undir-min1-max3", "MATCH (a)-[*1..3]-(b) RETURN count(*) AS c", 226), _undir("dir-degenerate-window", "MATCH (a)-[*1..1]->(b) RETURN count(*) AS c", 9), _seeded("dir-seeded-min1", "MATCH (a {kind:'a'})-[*1..2]->(b) RETURN count(*) AS c", 19), - _seeded("dir-non-first-segment", "MATCH (a)-[]->(b)-[*2..3]->(c) RETURN count(*) AS c", 80), + _seeded("dir-non-first-segment", "MATCH (a)-[]->(b)-[*2..3]->(c) RETURN count(*) AS c", 36), ] @@ -240,14 +249,12 @@ def test_degenerate_window_is_not_the_same_query_as_a_plain_edge(engine: str) -> plain = _count(_graph(engine, UNDIR_NODES, UNDIR_EDGES), "MATCH (a)-[]-(b) RETURN count(*) AS c", engine) assert plain == 18, "the plain undirected edge is served by every engine, unchanged" - if engine in POLARS_API_ENGINES: - with pytest.raises(NotImplementedError): - _count(_graph(engine, UNDIR_NODES, UNDIR_EDGES), - "MATCH (a)-[*1..1]-(b) RETURN count(*) AS c", engine) - else: - degenerate = _count(_graph(engine, UNDIR_NODES, UNDIR_EDGES), - "MATCH (a)-[*1..1]-(b) RETURN count(*) AS c", engine) - assert degenerate == 36 != plain + # openCypher trail semantics (#1903): a one-edge window IS the plain edge on + # EVERY engine now -- the old 36-vs-18 gap was the walk doubling this test + # was built to quarantine, and the polars decline that quarantined it is gone. + degenerate = _count(_graph(engine, UNDIR_NODES, UNDIR_EDGES), + "MATCH (a)-[*1..1]-(b) RETURN count(*) AS c", engine) + assert degenerate == 18 == plain @pytest.mark.parametrize("engine", PANDAS_API_ENGINES) @@ -276,7 +283,7 @@ def test_directed_min_hops_2_declines_only_when_the_segment_is_seeded(engine: st """ _require(engine) g = _graph(engine, SEEDED_NODES, SEEDED_EDGES) - assert _count(g, "MATCH (a)-[*2..3]->(b) RETURN count(*) AS c", engine) == 50 + assert _count(g, "MATCH (a)-[*2..3]->(b) RETURN count(*) AS c", engine) == 39 # trail oracle (#1903) with pytest.raises(NotImplementedError): _count(g, "MATCH (a {kind:'a'})-[*2..3]->(b) RETURN count(*) AS c", engine) @@ -291,8 +298,8 @@ def test_undirected_seed_declines_while_its_directed_twin_is_served(engine: str) _require(engine) g = _graph(engine, SEEDED_NODES, SEEDED_EDGES) assert _count(g, "MATCH (a {kind:'a'})-[*1..2]->(b) RETURN count(*) AS c", engine) == 19 - with pytest.raises(NotImplementedError): - _count(g, "MATCH (a {kind:'a'})-[*1..2]-(b) RETURN count(*) AS c", engine) + # #1903 gate shrink: seeded undirected serves with pandas parity (trail). + assert _count(g, "MATCH (a {kind:'a'})-[*1..2]-(b) RETURN count(*) AS c", engine) == 38 @pytest.mark.parametrize("engine", POLARS_API_ENGINES) @@ -302,9 +309,9 @@ def test_undirected_non_first_segment_declines_while_its_directed_twin_is_served """A non-first segment starts from less than the full node set, exactly like a seed.""" _require(engine) g = _graph(engine, SEEDED_NODES, SEEDED_EDGES) - assert _count(g, "MATCH (a)-[]->(b)-[*1..2]->(c) RETURN count(*) AS c", engine) == 50 - with pytest.raises(NotImplementedError): - _count(g, "MATCH (a)-[]->(b)-[*1..2]-(c) RETURN count(*) AS c", engine) + assert _count(g, "MATCH (a)-[]->(b)-[*1..2]->(c) RETURN count(*) AS c", engine) == 39 # trail oracle (#1903): the first edge cannot rebind + # #1903 gate shrink: the undirected non-first segment serves with parity. + assert _count(g, "MATCH (a)-[]->(b)-[*1..2]-(c) RETURN count(*) AS c", engine) == 134 @pytest.mark.parametrize("engine", POLARS_API_ENGINES) @@ -421,7 +428,7 @@ def test_seeded_undirected_degenerate_window_agrees_with_the_oracle(engine: str) g = _graph(engine, CUDF_DIVERGENCE_NODES, CUDF_DIVERGENCE_EDGES) query = "MATCH (a {kind:'a'})-[*1..1]-(b) RETURN count(*) AS c" if engine in POLARS_API_ENGINES: - with pytest.raises(NotImplementedError): - _count(g, query, engine) + # #1903 gate shrink: polars serves this shape now, trail-exact. + assert _count(g, query, engine) == 5 return - assert _count(g, query, engine) == 9 + assert _count(g, query, engine) == 5 # trail oracle (#1903): self-loops bind once diff --git a/graphistry/tests/test_compute_chain.py b/graphistry/tests/test_compute_chain.py index 5caaa6bffe..fed9d84df4 100644 --- a/graphistry/tests/test_compute_chain.py +++ b/graphistry/tests/test_compute_chain.py @@ -1815,8 +1815,11 @@ def test_native_chain_rows_bindings_reverse_edge(self): assert df["dst.id"].iloc[0] == "b" assert df["src.id"].iloc[0] == "a" - def test_native_chain_rows_select_undirected_self_loop_duplicates_both_directions(self): - """Undirected self-loops should surface both orientations in bindings rows.""" + def test_native_chain_rows_select_undirected_self_loop_binds_once(self): + """Undirected self-loops bind ONCE: both orientations of a self-loop edge + produce the identical (seed, r, friend) assignment, and one assignment is + one binding row (openCypher; #1903 A-1 — the old two-row expectation + characterized the orientation double-count bug).""" g = self._mk_graph( pd.DataFrame({"id": ["a"], "label__Person": [True], "firstName": ["Alice"]}), pd.DataFrame({"s": ["a"], "d": ["a"], "type": ["KNOWS"], "weight": [7]}), @@ -1832,7 +1835,6 @@ def test_native_chain_rows_select_undirected_self_loop_duplicates_both_direction ) assert records == [ {"seedId": "a", "friendId": "a", "w": 7}, - {"seedId": "a", "friendId": "a", "w": 7}, ] def test_direct_rows_binding_ops_supports_undirected_edge_alias_projection(self):