From d4975aed4f46c63711869f2f48a756e3bd02aaef Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 05:12:22 -0700 Subject: [PATCH 01/36] feat(gfql): plan connected join predicate pushdown --- graphistry/compute/gfql/cypher/lowering.py | 380 +++++++++++++++++- .../compute/gfql/cypher/test_lowering.py | 114 ++++++ 2 files changed, 476 insertions(+), 18 deletions(-) diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index 8188a7e741..e42a16db65 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -81,6 +81,7 @@ collect_identifiers, parse_expr, walk_expr_nodes, + is_expr_node, ) from graphistry.compute.gfql.cypher.reentry_plan import ReentryPlan from graphistry.compute.gfql.cypher.ast import ( @@ -1254,6 +1255,15 @@ def _connected_join_alias_identity_expr( ) def _rewrite(node_in: ExprNode) -> ExprNode: + if ( + isinstance(node_in, FunctionCall) + and node_in.name.lower() == "count" + and len(node_in.args) == 1 + and isinstance(node_in.args[0], Identifier) + and "." not in node_in.args[0].name + and node_in.args[0].name in alias_targets + ): + return node_in if isinstance(node_in, PropertyAccessExpr) and isinstance(node_in.value, Identifier): if "." not in node_in.value.name and node_in.value.name in alias_targets: return node_in @@ -2303,12 +2313,12 @@ def _expr_has_aggregate(node: ExprNode) -> bool: if isinstance(node, FunctionCall) and node.name.lower() in _CYPHER_AGGREGATES: return True for child in getattr(node, "__dict__", {}).values(): - if isinstance(child, ExprNode): - if _expr_has_aggregate(child): + if is_expr_node(child): + if _expr_has_aggregate(cast(ExprNode, child)): return True elif isinstance(child, (list, tuple)): for c in child: - if isinstance(c, ExprNode) and _expr_has_aggregate(c): + if is_expr_node(c) and _expr_has_aggregate(cast(ExprNode, c)): return True return False @@ -2336,7 +2346,7 @@ def _binding_prop_alias_set( return None if getattr(query, "where", None) is not None: return None - matches = getattr(query, "matches", ()) or () + matches = cast(Sequence[MatchClause], getattr(query, "matches", ()) or ()) if len(matches) != 1: return None # multi-MATCH / cartesian — conservative match_clause = matches[0] @@ -2433,12 +2443,12 @@ def _visit(n: ExprNode) -> None: if root in alias_targets: out.add(root) for child in getattr(n, "__dict__", {}).values(): - if isinstance(child, ExprNode): - _visit(child) + if is_expr_node(child): + _visit(cast(ExprNode, child)) elif isinstance(child, (list, tuple)): for c in child: - if isinstance(c, ExprNode): - _visit(c) + if is_expr_node(c): + _visit(cast(ExprNode, c)) _visit(node) return out @@ -7676,6 +7686,7 @@ class ConnectedMatchJoinPlan: pattern_chains: Tuple[Chain, ...] pattern_shared_node_aliases: Tuple[Tuple[str, ...], ...] post_join_chain: Chain + pattern_attach_prop_aliases: Tuple[Optional[Tuple[str, ...]], ...] = () def _pattern_alias_lists(pattern: Sequence[PatternElement]) -> Tuple[List[str], List[str]]: @@ -7771,6 +7782,325 @@ def _next_id() -> int: ) +def _connected_join_literal_op(op: str, *, reverse: bool = False) -> Optional[str]: + normalized = "==" if op == "=" else op + if normalized not in {"==", "!=", "<", "<=", ">", ">="}: + return None + if not reverse: + return normalized + return { + "==": "==", + "!=": "!=", + "<": ">", + "<=": ">=", + ">": "<", + ">=": "<=", + }[normalized] + + +def _connected_join_expr_property_ref( + node: ExprNode, + *, + span: SourceSpan, +) -> Optional[PropertyRef]: + if isinstance(node, PropertyAccessExpr) and isinstance(node.value, Identifier): + if "." in node.value.name: + return None + return PropertyRef(alias=node.value.name, property=node.property, span=span) + if isinstance(node, Identifier): + parts = node.name.split(".") + if len(parts) == 2 and all(parts): + return PropertyRef(alias=parts[0], property=parts[1], span=span) + return None + + +def _connected_join_expr_literal_value(node: ExprNode) -> Tuple[bool, Optional[CypherLiteral]]: + if isinstance(node, ExprLiteral): + return True, cast(Optional[CypherLiteral], node.value) + if isinstance(node, UnaryOp) and node.op in {"+", "-"} and isinstance(node.operand, ExprLiteral): + value = node.operand.value + if isinstance(value, (int, float)) and not isinstance(value, bool): + return True, cast(CypherLiteral, value if node.op == "+" else -value) + return False, None + + +def _connected_join_lower_property_ref( + node: ExprNode, + *, + span: SourceSpan, +) -> Optional[PropertyRef]: + if not isinstance(node, FunctionCall) or len(node.args) != 1: + return None + if node.name.lower() not in {"tolower", "lower"}: + return None + return _connected_join_expr_property_ref(node.args[0], span=span) + + +def _connected_join_lower_literal_value(node: ExprNode) -> Tuple[bool, Optional[str]]: + if isinstance(node, FunctionCall) and len(node.args) == 1 and node.name.lower() in {"tolower", "lower"}: + arg = node.args[0] + if isinstance(arg, ExprLiteral) and isinstance(arg.value, str): + return True, arg.value + return False, None + if isinstance(node, ExprLiteral) and isinstance(node.value, str): + return True, node.value + return False, None + + +def _apply_connected_join_node_filter( + alias_targets_by_pattern: Sequence[Mapping[str, ASTObject]], + *, + prop_ref: PropertyRef, + op: str, + value: Optional[CypherLiteral], + params: Optional[Mapping[str, Any]], +) -> bool: + pushed = False + for alias_targets in alias_targets_by_pattern: + target = alias_targets.get(prop_ref.alias) + if not isinstance(target, ASTNode): + continue + _apply_literal_where( + cast(Dict[str, ASTObject], alias_targets), + left=prop_ref, + op=op, + right=value, + params=params, + ) + pushed = True + return pushed + + +def _apply_connected_join_node_predicate( + alias_targets_by_pattern: Sequence[Mapping[str, ASTObject]], + *, + prop_ref: PropertyRef, + predicate: ASTPredicate, +) -> bool: + pushed = False + for alias_targets in alias_targets_by_pattern: + target = alias_targets.get(prop_ref.alias) + if not isinstance(target, ASTNode): + continue + filter_dict = dict(target.filter_dict or {}) + existing_filter = filter_dict.get(prop_ref.property) + if existing_filter is None or prop_ref.property not in filter_dict: + filter_dict[prop_ref.property] = predicate + else: + filter_dict[prop_ref.property] = _merge_filter_predicates( + existing_filter, + predicate, + field=f"where.{prop_ref.alias}.{prop_ref.property}", + line=prop_ref.span.line, + column=prop_ref.span.column, + ) + target.filter_dict = filter_dict + pushed = True + return pushed + + +def _pushdown_connected_join_atom_filter( + expr: ExpressionText, + alias_targets_by_pattern: Sequence[Mapping[str, ASTObject]], + alias_targets: Mapping[str, ASTObject], + *, + params: Optional[Mapping[str, Any]], +) -> bool: + try: + node = _parse_row_expr( + expr.text, + params=params, + alias_targets=alias_targets, + field="where", + line=expr.span.line, + column=expr.span.column, + ) + except GFQLValidationError: + return False + if not isinstance(node, BinaryOp): + return False + + op = _connected_join_literal_op(node.op) + if op is not None: + left_ref = _connected_join_expr_property_ref(node.left, span=expr.span) + right_is_literal, right_value = _connected_join_expr_literal_value(node.right) + if left_ref is not None and right_is_literal: + return _apply_connected_join_node_filter( + alias_targets_by_pattern, + prop_ref=left_ref, + op=op, + value=right_value, + params=params, + ) + reverse_op = _connected_join_literal_op(node.op, reverse=True) + right_ref = _connected_join_expr_property_ref(node.right, span=expr.span) + left_is_literal, left_value = _connected_join_expr_literal_value(node.left) + if right_ref is not None and left_is_literal and reverse_op is not None: + return _apply_connected_join_node_filter( + alias_targets_by_pattern, + prop_ref=right_ref, + op=reverse_op, + value=left_value, + params=params, + ) + + if node.op in {"=", "=="}: + left_ref = _connected_join_lower_property_ref(node.left, span=expr.span) + right_is_literal, right_value = _connected_join_lower_literal_value(node.right) + if left_ref is not None and right_is_literal and right_value is not None: + return _apply_connected_join_node_predicate( + alias_targets_by_pattern, + prop_ref=left_ref, + predicate=fullmatch(re.escape(right_value), case=False, na=False), + ) + right_ref = _connected_join_lower_property_ref(node.right, span=expr.span) + left_is_literal, left_value = _connected_join_lower_literal_value(node.left) + if right_ref is not None and left_is_literal and left_value is not None: + return _apply_connected_join_node_predicate( + alias_targets_by_pattern, + prop_ref=right_ref, + predicate=fullmatch(re.escape(left_value), case=False, na=False), + ) + + return False + + +def _pushdown_connected_join_where_filters( + where: Optional[WhereClause], + alias_targets_by_pattern: Sequence[Mapping[str, ASTObject]], + alias_targets: Mapping[str, ASTObject], + *, + params: Optional[Mapping[str, Any]], +) -> Optional[List[ExpressionText]]: + if where is None: + return [] + + # Push structured predicates on the expr_tree parser path. When no expr_tree is + # present, the residual loop below treats predicates as an implicit AND list and + # applies pushdown exactly once. + if where.expr_tree is not None: + for predicate in where.predicates: + if not isinstance(predicate, WherePredicate): + continue + if isinstance(predicate.left, LabelRef): + for pattern_targets in alias_targets_by_pattern: + target = pattern_targets.get(predicate.left.alias) + if isinstance(target, ASTNode): + _apply_label_where(cast(Dict[str, ASTObject], pattern_targets), left=predicate.left) + continue + if isinstance(predicate.right, PropertyRef): + continue + if not isinstance(predicate.left, PropertyRef): + continue + _apply_connected_join_node_filter( + alias_targets_by_pattern, + prop_ref=predicate.left, + op=cast(str, predicate.op), + value=cast(Optional[CypherLiteral], predicate.right), + params=params, + ) + + residuals: List[ExpressionText] = [] + + def _walk(expr_node: BooleanExpr) -> None: + if expr_node.op == "and" and expr_node.left is not None and expr_node.right is not None: + _walk(expr_node.left) + _walk(expr_node.right) + return + if expr_node.op == "atom" and expr_node.atom_text is not None: + atom = ExpressionText(text=expr_node.atom_text, span=expr_node.atom_span or expr_node.span) + pushed = _pushdown_connected_join_atom_filter( + atom, + alias_targets_by_pattern, + alias_targets, + params=params, + ) + if not pushed: + residuals.append(atom) + return + synthesized = _where_clause_expr_text(where) + if synthesized is not None: + residuals.append(synthesized) + + if where.expr_tree is not None: + _walk(where.expr_tree) + return residuals + + for predicate in where.predicates: + if not isinstance(predicate, WherePredicate): + return None + pushed = False + if isinstance(predicate.left, LabelRef): + for pattern_targets in alias_targets_by_pattern: + target = pattern_targets.get(predicate.left.alias) + if isinstance(target, ASTNode): + _apply_label_where(cast(Dict[str, ASTObject], pattern_targets), left=predicate.left) + pushed = True + elif isinstance(predicate.left, PropertyRef) and not isinstance(predicate.right, PropertyRef): + pushed = _apply_connected_join_node_filter( + alias_targets_by_pattern, + prop_ref=predicate.left, + op=cast(str, predicate.op), + value=cast(Optional[CypherLiteral], predicate.right), + params=params, + ) + if pushed: + continue + row_text = _row_where_predicate_text(predicate) + if row_text is None: + return None + residuals.append(ExpressionText(text=row_text, span=where.span)) + return residuals + + +def _connected_join_required_property_aliases( + query: CypherQuery, + *, + alias_targets: Mapping[str, ASTObject], + residual_filters: Sequence[ExpressionText], + params: Optional[Mapping[str, Any]], +) -> Optional[Set[str]]: + if query.with_stages: + return None + node_aliases = {alias for alias, target in alias_targets.items() if isinstance(target, ASTNode)} + required: Set[str] = set() + + exprs: List[Tuple[str, int, int, str]] = [] + for expr in residual_filters: + exprs.append((expr.text, expr.span.line, expr.span.column, "where")) + for item in query.return_.items: + exprs.append((item.expression.text, item.span.line, item.span.column, query.return_.kind)) + if query.order_by is not None: + for order_item in query.order_by.items: + exprs.append((order_item.expression.text, order_item.span.line, order_item.span.column, "order_by")) + + for text, line, column, field_name in exprs: + if text == "*": + continue + try: + non_aggregate_aliases, _aggregate_aliases = _expr_match_alias_usage( + text, + alias_targets=alias_targets, + params=params, + field=field_name, + line=line, + column=column, + ) + prop_aliases = _expr_property_access_node_aliases( + text, + alias_targets=alias_targets, + params=params, + field=field_name, + line=line, + column=column, + ) + except GFQLValidationError: + return None + required.update(alias for alias in non_aggregate_aliases if alias in node_aliases) + required.update(alias for alias in prop_aliases if alias in node_aliases) + return required + + def _compile_connected_match_join( query: CypherQuery, *, @@ -7780,6 +8110,7 @@ def _compile_connected_match_join( clause = query.matches[0] pattern_chains: List[Chain] = [] pattern_node_aliases: List[Set[str]] = [] + alias_targets_by_pattern: List[Mapping[str, ASTObject]] = [] combined_alias_targets: Dict[str, ASTObject] = {} pre_join_filters: List[ExpressionText] = [] @@ -7793,6 +8124,7 @@ def _compile_connected_match_join( ) ops, duplicate_alias_where = _lower_match_clause_with_alias_equalities(single_clause, params=params) alias_targets = _alias_target(ops) + alias_targets_by_pattern.append(alias_targets) dynamic_where_out, dynamic_row_preds = _dynamic_property_entry_constraints( single_clause, alias_targets=alias_targets, @@ -7824,15 +8156,13 @@ def _compile_connected_match_join( accumulated_aliases.update(node_aliases) if query.where is not None: - # #1712: a WHERE on a connected comma-pattern join must be applied as a - # post-join row filter. Previously this only handled the ``expr_tree`` form - # and SILENTLY DROPPED the structured ``predicates`` form (e.g. a plain - # ``WHERE i.k = 'x'``), returning an unfiltered — wrong — result. - # ``_where_clause_expr_text`` renders BOTH forms; a WHERE it can't render - # (has-labels / outside the row-renderable subset) must NIE honestly, never - # be dropped. - synthesized = _where_clause_expr_text(query.where) - if synthesized is None: + residual_filters = _pushdown_connected_join_where_filters( + query.where, + alias_targets_by_pattern, + combined_alias_targets, + params=params, + ) + if residual_filters is None: raise _unsupported( "Cypher connected comma-pattern join lowering cannot render this WHERE clause " "to a row filter; use engine='pandas' with a supported WHERE shape", @@ -7841,7 +8171,20 @@ def _compile_connected_match_join( line=clause.span.line, column=clause.span.column, ) - pre_join_filters.append(synthesized) + pre_join_filters.extend(residual_filters) + + required_prop_aliases = _connected_join_required_property_aliases( + query, + alias_targets=combined_alias_targets, + residual_filters=pre_join_filters, + params=params, + ) + pattern_attach_prop_aliases: List[Optional[Tuple[str, ...]]] = [] + for pattern_alias_targets in alias_targets_by_pattern: + if required_prop_aliases is None: + pattern_attach_prop_aliases.append(None) + else: + pattern_attach_prop_aliases.append(tuple(sorted(required_prop_aliases & set(pattern_alias_targets.keys())))) for projection_clause in [stage.clause for stage in query.with_stages] + [query.return_]: _reject_unsupported_connected_join_clause_shapes( @@ -7949,6 +8292,7 @@ def _compile_connected_match_join( pattern_chains=tuple(pattern_chains), pattern_shared_node_aliases=tuple(shared_aliases_per_pattern), post_join_chain=Chain(row_steps), + pattern_attach_prop_aliases=tuple(pattern_attach_prop_aliases), ), query_graph=query_graph, logical_plan=logical_plan, diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 57e2c30cda..5f84f963d1 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -14933,6 +14933,7 @@ def test_issue_1273_multi_source_grouped_aggregate(agg: str, expected: list) -> assert result._nodes.to_dict(orient="records") == expected + def test_issue_1712_connected_comma_pattern_where_intersects() -> None: """#1712: a connected comma-pattern sharing a node alias with a WHERE on a leaf alias must intersect both patterns (the WHERE was silently dropped on the @@ -14946,6 +14947,119 @@ def test_issue_1712_connected_comma_pattern_where_intersects() -> None: assert result._nodes.to_dict(orient="records") == [{"numPersons": 2}] +def _mk_graph_benchmark_t1_shape_graph() -> "_CypherTestGraph": + nodes = pd.DataFrame({ + "id": [0, 1, 2, 10, 11, 20, 21], + "node_type": ["Person", "Person", "Person", "City", "City", "Interest", "Interest"], + "gender": ["MALE", "female", "male", None, None, None, None], + "age": [25, 27, 35, None, None, None, None], + "city": [None, None, None, "London", "Paris", None, None], + "country": [None, None, None, "United Kingdom", "France", None, None], + "state": [None, None, None, "England", "Ile-de-France", None, None], + "interest": [None, None, None, None, None, "Fine Dining", "photography"], + }) + edges = pd.DataFrame({ + "s": [0, 1, 2, 0, 1, 2], + "d": [20, 20, 21, 10, 10, 11], + "rel": ["HAS_INTEREST", "HAS_INTEREST", "HAS_INTEREST", "LIVES_IN", "LIVES_IN", "LIVES_IN"], + }) + return _mk_graph(nodes, edges) + + +def _compiled_connected_join_filters(query: str) -> list[dict[str, Any]]: + compiled = _compile_query(query) + plan = _compiled_execution_extras(compiled).connected_match_join + assert plan is not None + out: list[dict[str, Any]] = [] + for chain in plan.pattern_chains: + for op in chain.chain: + if isinstance(op, ASTNode) and isinstance(op._name, str): + out.append({op._name: dict(op.filter_dict or {})}) + return out + + +def _compiled_connected_join_plan(query: str) -> Any: + compiled = _compile_query(query) + plan = _compiled_execution_extras(compiled).connected_match_join + assert plan is not None + return plan + + +def _post_join_functions(query: str) -> list[str]: + return [op.function for op in _compiled_connected_join_plan(query).post_join_chain.chain if isinstance(op, ASTCall)] + + +def test_t1_connected_comma_q5_fully_pushed_prunes_residual_and_props() -> None: + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + "WHERE toLower(i.interest) = toLower('fine dining') " + "AND toLower(p.gender) = toLower('male') " + "AND c.city = 'London' AND c.country = 'United Kingdom' " + "RETURN count(p) AS numPersons" + ) + + plan = _compiled_connected_join_plan(query) + assert "where_rows" not in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == ((), ()) + + +def test_t1_connected_comma_grouped_projection_attaches_only_city_props() -> None: + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + "WHERE toLower(i.interest) = toLower('fine dining') " + "AND p.age >= 23 AND p.age <= 30 " + "AND c.country = 'United Kingdom' " + "RETURN count(p) AS numPersons, c.state AS state, c.country AS country " + "ORDER BY state" + ) + + plan = _compiled_connected_join_plan(query) + assert "where_rows" not in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == ((), ("c",)) + + +def test_t1_connected_comma_pushes_q5_single_alias_filters_before_join() -> None: + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + "WHERE toLower(i.interest) = toLower('fine dining') " + "AND toLower(p.gender) = toLower('male') " + "AND c.city = 'London' AND c.country = 'United Kingdom' " + "RETURN count(p) AS numPersons" + ) + + result = _mk_graph_benchmark_t1_shape_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == [{"numPersons": 1}] + + filters_by_alias = _compiled_connected_join_filters(query) + assert any(entry.get("i", {}).get("interest").__class__.__name__ == "Fullmatch" for entry in filters_by_alias) + assert any(entry.get("p", {}).get("gender").__class__.__name__ == "Fullmatch" for entry in filters_by_alias) + assert any(entry.get("c", {}).get("city") == "London" for entry in filters_by_alias) + assert any(entry.get("c", {}).get("country") == "United Kingdom" for entry in filters_by_alias) + + +def test_t1_connected_comma_pushes_q7_range_filters_before_join() -> None: + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + "WHERE toLower(i.interest) = toLower('photography') " + "AND p.age >= 23 AND p.age <= 30 " + "AND c.country = 'France' " + "RETURN count(p) AS numPersons, c.state AS state, c.country AS country" + ) + + result = _mk_graph_benchmark_t1_shape_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == [] + + filters_by_alias = _compiled_connected_join_filters(query) + assert any("age" in entry.get("p", {}) for entry in filters_by_alias) + assert any(entry.get("i", {}).get("interest").__class__.__name__ == "Fullmatch" for entry in filters_by_alias) + assert any(entry.get("c", {}).get("country") == "France" for entry in filters_by_alias) + + + def test_issue_1413_ic3_entity_membership_positive_same_city_friend_only() -> None: graph = _mk_ic3_cross_country_shape_graph() result = graph.gfql( From ebb7113a4a58c596dc147d54e74a30e62b576c96 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 10:22:22 -0700 Subject: [PATCH 02/36] fix(gfql): preserve lower literal comparison semantics --- graphistry/compute/gfql/cypher/lowering.py | 2 +- .../compute/gfql/cypher/test_lowering.py | 57 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index d7a67ba019..e7c6aabc97 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7843,7 +7843,7 @@ def _connected_join_lower_literal_value(node: ExprNode) -> Tuple[bool, Optional[ return True, arg.value return False, None if isinstance(node, ExprLiteral) and isinstance(node.value, str): - return True, node.value + return node.value == node.value.lower(), node.value return False, None diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 5f84f963d1..5041191a0c 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15040,6 +15040,63 @@ def test_t1_connected_comma_pushes_q5_single_alias_filters_before_join() -> None assert any(entry.get("c", {}).get("country") == "United Kingdom" for entry in filters_by_alias) +def test_t1_connected_comma_pushes_reversed_single_alias_filters_before_join() -> None: + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + "WHERE toLower('fine dining') = toLower(i.interest) " + "AND 23 <= p.age AND 30 >= p.age " + "AND 'London' = c.city " + "RETURN count(p) AS numPersons" + ) + + result = _mk_graph_benchmark_t1_shape_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == [{"numPersons": 2}] + + plan = _compiled_connected_join_plan(query) + assert "where_rows" not in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == ((), ()) + + filters_by_alias = _compiled_connected_join_filters(query) + assert any(entry.get("i", {}).get("interest").__class__.__name__ == "Fullmatch" for entry in filters_by_alias) + assert any("age" in entry.get("p", {}) for entry in filters_by_alias) + assert any(entry.get("c", {}).get("city") == "London" for entry in filters_by_alias) + + +def test_t1_connected_comma_pushes_lower_property_plain_lowercase_literal() -> None: + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + "WHERE toLower(i.interest) = 'fine dining' " + "AND p.age >= 23 AND p.age <= 30 AND c.city = 'London' " + "RETURN count(p) AS numPersons" + ) + + result = _mk_graph_benchmark_t1_shape_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == [{"numPersons": 2}] + + plan = _compiled_connected_join_plan(query) + assert "where_rows" not in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == ((), ()) + + +def test_t1_connected_comma_retains_reversed_uppercase_plain_literal_residual() -> None: + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + "WHERE 'FINE DINING' = toLower(i.interest) " + "AND p.age >= 23 AND p.age <= 30 AND c.city = 'London' " + "RETURN count(p) AS numPersons" + ) + + result = _mk_graph_benchmark_t1_shape_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == [] + + plan = _compiled_connected_join_plan(query) + assert "where_rows" in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == (("i",), ()) + + def test_t1_connected_comma_pushes_q7_range_filters_before_join() -> None: query = ( "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " From d152d057f9a5d0349d2302aa5a4c0e15d55b544f Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 11:06:56 -0700 Subject: [PATCH 03/36] fix(gfql): retain exact lower equality residuals --- graphistry/compute/gfql/cypher/lowering.py | 69 ---------------- .../compute/gfql/cypher/test_lowering.py | 78 +++++++++++++++---- 2 files changed, 63 insertions(+), 84 deletions(-) diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index e7c6aabc97..e78123fc47 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7824,29 +7824,6 @@ def _connected_join_expr_literal_value(node: ExprNode) -> Tuple[bool, Optional[C return False, None -def _connected_join_lower_property_ref( - node: ExprNode, - *, - span: SourceSpan, -) -> Optional[PropertyRef]: - if not isinstance(node, FunctionCall) or len(node.args) != 1: - return None - if node.name.lower() not in {"tolower", "lower"}: - return None - return _connected_join_expr_property_ref(node.args[0], span=span) - - -def _connected_join_lower_literal_value(node: ExprNode) -> Tuple[bool, Optional[str]]: - if isinstance(node, FunctionCall) and len(node.args) == 1 and node.name.lower() in {"tolower", "lower"}: - arg = node.args[0] - if isinstance(arg, ExprLiteral) and isinstance(arg.value, str): - return True, arg.value - return False, None - if isinstance(node, ExprLiteral) and isinstance(node.value, str): - return node.value == node.value.lower(), node.value - return False, None - - def _apply_connected_join_node_filter( alias_targets_by_pattern: Sequence[Mapping[str, ASTObject]], *, @@ -7871,34 +7848,6 @@ def _apply_connected_join_node_filter( return pushed -def _apply_connected_join_node_predicate( - alias_targets_by_pattern: Sequence[Mapping[str, ASTObject]], - *, - prop_ref: PropertyRef, - predicate: ASTPredicate, -) -> bool: - pushed = False - for alias_targets in alias_targets_by_pattern: - target = alias_targets.get(prop_ref.alias) - if not isinstance(target, ASTNode): - continue - filter_dict = dict(target.filter_dict or {}) - existing_filter = filter_dict.get(prop_ref.property) - if existing_filter is None or prop_ref.property not in filter_dict: - filter_dict[prop_ref.property] = predicate - else: - filter_dict[prop_ref.property] = _merge_filter_predicates( - existing_filter, - predicate, - field=f"where.{prop_ref.alias}.{prop_ref.property}", - line=prop_ref.span.line, - column=prop_ref.span.column, - ) - target.filter_dict = filter_dict - pushed = True - return pushed - - def _pushdown_connected_join_atom_filter( expr: ExpressionText, alias_targets_by_pattern: Sequence[Mapping[str, ASTObject]], @@ -7944,24 +7893,6 @@ def _pushdown_connected_join_atom_filter( params=params, ) - if node.op in {"=", "=="}: - left_ref = _connected_join_lower_property_ref(node.left, span=expr.span) - right_is_literal, right_value = _connected_join_lower_literal_value(node.right) - if left_ref is not None and right_is_literal and right_value is not None: - return _apply_connected_join_node_predicate( - alias_targets_by_pattern, - prop_ref=left_ref, - predicate=fullmatch(re.escape(right_value), case=False, na=False), - ) - right_ref = _connected_join_lower_property_ref(node.right, span=expr.span) - left_is_literal, left_value = _connected_join_lower_literal_value(node.left) - if right_ref is not None and left_is_literal and left_value is not None: - return _apply_connected_join_node_predicate( - alias_targets_by_pattern, - prop_ref=right_ref, - predicate=fullmatch(re.escape(left_value), case=False, na=False), - ) - return False diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 5041191a0c..c65fa1d00e 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -14989,7 +14989,7 @@ def _post_join_functions(query: str) -> list[str]: return [op.function for op in _compiled_connected_join_plan(query).post_join_chain.chain if isinstance(op, ASTCall)] -def test_t1_connected_comma_q5_fully_pushed_prunes_residual_and_props() -> None: +def test_t1_connected_comma_q5_retains_lower_residual_and_props() -> None: query = ( "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " @@ -15000,8 +15000,8 @@ def test_t1_connected_comma_q5_fully_pushed_prunes_residual_and_props() -> None: ) plan = _compiled_connected_join_plan(query) - assert "where_rows" not in _post_join_functions(query) - assert plan.pattern_attach_prop_aliases == ((), ()) + assert "where_rows" in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == (("i", "p"), ("p",)) def test_t1_connected_comma_grouped_projection_attaches_only_city_props() -> None: @@ -15016,11 +15016,11 @@ def test_t1_connected_comma_grouped_projection_attaches_only_city_props() -> Non ) plan = _compiled_connected_join_plan(query) - assert "where_rows" not in _post_join_functions(query) - assert plan.pattern_attach_prop_aliases == ((), ("c",)) + assert "where_rows" in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == (("i",), ("c",)) -def test_t1_connected_comma_pushes_q5_single_alias_filters_before_join() -> None: +def test_t1_connected_comma_pushes_q5_literal_filters_and_retains_lower_residual() -> None: query = ( "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " @@ -15033,9 +15033,13 @@ def test_t1_connected_comma_pushes_q5_single_alias_filters_before_join() -> None result = _mk_graph_benchmark_t1_shape_graph().gfql(query) assert result._nodes.to_dict(orient="records") == [{"numPersons": 1}] + plan = _compiled_connected_join_plan(query) + assert "where_rows" in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == (("i", "p"), ("p",)) + filters_by_alias = _compiled_connected_join_filters(query) - assert any(entry.get("i", {}).get("interest").__class__.__name__ == "Fullmatch" for entry in filters_by_alias) - assert any(entry.get("p", {}).get("gender").__class__.__name__ == "Fullmatch" for entry in filters_by_alias) + assert not any("interest" in entry.get("i", {}) for entry in filters_by_alias) + assert not any("gender" in entry.get("p", {}) for entry in filters_by_alias) assert any(entry.get("c", {}).get("city") == "London" for entry in filters_by_alias) assert any(entry.get("c", {}).get("country") == "United Kingdom" for entry in filters_by_alias) @@ -15054,16 +15058,16 @@ def test_t1_connected_comma_pushes_reversed_single_alias_filters_before_join() - assert result._nodes.to_dict(orient="records") == [{"numPersons": 2}] plan = _compiled_connected_join_plan(query) - assert "where_rows" not in _post_join_functions(query) - assert plan.pattern_attach_prop_aliases == ((), ()) + assert "where_rows" in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == (("i",), ()) filters_by_alias = _compiled_connected_join_filters(query) - assert any(entry.get("i", {}).get("interest").__class__.__name__ == "Fullmatch" for entry in filters_by_alias) + assert not any("interest" in entry.get("i", {}) for entry in filters_by_alias) assert any("age" in entry.get("p", {}) for entry in filters_by_alias) assert any(entry.get("c", {}).get("city") == "London" for entry in filters_by_alias) -def test_t1_connected_comma_pushes_lower_property_plain_lowercase_literal() -> None: +def test_t1_connected_comma_retains_lower_property_plain_lowercase_literal() -> None: query = ( "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " @@ -15076,8 +15080,8 @@ def test_t1_connected_comma_pushes_lower_property_plain_lowercase_literal() -> N assert result._nodes.to_dict(orient="records") == [{"numPersons": 2}] plan = _compiled_connected_join_plan(query) - assert "where_rows" not in _post_join_functions(query) - assert plan.pattern_attach_prop_aliases == ((), ()) + assert "where_rows" in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == (("i",), ()) def test_t1_connected_comma_retains_reversed_uppercase_plain_literal_residual() -> None: @@ -15097,6 +15101,46 @@ def test_t1_connected_comma_retains_reversed_uppercase_plain_literal_residual() assert plan.pattern_attach_prop_aliases == (("i",), ()) +@pytest.mark.parametrize( + "where_expr", + [ + "toLower(i.interest) = toLower('i')", + "toLower('i') = toLower(i.interest)", + ], +) +def test_t1_connected_comma_retains_unicode_lower_equality_residual(where_expr: str) -> None: + values = ["İ", "i", "I", "ı"] + person_ids = list(range(4)) + interest_ids = list(range(10, 14)) + city_id = 20 + nodes = pd.DataFrame({ + "id": person_ids + interest_ids + [city_id], + "node_type": ["Person"] * 4 + ["Interest"] * 4 + ["City"], + "interest": [None] * 4 + values + [None], + "city": [None] * 8 + ["London"], + }) + edges = pd.DataFrame({ + "s": person_ids + person_ids, + "d": interest_ids + [city_id] * 4, + "rel": ["HAS_INTEREST"] * 4 + ["LIVES_IN"] * 4, + }) + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + f"WHERE {where_expr} AND c.city = 'London' " + "RETURN count(p) AS n" + ) + + oracle_count = sum(value.lower() == "i".lower() for value in values) + assert oracle_count == 2 + result = _mk_graph(nodes, edges).gfql(query) + assert result._nodes.to_dict(orient="records") == [{"n": oracle_count}] + + plan = _compiled_connected_join_plan(query) + assert "where_rows" in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == (("i",), ()) + + def test_t1_connected_comma_pushes_q7_range_filters_before_join() -> None: query = ( "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " @@ -15110,9 +15154,13 @@ def test_t1_connected_comma_pushes_q7_range_filters_before_join() -> None: result = _mk_graph_benchmark_t1_shape_graph().gfql(query) assert result._nodes.to_dict(orient="records") == [] + plan = _compiled_connected_join_plan(query) + assert "where_rows" in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == (("i",), ("c",)) + filters_by_alias = _compiled_connected_join_filters(query) assert any("age" in entry.get("p", {}) for entry in filters_by_alias) - assert any(entry.get("i", {}).get("interest").__class__.__name__ == "Fullmatch" for entry in filters_by_alias) + assert not any("interest" in entry.get("i", {}) for entry in filters_by_alias) assert any(entry.get("c", {}).get("country") == "France" for entry in filters_by_alias) From 0ce47f1da32e773575ac6150cdf221d8fb3e1839 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 11:52:14 -0700 Subject: [PATCH 04/36] test(gfql): use row semantics for unicode lower oracle --- .../tests/compute/gfql/cypher/test_lowering.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index c65fa1d00e..03204f23aa 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15131,10 +15131,19 @@ def test_t1_connected_comma_retains_unicode_lower_equality_residual(where_expr: "RETURN count(p) AS n" ) - oracle_count = sum(value.lower() == "i".lower() for value in values) - assert oracle_count == 2 - result = _mk_graph(nodes, edges).gfql(query) - assert result._nodes.to_dict(orient="records") == [{"n": oracle_count}] + graph = _mk_graph(nodes, edges) + oracle_query = ( + "MATCH (i {node_type:'Interest'}) " + f"WHERE {where_expr} " + "RETURN count(i) AS n" + ) + oracle_compiled = cast(CompiledCypherQuery, compile_cypher(oracle_query)) + assert oracle_compiled.execution_extras.connected_match_join is None + oracle_rows = graph.gfql(oracle_query)._nodes.to_dict(orient="records") + assert oracle_rows[0]["n"] != len(values) + + result = graph.gfql(query) + assert result._nodes.to_dict(orient="records") == oracle_rows plan = _compiled_connected_join_plan(query) assert "where_rows" in _post_join_functions(query) From aa065364a93385dd6c7de1826d229b08f1b9cdc4 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 16:19:23 -0700 Subject: [PATCH 05/36] test(gfql): cover mixed connected-join residual; drop unreachable unary literal branch The py3.12 per-file coverage baseline gate failed on lowering.py (88.93% vs 89.05% floor). Two changes address it: Add a mixed connected-comma test exercising unary pushdown alongside an IN residual, covering a real planner path that had no coverage. Remove the UnaryOp branch of _connected_join_expr_literal_value. It is unreachable: the grammar folds signed numeric literals, so `p.age >= -1` parses to Literal(-1) and a surviving UnaryOp always wraps a non-literal operand (`-x` -> operand=Identifier). Param substitution rewrites text before parsing, so `-$minAge` folds the same way or fails closed on `--2`. The two callers both consume _parse_row_expr output via BinaryOp.left/.right, and the UnaryOp import stays live for six other uses. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql/cypher/lowering.py | 6 ++---- .../compute/gfql/cypher/test_lowering.py | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index e78123fc47..e920a6b513 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7815,12 +7815,10 @@ def _connected_join_expr_property_ref( def _connected_join_expr_literal_value(node: ExprNode) -> Tuple[bool, Optional[CypherLiteral]]: + # Signed numeric literals arrive folded (`-1` parses to `Literal(-1)`), so a + # `UnaryOp` here always wraps a non-literal operand and is not a literal value. if isinstance(node, ExprLiteral): return True, cast(Optional[CypherLiteral], node.value) - if isinstance(node, UnaryOp) and node.op in {"+", "-"} and isinstance(node.operand, ExprLiteral): - value = node.operand.value - if isinstance(value, (int, float)) and not isinstance(value, bool): - return True, cast(CypherLiteral, value if node.op == "+" else -value) return False, None diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 03204f23aa..6b1732a221 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -14989,6 +14989,25 @@ def _post_join_functions(query: str) -> list[str]: return [op.function for op in _compiled_connected_join_plan(query).post_join_chain.chain if isinstance(op, ASTCall)] +def test_t1_connected_comma_mixes_unary_pushdown_and_in_residual() -> None: + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + "WHERE toLower(i.interest) = toLower('fine dining') " + "AND p.age >= -1 AND p.age IN [25, 27] " + "RETURN count(p) AS numPersons" + ) + + result = _mk_graph_benchmark_t1_shape_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == [{"numPersons": 2}] + + plan = _compiled_connected_join_plan(query) + assert "where_rows" in _post_join_functions(query) + assert plan.pattern_attach_prop_aliases == (("i", "p"), ("p",)) + filters_by_alias = _compiled_connected_join_filters(query) + assert any("age" in entry.get("p", {}) for entry in filters_by_alias) + + def test_t1_connected_comma_q5_retains_lower_residual_and_props() -> None: query = ( "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " From 6831bb9e696911ba35bba5eeda6853d3ea6edb72 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 16:23:40 -0700 Subject: [PATCH 06/36] test(gfql): cover connected-join label predicate pushdown The dead-code removal alone left lowering.py at 89.0022%, passing the per-file floor only by exact equality with the 0.05 tolerance, and left changed-line coverage at 76.06% on the gfql lane, under the 80% gate. The connected-join planner pushes LabelRef predicates onto pattern targets, a real branch of this PR's new code that had no coverage. Cover it with a labelled T1 fixture and a label-plus-property query, asserting both the count semantics and that label__Person and age reach the pushed filters. lowering.py now clears its floor outright at 89.14% (no tolerance needed) and changed-line coverage reaches 80.28% on the gfql lane alone. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- .../compute/gfql/cypher/test_lowering.py | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 6b1732a221..95a514cfce 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -14989,6 +14989,41 @@ def _post_join_functions(query: str) -> list[str]: return [op.function for op in _compiled_connected_join_plan(query).post_join_chain.chain if isinstance(op, ASTCall)] +def _mk_graph_benchmark_t1_labelled_shape_graph() -> "_CypherTestGraph": + nodes = pd.DataFrame({ + "id": [0, 1, 2, 10, 11, 20, 21], + "node_type": ["Person", "Person", "Person", "City", "City", "Interest", "Interest"], + "age": [25, 27, 35, None, None, None, None], + "city": [None, None, None, "London", "Paris", None, None], + "interest": [None, None, None, None, None, "Fine Dining", "photography"], + "label__Person": [True, True, True, False, False, False, False], + "label__City": [False, False, False, True, True, False, False], + "label__Interest": [False, False, False, False, False, True, True], + }) + edges = pd.DataFrame({ + "s": [0, 1, 2, 0, 1, 2], + "d": [20, 20, 21, 10, 10, 11], + "rel": ["HAS_INTEREST", "HAS_INTEREST", "HAS_INTEREST", "LIVES_IN", "LIVES_IN", "LIVES_IN"], + }) + return _mk_graph(nodes, edges) + + +def test_t1_connected_comma_pushes_label_predicate_with_property_filter() -> None: + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + "WHERE p:Person AND p.age >= 26 " + "RETURN count(p) AS numPersons" + ) + + result = _mk_graph_benchmark_t1_labelled_shape_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == [{"numPersons": 2}] + + filters_by_alias = _compiled_connected_join_filters(query) + assert any("label__Person" in entry.get("p", {}) for entry in filters_by_alias) + assert any("age" in entry.get("p", {}) for entry in filters_by_alias) + + def test_t1_connected_comma_mixes_unary_pushdown_and_in_residual() -> None: query = ( "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " From 8da7e0769f4b17e65193512643ff55a1816443d6 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 16:35:10 -0700 Subject: [PATCH 07/36] fix(gfql): restore unary literal pushdown wrongly removed as dead code Commit aa065364 deleted the UnaryOp branch of _connected_join_expr_literal_value claiming it was unreachable. That was wrong. NUMBER carries its sign as a lexer terminal (/[+-]?.../), so a signed literal only folds when the sign is lexically adjacent: `-1` folds to Literal(-1), but `- 1` and `-(1)` keep the UnaryOp node and reach the helper with an ExprLiteral operand. The earlier probe only tried `-1` and `-x` and over-generalised from them. Removing the branch silently stopped pushing those comparisons onto pattern targets, degrading them to a where_rows residual. Results stayed correct, so no test caught it -- an optimization regression, not a wrong answer. Restore the branch and cover it with a parametrized `- 26` / `-(26)` / `+(26)` test asserting the filter reaches the pushed targets. Removing the branch again fails all three. Rename the mixed test, whose `-1` folds and never exercised a unary node, to say what it actually covers. lowering.py clears its per-file floor outright at 89.16% (3149/3532) with the branch restored, so the coverage gate needed real tests, not the deletion. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql/cypher/lowering.py | 10 +++++++-- .../compute/gfql/cypher/test_lowering.py | 22 ++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index e920a6b513..df39e19f3d 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7815,10 +7815,16 @@ def _connected_join_expr_property_ref( def _connected_join_expr_literal_value(node: ExprNode) -> Tuple[bool, Optional[CypherLiteral]]: - # Signed numeric literals arrive folded (`-1` parses to `Literal(-1)`), so a - # `UnaryOp` here always wraps a non-literal operand and is not a literal value. + # `NUMBER` carries an optional sign as a lexer terminal, so a signed literal only + # folds when the sign is lexically adjacent (`-1`). With a space or parens + # (`- 1`, `-(1)`) the unary survives as a node and must be folded here, or the + # comparison silently falls back to a row residual instead of pushing down. if isinstance(node, ExprLiteral): return True, cast(Optional[CypherLiteral], node.value) + if isinstance(node, UnaryOp) and node.op in {"+", "-"} and isinstance(node.operand, ExprLiteral): + value = node.operand.value + if isinstance(value, (int, float)) and not isinstance(value, bool): + return True, cast(CypherLiteral, value if node.op == "+" else -value) return False, None diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 95a514cfce..09108dd0fb 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15024,7 +15024,7 @@ def test_t1_connected_comma_pushes_label_predicate_with_property_filter() -> Non assert any("age" in entry.get("p", {}) for entry in filters_by_alias) -def test_t1_connected_comma_mixes_unary_pushdown_and_in_residual() -> None: +def test_t1_connected_comma_mixes_signed_literal_pushdown_and_in_residual() -> None: query = ( "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " @@ -15043,6 +15043,26 @@ def test_t1_connected_comma_mixes_unary_pushdown_and_in_residual() -> None: assert any("age" in entry.get("p", {}) for entry in filters_by_alias) +@pytest.mark.parametrize("predicate", ["p.age >= - 26", "p.age >= -(26)", "p.age >= +(26)"]) +def test_t1_connected_comma_pushes_unfolded_unary_literal(predicate: str) -> None: + # `NUMBER` carries its sign as a lexer terminal, so only a lexically adjacent sign + # folds into the literal. A spaced or parenthesised sign stays a UnaryOp node and + # must still push down rather than degrade to a row residual. + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + f"WHERE {predicate} " + "RETURN count(p) AS numPersons" + ) + + expected = 3 if predicate != "p.age >= +(26)" else 2 + result = _mk_graph_benchmark_t1_shape_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == [{"numPersons": expected}] + + filters_by_alias = _compiled_connected_join_filters(query) + assert any("age" in entry.get("p", {}) for entry in filters_by_alias) + + def test_t1_connected_comma_q5_retains_lower_residual_and_props() -> None: query = ( "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " From 55797c9addb86237ceb8f18d653037dcf988c119 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 17:01:42 -0700 Subject: [PATCH 08/36] fix(gfql): only push connected-join filters that survive filter_dict Connected-join pushdown moved WHERE atoms onto pattern targets without checking the op/literal pairing was representable there, regressing two shapes against master: WHERE p.nick = $v (v=None) master [] -> HEAD n=4 (every row, unfiltered) WHERE p.nick <> 'a' master n=2 -> HEAD GFQLTypeError Null equality: the executor re-binds the pushed chain through ASTNode.to_json, and _filter_dict_to_json drops null-valued entries, so the predicate silently disappeared and the residual that would have applied it had already been dropped. Non-numeric ordering: !=//>= lower to NumericASTPredicate, which admits only int/float, so a string or bool literal raised where master answered from a where_rows residual. Gate pushdown on representability and fall back to the residual otherwise. Parameters are resolved before the check -- $v arrives as an unresolved ParameterRef, and judging the ref rather than its value is how the null case got through. Unresolvable refs stay residual so the missing-parameter error still surfaces from its normal path. Pushdown is unaffected where it was already valid: numeric compares, string equality, and unary literals still push with no residual. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql/cypher/lowering.py | 35 ++++++++++ .../compute/gfql/cypher/test_lowering.py | 70 +++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index df39e19f3d..18e91e9041 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7828,6 +7828,39 @@ def _connected_join_expr_literal_value(node: ExprNode) -> Tuple[bool, Optional[C return False, None +def _connected_join_pushable_value( + op: str, + value: Optional[CypherLiteral], + *, + params: Optional[Mapping[str, Any]], +) -> bool: + """Whether `op`/`value` survives the trip into a node `filter_dict`. + + Pushdown is an optimization: anything not exactly representable must stay a + `where_rows` residual rather than push a filter that means something else. + + - `None` is unrepresentable: `_filter_dict_to_json` drops null-valued entries, so a + pushed `nick = null` would vanish on the executor's serialization round-trip and + silently return unfiltered rows. + - Ordering/inequality ops lower to `NumericASTPredicate`, which admits only int/float + (`bool` is an `int` subclass but is not a numeric column predicate), so pushing a + string or bool would raise where the residual answers correctly. + + Parameters must be resolved first: `$v` arrives as an unresolved `ParameterRef`, and + judging that ref instead of its value is how `nick = $v`/`{'v': None}` slipped through. + An unresolvable ref stays a residual, which reports the missing parameter itself. + """ + try: + resolved = _resolve_literal(cast(CypherLiteral, value), params=params, field="where") if value is not None else None + except GFQLValidationError: + return False + if resolved is None: + return False + if op == "==": + return True + return isinstance(resolved, (int, float)) and not isinstance(resolved, bool) + + def _apply_connected_join_node_filter( alias_targets_by_pattern: Sequence[Mapping[str, ASTObject]], *, @@ -7836,6 +7869,8 @@ def _apply_connected_join_node_filter( value: Optional[CypherLiteral], params: Optional[Mapping[str, Any]], ) -> bool: + if not _connected_join_pushable_value(op, value, params=params): + return False pushed = False for alias_targets in alias_targets_by_pattern: target = alias_targets.get(prop_ref.alias) diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 09108dd0fb..79e4abafc5 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15043,6 +15043,76 @@ def test_t1_connected_comma_mixes_signed_literal_pushdown_and_in_residual() -> N assert any("age" in entry.get("p", {}) for entry in filters_by_alias) +def _mk_graph_benchmark_t1_nullable_shape_graph() -> "_CypherTestGraph": + nodes = pd.DataFrame({ + "id": [0, 1, 2, 3, 10, 11, 20, 21], + "node_type": ["Person"] * 4 + ["City"] * 2 + ["Interest"] * 2, + "age": [25, 27, 35, None, None, None, None, None], + "nick": ["a", "b", "c", None, None, None, None, None], + "flag": [True, False, True, None, None, None, None, None], + "city": [None] * 4 + ["London", "Paris", None, None], + "interest": [None] * 6 + ["Fine Dining", "photography"], + }) + edges = pd.DataFrame({ + "s": [0, 1, 2, 3, 0, 1, 2, 3], + "d": [20, 20, 21, 20, 10, 10, 11, 11], + "rel": ["HAS_INTEREST"] * 4 + ["LIVES_IN"] * 4, + }) + return _mk_graph(nodes, edges) + + +def _t1_nullable_query(predicate: str) -> str: + return ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + f"WHERE {predicate} " + "RETURN count(p) AS n" + ) + + +def test_connected_join_does_not_push_null_equality() -> None: + # `filter_dict` serialization drops null values, so a pushed `nick = null` would + # vanish and return every row unfiltered. It must stay a residual instead. + query = _t1_nullable_query("p.nick = $v") + + result = _mk_graph_benchmark_t1_nullable_shape_graph().gfql(query, params={"v": None}) + assert result._nodes.to_dict(orient="records") == [] + + compiled = cast(CompiledCypherQuery, compile_cypher(query, params={"v": None})) + plan = _compiled_execution_extras(compiled).connected_match_join + assert plan is not None + pushed_props = { + prop + for chain in plan.pattern_chains + for op in chain.chain + if isinstance(op, ASTNode) + for prop in (op.filter_dict or {}) + } + assert "nick" not in pushed_props + residuals = [op.function for op in plan.post_join_chain.chain if isinstance(op, ASTCall)] + assert "where_rows" in residuals + + +@pytest.mark.parametrize( + "predicate,expected", + [ + ("p.nick <> 'a'", 2), + ("i.interest <> 'photography'", 3), + ("p.nick < 'c'", 2), + ("p.flag >= true", 2), + ], +) +def test_connected_join_does_not_push_non_numeric_ordering(predicate: str, expected: int) -> None: + # Ordering/inequality ops lower to numeric-only predicates; pushing a string or bool + # raises, so these must fall back to a residual and still answer correctly. + query = _t1_nullable_query(predicate) + + result = _mk_graph_benchmark_t1_nullable_shape_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == [{"n": expected}] + + assert "where_rows" in _post_join_functions(query) + + @pytest.mark.parametrize("predicate", ["p.age >= - 26", "p.age >= -(26)", "p.age >= +(26)"]) def test_t1_connected_comma_pushes_unfolded_unary_literal(predicate: str) -> None: # `NUMBER` carries its sign as a lexer terminal, so only a lexically adjacent sign From 529674f83bcbf878d8fb1e33dd60e408e6913bdc Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 17:36:37 -0700 Subject: [PATCH 09/36] fix(gfql): stop connected-join pushdown from injecting predicates and dropping identity Three more regressions this PR introduced against master, all found by independent adversarial review and verified against master 770d2a54: p.age = $v {"v":{"type":"GT","val":26}} master [] -> n=2 (ran `age > 26`) count(DISTINCT p), node id column != 'id' master 3 -> 1 p.nick STARTS WITH 'a' pre-guard 1 -> error (my guard over-rejected) Predicate injection: _filter_dict_to_json passes a raw dict through verbatim and maybe_filter_dict_from_json revives it via predicates_from_json, so a caller-supplied map param executed as an arbitrary predicate. Gate `==` to scalars. count(DISTINCT): the count() early-return left a bare alias unrewritten. It never checked FunctionCall.distinct, and a bare alias only resolves when the node id column is named `id`/`p`; otherwise it degrades to a constant and collapses the count. Master rewrites to the identity column, so drop the early-return and let it. String predicates: contains/starts_with/ends_with/regex lower to real round-trippable ASTPredicates and the residual cannot render them at all, so rejecting them turned working queries into errors. Allow them to push. Tests use a real Plottable, not _CypherTestGraph: the executor's serialization round-trip is where the dropped-null and revived-predicate filters bite, and the test double cannot observe it -- which is why the PR's own tests missed all of this. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql/cypher/lowering.py | 21 ++--- .../compute/gfql/cypher/test_lowering.py | 89 +++++++++++++++++++ 2 files changed, 100 insertions(+), 10 deletions(-) diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index 18e91e9041..39862eb89b 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -1255,15 +1255,6 @@ def _connected_join_alias_identity_expr( ) def _rewrite(node_in: ExprNode) -> ExprNode: - if ( - isinstance(node_in, FunctionCall) - and node_in.name.lower() == "count" - and len(node_in.args) == 1 - and isinstance(node_in.args[0], Identifier) - and "." not in node_in.args[0].name - and node_in.args[0].name in alias_targets - ): - return node_in if isinstance(node_in, PropertyAccessExpr) and isinstance(node_in.value, Identifier): if "." not in node_in.value.name and node_in.value.name in alias_targets: return node_in @@ -7828,6 +7819,9 @@ def _connected_join_expr_literal_value(node: ExprNode) -> Tuple[bool, Optional[C return False, None +_CONNECTED_JOIN_STRING_OPS = frozenset({"contains", "starts_with", "ends_with", "regex"}) + + def _connected_join_pushable_value( op: str, value: Optional[CypherLiteral], @@ -7856,8 +7850,15 @@ def _connected_join_pushable_value( return False if resolved is None: return False + if isinstance(resolved, (dict, list, tuple, set)): + # A dict survives `_filter_dict_to_json` verbatim and `maybe_filter_dict_from_json` + # revives it via `predicates_from_json`, so a param like {'type': 'GT', 'val': 26} + # would execute as `> 26` instead of an equality against a map. + return False + if op in _CONNECTED_JOIN_STRING_OPS: + return isinstance(resolved, str) if op == "==": - return True + return isinstance(resolved, (str, int, float, bool)) return isinstance(resolved, (int, float)) and not isinstance(resolved, bool) diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 79e4abafc5..79f4ec86d1 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -44,6 +44,7 @@ from graphistry.compute.gfql.ir.bound_ir import BoundIR, BoundQueryPart, SemanticTable from graphistry.compute.gfql.ir.compilation import PlanContext from graphistry.compute.gfql.ir.logical_plan import CHILD_SLOTS, Filter, PatternMatch, ProcedureCall as LogicalProcedureCall +from graphistry.Plottable import Plottable from graphistry.tests.test_compute import CGFull from graphistry.tests.compute.gfql.cypher._whole_entity_compat import entity_text_records @@ -15061,6 +15062,27 @@ def _mk_graph_benchmark_t1_nullable_shape_graph() -> "_CypherTestGraph": return _mk_graph(nodes, edges) +def _real_t1_nullable_graph() -> Plottable: + # A real Plottable, not `_CypherTestGraph`: the executor's serialize/deserialize + # round-trip is where dropped-null and revived-predicate filters actually bite, and + # the test double does not reproduce it. + nodes = pd.DataFrame({ + "id": [0, 1, 2, 3, 10, 11, 20, 21], + "node_type": ["Person"] * 4 + ["City"] * 2 + ["Interest"] * 2, + "age": [25, 27, 35, None, None, None, None, None], + "nick": ["a", "b", "c", None, None, None, None, None], + "flag": [True, False, True, None, None, None, None, None], + "city": [None] * 4 + ["London", "Paris", None, None], + "interest": [None] * 6 + ["Fine Dining", "photography"], + }) + edges = pd.DataFrame({ + "s": [0, 1, 2, 3, 0, 1, 2, 3], + "d": [20, 20, 21, 20, 10, 10, 11, 11], + "rel": ["HAS_INTEREST"] * 4 + ["LIVES_IN"] * 4, + }) + return graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + def _t1_nullable_query(predicate: str) -> str: return ( "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " @@ -15113,6 +15135,73 @@ def test_connected_join_does_not_push_non_numeric_ordering(predicate: str, expec assert "where_rows" in _post_join_functions(query) +@pytest.mark.parametrize( + "value", + [ + {"type": "GT", "val": 26}, + {"type": "Between", "lower": 0, "upper": 100, "inclusive": True}, + [25, 27], + ], +) +def test_connected_join_does_not_push_non_scalar_param(value: Any) -> None: + # `_filter_dict_to_json` passes a dict through verbatim and `maybe_filter_dict_from_json` + # revives it through `predicates_from_json`, so pushing a caller-supplied map would run + # an arbitrary predicate (`age > 26`) instead of an equality against a map. + # Must use a real Plottable: the serialization round-trip that resurrects the predicate + # only happens on the real executor, so `_CypherTestGraph` cannot observe this. + query = _t1_nullable_query("p.age = $v") + + result = _real_t1_nullable_graph().gfql(query, params={"v": value}) + assert result._nodes.to_dict(orient="records") == [] + + +@pytest.mark.parametrize( + "predicate,expected", + [ + ("p.nick STARTS WITH 'a'", 1), + ("p.nick CONTAINS 'a'", 1), + ("p.nick ENDS WITH 'a'", 1), + ("p.nick =~ 'a'", 1), + ], +) +def test_connected_join_pushes_string_predicates(predicate: str, expected: int) -> None: + # These lower to real round-trippable ASTPredicates, and the residual cannot render + # them at all, so they must push rather than fall back. + query = _t1_nullable_query(predicate) + + result = _mk_graph_benchmark_t1_nullable_shape_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == [{"n": expected}] + + filters_by_alias = _compiled_connected_join_filters(query) + assert any("nick" in entry.get("p", {}) for entry in filters_by_alias) + + +@pytest.mark.parametrize("node_id_column", ["id", "nid"]) +def test_connected_join_count_distinct_alias_is_node_identity(node_id_column: str) -> None: + # A bare alias must lower to its identity column. Left unrewritten it only resolves when + # the node id column happens to be named `id`/`p`, and otherwise degrades to a constant, + # collapsing count(DISTINCT p) to 1. + nodes = pd.DataFrame({ + node_id_column: [0, 1, 2, 10, 11, 20, 21], + "node_type": ["Person"] * 3 + ["City"] * 2 + ["Interest"] * 2, + "age": [25, 27, 35, None, None, None, None], + "interest": [None] * 5 + ["Fine Dining", "photography"], + }) + edges = pd.DataFrame({ + "s": [0, 1, 2, 0, 1, 2], + "d": [20, 20, 21, 10, 10, 11], + "rel": ["HAS_INTEREST"] * 3 + ["LIVES_IN"] * 3, + }) + g = graphistry.nodes(nodes, node_id_column).edges(edges, "s", "d") + query = ( + "MATCH (p {node_type:'Person'})-[{rel:'HAS_INTEREST'}]->(i {node_type:'Interest'}), " + "(p)-[{rel:'LIVES_IN'}]->(c {node_type:'City'}) " + "RETURN count(DISTINCT p) AS n" + ) + + assert g.gfql(query)._nodes.to_dict(orient="records") == [{"n": 3}] + + @pytest.mark.parametrize("predicate", ["p.age >= - 26", "p.age >= -(26)", "p.age >= +(26)"]) def test_t1_connected_comma_pushes_unfolded_unary_literal(predicate: str) -> None: # `NUMBER` carries its sign as a lexer terminal, so only a lexically adjacent sign From bfde19ba3c28ab48df59edc58d0d1b46932658ba Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 17:45:18 -0700 Subject: [PATCH 10/36] fix(gfql): let column dtypes decide connected-join pushdown A pushed filter_dict is schema-validated against the real column, while the row residual evaluates leniently (Cypher 3-valued logic yields no rows). Pushing a type-incompatible atom therefore turned a correct empty result into a hard error: p.age = 'foo' master [] -> GFQLSchemaError (string literal, numeric column) p.nick >= 26 master [] -> GFQLTypeError (numeric literal, string column) No value-type rule can fix this, because safety depends on the column's dtype and not the literal's. Compilation had no way to know: compile_cypher() takes no graph, so `p.age = 'foo'` and `p.nick = 'a'` are indistinguishable at lowering. Thread the node dtypes gfql() already has down to the pushdown decision, and admit an atom only when the column accepts it (mirroring compute/validate_schema.py). Without dtypes -- bare compile_cypher(), which never executes -- the value-type decision stands, so translation-only callers are unaffected. The planner now picks per atom: numeric compares, string equality, string predicates, and unary literals push with no residual; dtype-incompatible atoms fall back to the residual and answer correctly. Every probed shape now matches master, except STARTS WITH/CONTAINS/=~, which this PR makes work where master errors. Also set lowering_py_max_lines to the exact count rather than leaving headroom a later rung could absorb silently. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- bin/ci_cypher_surface_guard_baseline.json | 2 +- graphistry/compute/gfql/cypher/api.py | 3 +- graphistry/compute/gfql/cypher/lowering.py | 63 +++++++++++++++++-- graphistry/compute/gfql_unified.py | 28 ++++++++- .../compute/gfql/cypher/test_lowering.py | 45 +++++++++++++ 5 files changed, 132 insertions(+), 9 deletions(-) diff --git a/bin/ci_cypher_surface_guard_baseline.json b/bin/ci_cypher_surface_guard_baseline.json index 49c5451467..4cfd972e7c 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": 9122 + "lowering_py_max_lines": 9146 } diff --git a/graphistry/compute/gfql/cypher/api.py b/graphistry/compute/gfql/cypher/api.py index c7b660adf4..733167a38b 100644 --- a/graphistry/compute/gfql/cypher/api.py +++ b/graphistry/compute/gfql/cypher/api.py @@ -81,6 +81,7 @@ def compile_cypher( query: str, *, params: Optional[Mapping[str, Any]] = None, + _node_dtypes: Optional[Mapping[str, Any]] = None, _warn_deprecated: bool = True, ) -> Union[CompiledCypherQuery, CompiledCypherUnionQuery, CompiledCypherGraphQuery]: """Deprecated compatibility helper for inspecting compiled Cypher internals. @@ -110,4 +111,4 @@ def compile_cypher( stacklevel=2, ) parsed = parse_cypher(query) - return compile_cypher_query(parsed, params=params) + return compile_cypher_query(parsed, params=params, node_dtypes=_node_dtypes) diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index 39862eb89b..a120012453 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7820,6 +7820,33 @@ def _connected_join_expr_literal_value(node: ExprNode) -> Tuple[bool, Optional[C _CONNECTED_JOIN_STRING_OPS = frozenset({"contains", "starts_with", "ends_with", "regex"}) +_CONNECTED_JOIN_ORDERING_OPS = frozenset({"!=", "<", "<=", ">", ">="}) + + +def _connected_join_dtype_admits(op: str, value: Any, dtype: Any) -> bool: + """Whether pushing `op`/`value` onto a column of `dtype` matches residual semantics. + + A pushed `filter_dict` is schema-validated against the real column, while the row + residual evaluates leniently (Cypher 3-valued logic yields no rows). Pushing a + type-incompatible atom therefore turns a correct empty result into a hard error, so + those atoms have to stay residual. Mirrors `compute/validate_schema.py`. + """ + import pandas as _pd + + is_numeric_col = bool(_pd.api.types.is_numeric_dtype(dtype)) and not bool(_pd.api.types.is_bool_dtype(dtype)) + is_string_col = bool(_pd.api.types.is_string_dtype(dtype)) or bool(_pd.api.types.is_object_dtype(dtype)) + if op in _CONNECTED_JOIN_STRING_OPS: + return is_string_col + if op in _CONNECTED_JOIN_ORDERING_OPS: + # These lower to NumericASTPredicate, which requires a numeric column. + return is_numeric_col + if isinstance(value, str): + return not is_numeric_col + if isinstance(value, bool): + return not is_numeric_col or bool(_pd.api.types.is_bool_dtype(dtype)) + if isinstance(value, (int, float)): + return not is_string_col + return True def _connected_join_pushable_value( @@ -7827,6 +7854,8 @@ def _connected_join_pushable_value( value: Optional[CypherLiteral], *, params: Optional[Mapping[str, Any]], + column: Optional[str] = None, + node_dtypes: Optional[Mapping[str, Any]] = None, ) -> bool: """Whether `op`/`value` survives the trip into a node `filter_dict`. @@ -7856,10 +7885,20 @@ def _connected_join_pushable_value( # would execute as `> 26` instead of an equality against a map. return False if op in _CONNECTED_JOIN_STRING_OPS: - return isinstance(resolved, str) - if op == "==": - return isinstance(resolved, (str, int, float, bool)) - return isinstance(resolved, (int, float)) and not isinstance(resolved, bool) + if not isinstance(resolved, str): + return False + elif op == "==": + if not isinstance(resolved, (str, int, float, bool)): + return False + elif not (isinstance(resolved, (int, float)) and not isinstance(resolved, bool)): + return False + if node_dtypes is None: + # No schema (e.g. bare `compile_cypher()` with no graph). Keep the value-type + # decision; execution always goes through `gfql()`, which supplies dtypes. + return True + if column is None or column not in node_dtypes: + return False + return _connected_join_dtype_admits(op, resolved, node_dtypes[column]) def _apply_connected_join_node_filter( @@ -7869,8 +7908,11 @@ def _apply_connected_join_node_filter( op: str, value: Optional[CypherLiteral], params: Optional[Mapping[str, Any]], + node_dtypes: Optional[Mapping[str, Any]] = None, ) -> bool: - if not _connected_join_pushable_value(op, value, params=params): + if not _connected_join_pushable_value( + op, value, params=params, column=prop_ref.property, node_dtypes=node_dtypes + ): return False pushed = False for alias_targets in alias_targets_by_pattern: @@ -7894,6 +7936,7 @@ def _pushdown_connected_join_atom_filter( alias_targets: Mapping[str, ASTObject], *, params: Optional[Mapping[str, Any]], + node_dtypes: Optional[Mapping[str, Any]] = None, ) -> bool: try: node = _parse_row_expr( @@ -7920,6 +7963,7 @@ def _pushdown_connected_join_atom_filter( op=op, value=right_value, params=params, + node_dtypes=node_dtypes, ) reverse_op = _connected_join_literal_op(node.op, reverse=True) right_ref = _connected_join_expr_property_ref(node.right, span=expr.span) @@ -7931,6 +7975,7 @@ def _pushdown_connected_join_atom_filter( op=reverse_op, value=left_value, params=params, + node_dtypes=node_dtypes, ) return False @@ -7942,6 +7987,7 @@ def _pushdown_connected_join_where_filters( alias_targets: Mapping[str, ASTObject], *, params: Optional[Mapping[str, Any]], + node_dtypes: Optional[Mapping[str, Any]] = None, ) -> Optional[List[ExpressionText]]: if where is None: return [] @@ -7969,6 +8015,7 @@ def _pushdown_connected_join_where_filters( op=cast(str, predicate.op), value=cast(Optional[CypherLiteral], predicate.right), params=params, + node_dtypes=node_dtypes, ) residuals: List[ExpressionText] = [] @@ -7985,6 +8032,7 @@ def _walk(expr_node: BooleanExpr) -> None: alias_targets_by_pattern, alias_targets, params=params, + node_dtypes=node_dtypes, ) if not pushed: residuals.append(atom) @@ -8014,6 +8062,7 @@ def _walk(expr_node: BooleanExpr) -> None: op=cast(str, predicate.op), value=cast(Optional[CypherLiteral], predicate.right), params=params, + node_dtypes=node_dtypes, ) if pushed: continue @@ -8077,6 +8126,7 @@ def _compile_connected_match_join( *, params: Optional[Mapping[str, Any]] = None, semantic_entity_kinds: Optional[Mapping[str, Literal["node", "edge", "scalar"]]] = None, + node_dtypes: Optional[Mapping[str, Any]] = None, ) -> CompiledCypherQuery: clause = query.matches[0] pattern_chains: List[Chain] = [] @@ -8132,6 +8182,7 @@ def _compile_connected_match_join( alias_targets_by_pattern, combined_alias_targets, params=params, + node_dtypes=node_dtypes, ) if residual_filters is None: raise _unsupported( @@ -8672,6 +8723,7 @@ def compile_cypher_query( query: Union[CypherQuery, CypherUnionQuery, CypherGraphQuery], *, params: Optional[Mapping[str, Any]] = None, + node_dtypes: Optional[Mapping[str, Any]] = None, ) -> Union[CompiledCypherQuery, CompiledCypherUnionQuery, CompiledCypherGraphQuery]: from graphistry.compute.gfql.cypher import projection_planning as _projection @@ -8824,6 +8876,7 @@ def _attach_graph_context(result: CompiledCypherQuery) -> CompiledCypherQuery: query, params=params, semantic_entity_kinds=bound_context.entity_kinds, + node_dtypes=node_dtypes, ) ) if _is_connected_optional_match_query(query): diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index 0583073072..6ea513e3bc 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -1341,11 +1341,30 @@ def detect_query_type(query: Any) -> QueryType: return "single" +def _node_dtypes_for_pushdown(g: Plottable) -> Optional[Mapping[str, Any]]: + """Column dtypes used to decide whether a WHERE atom may be pushed into a filter_dict. + + A pushed filter is schema-validated against the real column while the row residual + evaluates leniently, so the planner needs dtypes to avoid turning a correct empty + result into a type error. Returns None when unavailable, which keeps the + value-type-only decision. + """ + nodes = getattr(g, "_nodes", None) + dtypes = getattr(nodes, "dtypes", None) + if dtypes is None: + return None + try: + return {str(col): dtype for col, dtype in dtypes.items()} + except Exception: + return None + + def _compile_string_query( query: str, *, language: Optional[Literal["cypher", "gremlin"]], params: Optional[Mapping[str, Any]], + node_dtypes: Optional[Mapping[str, Any]] = None, ) -> Any: query_language = language or "cypher" if query_language != "cypher": @@ -1357,7 +1376,7 @@ def _compile_string_query( suggestion="Use language='cypher' for now; Gremlin string compilation is not implemented yet.", language="gfql", ) - return compile_cypher(query, params=params, _warn_deprecated=False) + return compile_cypher(query, params=params, _node_dtypes=node_dtypes, _warn_deprecated=False) def _compile_value_repr(value: Any) -> str: @@ -1653,7 +1672,12 @@ def gfql(self: Plottable, if isinstance(query, str): query_language = language or "cypher" try: - compiled_query = _compile_string_query(query, language=language, params=params) + compiled_query = _compile_string_query( + query, + language=language, + params=params, + node_dtypes=_node_dtypes_for_pushdown(self), + ) except GFQLValidationError as exc: _fire_postcompile_policy( expanded_policy, diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 79f4ec86d1..181128616e 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -44,6 +44,7 @@ from graphistry.compute.gfql.ir.bound_ir import BoundIR, BoundQueryPart, SemanticTable from graphistry.compute.gfql.ir.compilation import PlanContext from graphistry.compute.gfql.ir.logical_plan import CHILD_SLOTS, Filter, PatternMatch, ProcedureCall as LogicalProcedureCall +from graphistry.compute.gfql_unified import _node_dtypes_for_pushdown from graphistry.Plottable import Plottable from graphistry.tests.test_compute import CGFull from graphistry.tests.compute.gfql.cypher._whole_entity_compat import entity_text_records @@ -15176,6 +15177,50 @@ def test_connected_join_pushes_string_predicates(predicate: str, expected: int) assert any("nick" in entry.get("p", {}) for entry in filters_by_alias) +@pytest.mark.parametrize( + "predicate,expected", + [ + # string literal vs numeric column, and numeric literal vs string column: the + # residual answers these leniently, so pushing them must not turn them into errors + ("p.age = 'foo'", []), + ("p.age = date('2020-01-01')", []), + ("p.nick >= 26", []), + ("p.nick < 26", []), + ("p.nick <> 26", [{"n": 3}]), + ], +) +def test_connected_join_does_not_push_dtype_incompatible_atoms(predicate: str, expected: Any) -> None: + query = _t1_nullable_query(predicate) + + result = _real_t1_nullable_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == expected + + +def test_connected_join_dtype_schema_selects_pushdown() -> None: + # With dtypes the planner pushes what the column admits and leaves the rest residual. + g = _real_t1_nullable_graph() + dtypes = _node_dtypes_for_pushdown(g) + assert dtypes is not None + + def pushed_props(predicate: str) -> set: + compiled = cast(CompiledCypherQuery, compile_cypher(_t1_nullable_query(predicate), _node_dtypes=dtypes)) + plan = _compiled_execution_extras(compiled).connected_match_join + assert plan is not None + return { + prop + for chain in plan.pattern_chains + for op in chain.chain + if isinstance(op, ASTNode) + for prop in (op.filter_dict or {}) + } - {"node_type"} + + assert pushed_props("p.age >= 26") == {"age"} + assert pushed_props("p.nick = 'a'") == {"nick"} + assert pushed_props("p.nick STARTS WITH 'a'") == {"nick"} + assert pushed_props("p.nick >= 26") == set() + assert pushed_props("p.age = 'foo'") == set() + + @pytest.mark.parametrize("node_id_column", ["id", "nid"]) def test_connected_join_count_distinct_alias_is_node_identity(node_id_column: str) -> None: # A bare alias must lower to its identity column. Left unrewritten it only resolves when From 68bde81f7130d87334d6a3866adb9a09c32f005f Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 18:03:44 -0700 Subject: [PATCH 11/36] fix(gfql): mirror the live validator in the pushdown dtype gate Independent review of bfde19ba found the gate mirrored the wrong file and left three divergences from master: p.flag = 'yes' (bool column) master [] -> GFQLSchemaError p.flag = 99999999999999999999 master range error -> raw OverflowError p.age <> 99999999999999999999 master range error -> n=4 compute/validate_schema.py is dead code -- only a docs test imports it -- and it disagrees with the validator that actually runs, compute/filter_by_dict.py, on bool: the live one counts bool as numeric. The gate's `and not is_bool_dtype(...)` carve-out therefore admitted a string against a bool column, straight into the error the gate exists to prevent. Reuse the live validator's own _is_numeric_dtype_safe / _is_string_dtype_safe so the two agree by construction and stay correct on non-pandas dtypes. The 64-bit literal guard lives on the row-expr path, so pushing an out-of-range int evaded it and reached pandas. Reject those and let the residual report the range error. _node_dtypes_for_pushdown used dtypes.items(), which only pandas and cuDF expose; polars' .dtypes is a plain list, so it raised, was swallowed, and returned None -- silently reverting every polars-backed graph to the value-type-only decision. Zip columns with dtypes, which is correct on all three. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- bin/ci_cypher_surface_guard_baseline.json | 2 +- graphistry/compute/gfql/cypher/lowering.py | 25 +++++++-- graphistry/compute/gfql_unified.py | 8 ++- .../compute/gfql/cypher/test_lowering.py | 56 +++++++++++++++++++ 4 files changed, 82 insertions(+), 9 deletions(-) diff --git a/bin/ci_cypher_surface_guard_baseline.json b/bin/ci_cypher_surface_guard_baseline.json index 4cfd972e7c..29183d2471 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": 9146 + "lowering_py_max_lines": 9159 } diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index a120012453..433944f6df 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7829,21 +7829,26 @@ def _connected_join_dtype_admits(op: str, value: Any, dtype: Any) -> bool: A pushed `filter_dict` is schema-validated against the real column, while the row residual evaluates leniently (Cypher 3-valued logic yields no rows). Pushing a type-incompatible atom therefore turns a correct empty result into a hard error, so - those atoms have to stay residual. Mirrors `compute/validate_schema.py`. + those atoms have to stay residual. + + Mirrors the validator that actually runs, `compute/filter_by_dict.py`, and reuses its + dtype helpers: they agree by construction (`bool` counts as numeric there) and stay + correct on non-pandas dtypes. `compute/validate_schema.py` looks similar but is dead + code -- only a docs test imports it -- and it disagrees on `bool`. """ - import pandas as _pd + from graphistry.compute.filter_by_dict import _is_numeric_dtype_safe, _is_string_dtype_safe - is_numeric_col = bool(_pd.api.types.is_numeric_dtype(dtype)) and not bool(_pd.api.types.is_bool_dtype(dtype)) - is_string_col = bool(_pd.api.types.is_string_dtype(dtype)) or bool(_pd.api.types.is_object_dtype(dtype)) + is_numeric_col = bool(_is_numeric_dtype_safe(dtype)) + is_string_col = bool(_is_string_dtype_safe(dtype)) if op in _CONNECTED_JOIN_STRING_OPS: return is_string_col if op in _CONNECTED_JOIN_ORDERING_OPS: - # These lower to NumericASTPredicate, which requires a numeric column. + # These lower to NumericASTPredicate, rejected on a non-numeric column. return is_numeric_col if isinstance(value, str): return not is_numeric_col if isinstance(value, bool): - return not is_numeric_col or bool(_pd.api.types.is_bool_dtype(dtype)) + return True if isinstance(value, (int, float)): return not is_string_col return True @@ -7884,6 +7889,14 @@ def _connected_join_pushable_value( # revives it via `predicates_from_json`, so a param like {'type': 'GT', 'val': 26} # would execute as `> 26` instead of an equality against a map. return False + if ( + isinstance(resolved, int) + and not isinstance(resolved, bool) + and not (_CYPHER_INT64_MIN <= resolved <= _CYPHER_INT64_MAX) + ): + # The 64-bit literal guard lives on the row-expr path, so pushing an out-of-range + # int would evade it and reach pandas, which overflows with a raw OverflowError. + return False if op in _CONNECTED_JOIN_STRING_OPS: if not isinstance(resolved, str): return False diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index 6ea513e3bc..16b9ecf2e7 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -1351,12 +1351,16 @@ def _node_dtypes_for_pushdown(g: Plottable) -> Optional[Mapping[str, Any]]: """ nodes = getattr(g, "_nodes", None) dtypes = getattr(nodes, "dtypes", None) - if dtypes is None: + columns = getattr(nodes, "columns", None) + if dtypes is None or columns is None: return None try: - return {str(col): dtype for col, dtype in dtypes.items()} + # zip rather than `dtypes.items()`: pandas/cuDF expose a column-indexed Series but + # polars exposes a plain list, and `.items()` there would silently yield no schema. + mapping = {str(col): dtype for col, dtype in zip(list(columns), list(dtypes))} except Exception: return None + return mapping or None def _compile_string_query( diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 181128616e..5627ad09ba 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -45,6 +45,7 @@ from graphistry.compute.gfql.ir.compilation import PlanContext from graphistry.compute.gfql.ir.logical_plan import CHILD_SLOTS, Filter, PatternMatch, ProcedureCall as LogicalProcedureCall from graphistry.compute.gfql_unified import _node_dtypes_for_pushdown +from graphistry.compute.gfql.cypher.lowering import _connected_join_dtype_admits from graphistry.Plottable import Plottable from graphistry.tests.test_compute import CGFull from graphistry.tests.compute.gfql.cypher._whole_entity_compat import entity_text_records @@ -15196,6 +15197,61 @@ def test_connected_join_does_not_push_dtype_incompatible_atoms(predicate: str, e assert result._nodes.to_dict(orient="records") == expected +def _real_bool_shape_graph() -> Plottable: + nodes = pd.DataFrame({ + "id": ["a", "b", "c", "d"], + "flag": pd.Series([True, False, True, False], dtype="bool"), + "age": pd.Series([25, 27, 35, 40], dtype="int64"), + }) + edges = pd.DataFrame({"s": ["a", "b", "c", "d"], "d": ["b", "c", "d", "a"]}) + return graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + +@pytest.mark.parametrize( + "predicate,expected", + [ + # `bool` counts as numeric to the live validator, so a string value against a bool + # column must stay residual rather than push into a type error. + ("p.flag = 'yes'", []), + ("p.flag = true", [{"n": 2}]), + ("p.age >= 26", [{"n": 3}]), + ], +) +def test_connected_join_bool_column_matches_validator(predicate: str, expected: Any) -> None: + query = f"MATCH (p)-[]->(x), (p)-[]->(y) WHERE {predicate} RETURN count(p) AS n" + + result = _real_bool_shape_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == expected + + +@pytest.mark.parametrize( + "predicate", + [ + "p.age = 99999999999999999999", + "p.age <> 99999999999999999999", + "p.flag = 99999999999999999999", + ], +) +def test_connected_join_out_of_range_int_still_reports_range_error(predicate: str) -> None: + # The 64-bit literal guard lives on the row-expr path; pushing an out-of-range int + # would evade it and surface a raw OverflowError from pandas instead. + query = f"MATCH (p)-[]->(x), (p)-[]->(y) WHERE {predicate} RETURN count(p) AS n" + + with pytest.raises(GFQLValidationError): + _real_bool_shape_graph().gfql(query) + + +def test_node_dtypes_for_pushdown_reads_columns_not_items() -> None: + # pandas/cuDF expose a column-indexed Series but polars exposes a plain list, so the + # mapping must be built by zipping columns with dtypes rather than calling `.items()`. + g = _real_bool_shape_graph() + dtypes = _node_dtypes_for_pushdown(g) + assert dtypes is not None + assert set(dtypes) == {"id", "flag", "age"} + assert _connected_join_dtype_admits("==", "yes", dtypes["flag"]) is False + assert _connected_join_dtype_admits("==", "yes", dtypes["id"]) is True + + def test_connected_join_dtype_schema_selects_pushdown() -> None: # With dtypes the planner pushes what the column admits and leaves the rest residual. g = _real_t1_nullable_graph() From 040c72465a8f49909ca0c9bb4d375219807fa577 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 18:24:13 -0700 Subject: [PATCH 12/36] fix(gfql): classify non-pandas dtypes so the pushdown gate works on polars The dtype gate was inert on polars-typed nodes, so the exact bug it was added for came back on them: polars nodes, WHERE p.age = 'foo' master [] -> GFQLSchemaError pd.api.types.is_numeric_dtype(pl.Int64()) returns False rather than raising, so filter_by_dict's helpers never reach the kind/text fallback that exists to handle non-pandas dtypes. Every polars column came back neither-numeric-nor-string, and `isinstance(value, str) -> return not is_numeric_col` read that as pushable. The gate's dtypes came from the polars frame while the validator saw the pandas-converted one. Classify explicitly: use the helpers, and when both say False re-derive from kind/text. Unrecognized dtypes now fail closed rather than being read as safe. Polars verdicts are now identical to pandas, so ordering pushdown -- previously rejected outright on polars, making the optimization dead there -- engages as intended. Tests cover the fallback with synthetic dtype objects so the pandas lane exercises it (the gfql lane has no polars and would skip), plus real polars cases via importorskip. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- bin/ci_cypher_surface_guard_baseline.json | 2 +- graphistry/compute/gfql/cypher/lowering.py | 38 +++++- .../compute/gfql/cypher/test_lowering.py | 109 +++++++++++++++++- 3 files changed, 143 insertions(+), 6 deletions(-) diff --git a/bin/ci_cypher_surface_guard_baseline.json b/bin/ci_cypher_surface_guard_baseline.json index 29183d2471..ecb7253f39 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": 9159 + "lowering_py_max_lines": 9189 } diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index 433944f6df..235126890b 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7823,6 +7823,35 @@ def _connected_join_expr_literal_value(node: ExprNode) -> Tuple[bool, Optional[C _CONNECTED_JOIN_ORDERING_OPS = frozenset({"!=", "<", "<=", ">", ">="}) +def _connected_join_dtype_classes(dtype: Any) -> Tuple[bool, bool]: + """Classify `dtype` as (numeric, string), across pandas / cuDF / polars. + + `filter_by_dict`'s helpers only reach their kind/text fallback when `pd.api.types` + RAISES. For a polars dtype it returns False instead, so a polars column would come back + neither-numeric-nor-string and the caller would read that as "safe to push". Re-derive + from `kind`/text when both say False, and let the caller fail closed if it stays unknown. + """ + from graphistry.compute.filter_by_dict import _is_numeric_dtype_safe, _is_string_dtype_safe + + is_numeric_col = bool(_is_numeric_dtype_safe(dtype)) + is_string_col = bool(_is_string_dtype_safe(dtype)) + if is_numeric_col or is_string_col: + return is_numeric_col, is_string_col + kind = getattr(dtype, "kind", None) + if isinstance(kind, str) and kind in {"b", "i", "u", "f", "c"}: + return True, False + text = "" + try: + text = str(dtype).lower() + except Exception: + return False, False + if any(token in text for token in ("bool", "int", "float", "double", "decimal")): + return True, False + if any(token in text for token in ("string", "utf8", "object", "categorical", "enum", "str")): + return False, True + return False, False + + def _connected_join_dtype_admits(op: str, value: Any, dtype: Any) -> bool: """Whether pushing `op`/`value` onto a column of `dtype` matches residual semantics. @@ -7836,10 +7865,11 @@ def _connected_join_dtype_admits(op: str, value: Any, dtype: Any) -> bool: correct on non-pandas dtypes. `compute/validate_schema.py` looks similar but is dead code -- only a docs test imports it -- and it disagrees on `bool`. """ - from graphistry.compute.filter_by_dict import _is_numeric_dtype_safe, _is_string_dtype_safe - - is_numeric_col = bool(_is_numeric_dtype_safe(dtype)) - is_string_col = bool(_is_string_dtype_safe(dtype)) + is_numeric_col, is_string_col = _connected_join_dtype_classes(dtype) + if not is_numeric_col and not is_string_col: + # Unrecognized dtype: fail closed. Pushing on a guess is how a correct empty + # result becomes a type error. + return False if op in _CONNECTED_JOIN_STRING_OPS: return is_string_col if op in _CONNECTED_JOIN_ORDERING_OPS: diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 5627ad09ba..0fdbbce1ff 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -45,7 +45,7 @@ from graphistry.compute.gfql.ir.compilation import PlanContext from graphistry.compute.gfql.ir.logical_plan import CHILD_SLOTS, Filter, PatternMatch, ProcedureCall as LogicalProcedureCall from graphistry.compute.gfql_unified import _node_dtypes_for_pushdown -from graphistry.compute.gfql.cypher.lowering import _connected_join_dtype_admits +from graphistry.compute.gfql.cypher.lowering import _connected_join_dtype_admits, _connected_join_dtype_classes from graphistry.Plottable import Plottable from graphistry.tests.test_compute import CGFull from graphistry.tests.compute.gfql.cypher._whole_entity_compat import entity_text_records @@ -15241,6 +15241,113 @@ def test_connected_join_out_of_range_int_still_reports_range_error(predicate: st _real_bool_shape_graph().gfql(query) +class _FakeDtype: + """Stands in for a non-pandas dtype (polars/cuDF) without importing that engine.""" + + def __init__(self, text: str, kind: Optional[str] = None) -> None: + self._text = text + if kind is not None: + self.kind = kind + + def __str__(self) -> str: + return self._text + + +class _UnprintableDtype: + def __str__(self) -> str: + raise RuntimeError("dtype has no text form") + + +@pytest.mark.parametrize( + "dtype,expected", + [ + # `pd.api.types` returns False (not raises) for these, so classification has to + # fall back to kind/text or the gate would read them as "safe to push". + (_FakeDtype("Int64"), (True, False)), + (_FakeDtype("Float64"), (True, False)), + (_FakeDtype("Boolean"), (True, False)), + (_FakeDtype("Decimal(38,10)"), (True, False)), + (_FakeDtype("String"), (False, True)), + (_FakeDtype("Utf8"), (False, True)), + (_FakeDtype("Categorical(ordering='physical')"), (False, True)), + (_FakeDtype("i-am-not-a-dtype", kind="i"), (True, False)), + # Unrecognized must fail closed, not be guessed at. + (_FakeDtype("Date"), (False, False)), + (_FakeDtype("Duration(time_unit='us')"), (False, False)), + (_UnprintableDtype(), (False, False)), + ], +) +def test_connected_join_dtype_classes_falls_back_for_non_pandas_dtypes(dtype: Any, expected: Any) -> None: + assert _connected_join_dtype_classes(dtype) == expected + + +@pytest.mark.parametrize( + "op,value,dtype", + [ + ("==", "foo", _FakeDtype("Date")), + (">=", 26, _FakeDtype("Date")), + ("contains", "o", _FakeDtype("Date")), + ], +) +def test_connected_join_unknown_dtype_never_pushes(op: str, value: Any, dtype: Any) -> None: + assert _connected_join_dtype_admits(op, value, dtype) is False + + +def test_connected_join_fake_dtype_verdicts_match_pandas() -> None: + numeric_fake, string_fake = _FakeDtype("Int64"), _FakeDtype("String") + numeric_pd, string_pd = pd.Series([1]).dtype, pd.Series(["a"]).dtype + + for numeric, string in [(numeric_pd, string_pd), (numeric_fake, string_fake)]: + assert _connected_join_dtype_admits("==", "foo", numeric) is False + assert _connected_join_dtype_admits(">=", 26, numeric) is True + assert _connected_join_dtype_admits("contains", "o", string) is True + assert _connected_join_dtype_admits("==", 26, string) is False + + +def test_connected_join_dtype_classes_handles_non_pandas_dtypes() -> None: + # `pd.api.types.is_numeric_dtype(pl.Int64())` returns False rather than raising, so + # filter_by_dict's helpers never reach their fallback and a polars column would look + # neither-numeric-nor-string -- which the gate would read as "safe to push". + pl = pytest.importorskip("polars") + + assert _connected_join_dtype_classes(pl.Int64()) == (True, False) + assert _connected_join_dtype_classes(pl.Float64()) == (True, False) + assert _connected_join_dtype_classes(pl.String()) == (False, True) + assert _connected_join_dtype_classes(pd.Series([1]).dtype) == (True, False) + assert _connected_join_dtype_classes(pd.Series(["a"]).dtype) == (False, True) + # Unrecognized dtype must fail closed rather than be treated as pushable. + assert _connected_join_dtype_classes(pl.Date()) == (False, False) + assert _connected_join_dtype_admits("==", "foo", pl.Date()) is False + + +def test_connected_join_polars_nodes_match_pandas_gate_verdicts() -> None: + pl = pytest.importorskip("polars") + + for numeric_dtype, string_dtype in [ + (pd.Series([1]).dtype, pd.Series(["a"]).dtype), + (pl.Int64(), pl.String()), + ]: + assert _connected_join_dtype_admits("==", "foo", numeric_dtype) is False + assert _connected_join_dtype_admits(">=", 26, numeric_dtype) is True + assert _connected_join_dtype_admits("contains", "o", string_dtype) is True + assert _connected_join_dtype_admits("==", 26, string_dtype) is False + + +def test_connected_join_polars_backed_graph_matches_pandas_results() -> None: + # The whole dtype gate was inert on polars-typed nodes: `p.age = 'foo'` pushed and + # raised where a pandas-backed graph correctly answers []. + pl = pytest.importorskip("polars") + + nodes = pd.DataFrame({"id": ["p1", "p2", "p3", "x1", "y1"], "age": [25, 26, 30, 1, 2]}) + edges = pd.DataFrame({"s": ["p1", "p1", "p2", "p2", "p3", "p3"], "d": ["x1", "y1", "x1", "y1", "x1", "y1"]}) + query = "MATCH (p)-->(a), (p)-->(b) WHERE p.age = 'foo' RETURN count(p) AS n" + + g_polars = graphistry.nodes(pl.from_pandas(nodes), "id").edges(pl.from_pandas(edges), "s", "d") + result = g_polars.gfql(query)._nodes + records = result.to_dict(orient="records") if hasattr(result, "to_dict") else result.to_dicts() + assert records == [] + + def test_node_dtypes_for_pushdown_reads_columns_not_items() -> None: # pandas/cuDF expose a column-indexed Series but polars exposes a plain list, so the # mapping must be built by zipping columns with dtypes rather than calling `.items()`. From 3d4122961613dd022a16c1d14b51e1c6441014c9 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 18:39:14 -0700 Subject: [PATCH 13/36] fix(gfql): fail closed on container dtypes in the pushdown gate The dtype text fallback matched substrings, and container dtypes embed their element type, so it read them as scalars: interval[int64, right] contains "int" -> numeric List(Int64) contains "int" -> numeric struct contains "str" -> string An IntervalDtype node column then pushed a comparison and crashed where master answers: WHERE p.iv > 1 master [] -> raw builtins.ValueError WHERE p.iv <> 1 master n=8 -> GFQLTypeError The underlying pandas ValueError is pre-existing -- g.chain() with GT on an interval column raises identically on master -- but pushdown made it reachable from Cypher, where master went through the residual and answered. It also leaked unwrapped, while master wraps it as a GFQL error. Test container tokens before the scalar ones so these fail closed. Exposure was low -- pd.cut yields category dtype, which already failed closed -- but the crash direction was the gate's stated contract inverted. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- bin/ci_cypher_surface_guard_baseline.json | 2 +- graphistry/compute/gfql/cypher/lowering.py | 18 +++++++++++++++ .../compute/gfql/cypher/test_lowering.py | 22 +++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/bin/ci_cypher_surface_guard_baseline.json b/bin/ci_cypher_surface_guard_baseline.json index ecb7253f39..fd975f6d80 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": 9189 + "lowering_py_max_lines": 9207 } diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index 235126890b..c224a17ea8 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7821,6 +7821,18 @@ def _connected_join_expr_literal_value(node: ExprNode) -> Tuple[bool, Optional[C _CONNECTED_JOIN_STRING_OPS = frozenset({"contains", "starts_with", "ends_with", "regex"}) _CONNECTED_JOIN_ORDERING_OPS = frozenset({"!=", "<", "<=", ">", ">="}) +# Container dtypes whose text embeds an element type, e.g. `interval[int64, right]`, +# `List(Int64)`, `struct`. Never scalar-comparable, so they must never push. +_CONNECTED_JOIN_CONTAINER_DTYPE_TOKENS = ( + "interval", + "struct", + "list", + "array", + "map", + "binary", + "void", + "record", +) def _connected_join_dtype_classes(dtype: Any) -> Tuple[bool, bool]: @@ -7845,6 +7857,12 @@ def _connected_join_dtype_classes(dtype: Any) -> Tuple[bool, bool]: text = str(dtype).lower() except Exception: return False, False + if any(token in text for token in _CONNECTED_JOIN_CONTAINER_DTYPE_TOKENS): + # Container dtypes must be tested first: their text embeds their element type, so + # `interval[int64, right]` and `List(Int64)` contain "int" and `struct` contains + # "str". Matching those as scalars pushes a numeric predicate onto a container + # column, which raises where the residual answers. + return False, False if any(token in text for token in ("bool", "int", "float", "double", "decimal")): return True, False if any(token in text for token in ("string", "utf8", "object", "categorical", "enum", "str")): diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 0fdbbce1ff..c8ef94b45c 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15271,6 +15271,15 @@ def __str__(self) -> str: (_FakeDtype("Utf8"), (False, True)), (_FakeDtype("Categorical(ordering='physical')"), (False, True)), (_FakeDtype("i-am-not-a-dtype", kind="i"), (True, False)), + # Container dtypes embed their element type, so a substring match reads + # `interval[int64, right]` / `List(Int64)` as numeric and `struct` as string. + # They are never scalar-comparable and must fail closed. + (_FakeDtype("interval[int64, right]"), (False, False)), + (_FakeDtype("List(Int64)"), (False, False)), + (_FakeDtype("Array(Float64, 2)"), (False, False)), + (_FakeDtype("struct"), (False, False)), + (_FakeDtype("Struct({'a': Int64})"), (False, False)), + (_FakeDtype("Binary"), (False, False)), # Unrecognized must fail closed, not be guessed at. (_FakeDtype("Date"), (False, False)), (_FakeDtype("Duration(time_unit='us')"), (False, False)), @@ -15304,6 +15313,19 @@ def test_connected_join_fake_dtype_verdicts_match_pandas() -> None: assert _connected_join_dtype_admits("==", 26, string) is False +@pytest.mark.parametrize("predicate,expected", [("p.iv > 1", []), ("p.iv >= 1", []), ("p.iv <> 1", [{"n": 8}]), ("p.iv = 1", [])]) +def test_connected_join_interval_column_matches_master(predicate: str, expected: Any) -> None: + # `interval[int64, right]` contains "int": classified numeric, a comparison pushed onto + # it raised a raw ValueError out of pandas where the residual answers correctly. + nodes = pd.DataFrame({"id": ["p1", "p2", "a1", "b1", "a2", "b2"]}) + nodes["iv"] = pd.arrays.IntervalArray.from_breaks([0, 1, 2, 3, 4, 5, 6]) + edges = pd.DataFrame({"s": ["p1", "p1", "p2", "p2"], "d": ["a1", "b1", "a2", "b2"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + query = f"MATCH (p)-[]->(x), (p)-[]->(y) WHERE {predicate} RETURN count(p) AS n" + + assert g.gfql(query)._nodes.to_dict(orient="records") == expected + + def test_connected_join_dtype_classes_handles_non_pandas_dtypes() -> None: # `pd.api.types.is_numeric_dtype(pl.Int64())` returns False rather than raising, so # filter_by_dict's helpers never reach their fallback and a polars column would look From bf46ea7f028cff5acf0c1f54c1c92f2e0f9438f6 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 18:55:25 -0700 Subject: [PATCH 14/36] fix(gfql): don't push onto an inline string property filter A connected comma join with a string-valued inline property map and a pushable WHERE atom on that same property hard-errored on a query master answers: MATCH (person:Person {id:'p1'})-[:L]->(city:Place), (friend:Person {nick:'aa'})-[:L]->(city) WHERE friend.nick = 'aa' RETURN count(friend) AS n master [{'n': 1}] -> GFQLTypeError: val must be numeric (int or float) | value: 'str' Pushing merges with the inline value via _merge_filter_predicates, which wraps raw scalars using comparison.eq. That serializes to {'type': 'EQ'}, but predicates/from_json.py binds the EQ tag to predicates.numeric.EQ, which admits only int/float -- so the executor raises when it rehydrates the string. That write/read registry split is pre-existing and lives well outside this PR; master never reached it because it never pushed connected-join WHERE atoms, leaving inline values as raw scalars that serialize without an EQ wrapper. Fixing the split belongs in its own change. So don't create the shape: skip pushdown when an inline map already set a non-numeric scalar on the property, and let the residual answer as before. Absent properties, numeric inline values, and already-built predicates are unaffected and still push. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- bin/ci_cypher_surface_guard_baseline.json | 2 +- graphistry/compute/gfql/cypher/lowering.py | 25 +++++++++++++ .../compute/gfql/cypher/test_lowering.py | 37 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/bin/ci_cypher_surface_guard_baseline.json b/bin/ci_cypher_surface_guard_baseline.json index fd975f6d80..d7aa112265 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": 9207 + "lowering_py_max_lines": 9232 } diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index c224a17ea8..30b46a6b45 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7962,6 +7962,25 @@ def _connected_join_pushable_value( return _connected_join_dtype_admits(op, resolved, node_dtypes[column]) +def _connected_join_mergeable_existing_filter(target: ASTNode, prop: str) -> bool: + """Whether pushing onto `prop` can merge with what an inline property map already set. + + `_merge_filter_predicates` wraps raw scalars with `comparison.eq`, which serializes to + `{'type': 'EQ'}` -- but `predicates/from_json.py` binds that tag to `predicates.numeric.EQ`, + which admits only int/float. So merging onto an existing STRING value produces a filter + that raises when the executor rehydrates it. That write/read split is not ours to fix + here; just don't create it. Nothing merges when the property is absent, and an existing + numeric or already-built predicate rehydrates fine. + """ + existing_filter = _target_filter_dict(target) or {} + if prop not in existing_filter: + return True + existing = existing_filter[prop] + if isinstance(existing, ASTPredicate): + return True + return isinstance(existing, (int, float)) and not isinstance(existing, bool) + + def _apply_connected_join_node_filter( alias_targets_by_pattern: Sequence[Mapping[str, ASTObject]], *, @@ -7975,6 +7994,12 @@ def _apply_connected_join_node_filter( op, value, params=params, column=prop_ref.property, node_dtypes=node_dtypes ): return False + for alias_targets in alias_targets_by_pattern: + target = alias_targets.get(prop_ref.alias) + if not isinstance(target, ASTNode): + continue + if not _connected_join_mergeable_existing_filter(target, prop_ref.property): + return False pushed = False for alias_targets in alias_targets_by_pattern: target = alias_targets.get(prop_ref.alias) diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index c8ef94b45c..06bbf5b065 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15326,6 +15326,43 @@ def test_connected_join_interval_column_matches_master(predicate: str, expected: assert g.gfql(query)._nodes.to_dict(orient="records") == expected +def _real_labelled_inline_graph() -> Plottable: + nodes = pd.DataFrame([ + {"id": "p1", "label__Person": True, "label__Place": False, "nick": "aa", "age": 30}, + {"id": "p2", "label__Person": True, "label__Place": False, "nick": "bb", "age": 40}, + {"id": "c1", "label__Person": False, "label__Place": True, "nick": None, "age": None}, + ]) + edges = pd.DataFrame([{"s": "p1", "d": "c1", "type": "L"}, {"s": "p2", "d": "c1", "type": "L"}]) + return graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + +@pytest.mark.parametrize( + "inline,predicate,expected", + [ + # Merging onto an existing STRING inline value wraps it with comparison.eq, which + # serializes to {'type':'EQ'} -- a tag from_json binds to the numeric-only EQ, so the + # executor raises when it rehydrates. Don't create that shape; stay residual. + ("{nick:'aa'}", "friend.nick = 'aa'", [{"n": 1}]), + ("{nick:'aa'}", "friend.nick = 'bb'", []), + # A different property never merges, and a numeric inline value rehydrates fine. + ("{nick:'aa'}", "friend.age > 20", [{"n": 1}]), + ("{age:30}", "friend.age > 20", [{"n": 1}]), + ("{}", "friend.nick = 'aa'", [{"n": 1}]), + ], +) +def test_connected_join_inline_string_property_merge_matches_master( + inline: str, predicate: str, expected: Any +) -> None: + query = ( + "MATCH (person:Person {id:'p1'})-[:L]->(city:Place), " + f"(friend:Person {inline})-[:L]->(city) " + f"WHERE {predicate} RETURN count(friend) AS n" + ) + + result = _real_labelled_inline_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == expected + + def test_connected_join_dtype_classes_handles_non_pandas_dtypes() -> None: # `pd.api.types.is_numeric_dtype(pl.Int64())` returns False rather than raising, so # filter_by_dict's helpers never reach their fallback and a polars column would look From 29a40329f040186796184ba0abf15aa061ab1590 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 19:14:25 -0700 Subject: [PATCH 15/36] fix(gfql): check both sides of a connected-join filter merge The merge guard inspected only the existing value, so it was order-dependent. filter_dict is mutated as earlier atoms in the same WHERE push, and a previously-pushed string predicate is an ASTPredicate -- which the guard green-lit -- so the next raw string merged behind it into the same broken EQ shape: WHERE p.name CONTAINS 'al' AND p.name = 'alice' -> GFQLTypeError: val must be numeric WHERE p.name = 'alice' AND p.name CONTAINS 'al' -> blocked correctly The error named no numeric anywhere in the query, and CONTAINS works standalone on this branch, so the shape is one a user would reasonably write. Check the incoming value too: `_predicate_value` keeps `==` as a raw value while every other op builds a real predicate that round-trips on its own tag, so refuse the merge when `==` carries a non-numeric. Both orders are now safe, and the first answers correctly (n=4) where master could not render the clause at all. Numeric merges still push. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- bin/ci_cypher_surface_guard_baseline.json | 2 +- graphistry/compute/gfql/cypher/lowering.py | 36 +++++++++++++------ .../compute/gfql/cypher/test_lowering.py | 34 ++++++++++++++++++ 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/bin/ci_cypher_surface_guard_baseline.json b/bin/ci_cypher_surface_guard_baseline.json index d7aa112265..008feb8916 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": 9232 + "lowering_py_max_lines": 9248 } diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index 30b46a6b45..02503e436c 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7962,23 +7962,31 @@ def _connected_join_pushable_value( return _connected_join_dtype_admits(op, resolved, node_dtypes[column]) -def _connected_join_mergeable_existing_filter(target: ASTNode, prop: str) -> bool: - """Whether pushing onto `prop` can merge with what an inline property map already set. +def _connected_join_mergeable_filter(target: ASTNode, prop: str, *, op: str, resolved: Any) -> bool: + """Whether pushing `op`/`resolved` onto `prop` can merge with what is already there. `_merge_filter_predicates` wraps raw scalars with `comparison.eq`, which serializes to `{'type': 'EQ'}` -- but `predicates/from_json.py` binds that tag to `predicates.numeric.EQ`, - which admits only int/float. So merging onto an existing STRING value produces a filter - that raises when the executor rehydrates it. That write/read split is not ours to fix - here; just don't create it. Nothing merges when the property is absent, and an existing - numeric or already-built predicate rehydrates fine. + which admits only int/float. Merging a non-numeric scalar on EITHER side therefore builds + a filter that raises when the executor rehydrates it. That write/read split is not ours to + fix here; just don't create it. + + Both sides must be checked. `filter_dict` is mutated as earlier atoms in the same WHERE + push, so the existing value may be an inline scalar OR a predicate a previous push left + behind -- checking only the existing side made this order-dependent + (`= 'a' AND CONTAINS 'x'` blocked, `CONTAINS 'x' AND = 'a'` not). """ existing_filter = _target_filter_dict(target) or {} if prop not in existing_filter: return True existing = existing_filter[prop] - if isinstance(existing, ASTPredicate): - return True - return isinstance(existing, (int, float)) and not isinstance(existing, bool) + if not (isinstance(existing, ASTPredicate) or (isinstance(existing, (int, float)) and not isinstance(existing, bool))): + return False + # `_predicate_value` keeps `==` as a raw value; every other supported op builds a real + # predicate, which round-trips on its own tag. + if op == "==" and not isinstance(resolved, (int, float)): + return False + return True def _apply_connected_join_node_filter( @@ -7994,11 +8002,19 @@ def _apply_connected_join_node_filter( op, value, params=params, column=prop_ref.property, node_dtypes=node_dtypes ): return False + try: + resolved_value = ( + _resolve_literal(cast(CypherLiteral, value), params=params, field="where") + if value is not None + else None + ) + except GFQLValidationError: + return False for alias_targets in alias_targets_by_pattern: target = alias_targets.get(prop_ref.alias) if not isinstance(target, ASTNode): continue - if not _connected_join_mergeable_existing_filter(target, prop_ref.property): + if not _connected_join_mergeable_filter(target, prop_ref.property, op=op, resolved=resolved_value): return False pushed = False for alias_targets in alias_targets_by_pattern: diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 06bbf5b065..bdc5889fdb 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15363,6 +15363,40 @@ def test_connected_join_inline_string_property_merge_matches_master( assert result._nodes.to_dict(orient="records") == expected +def _real_string_merge_graph() -> Plottable: + nodes = pd.DataFrame({ + "id": ["p1", "p2", "p3", "aa", "bb"], + "name": ["alice", "bob", "carol", "aa", "bb"], + "age": [30, 40, 50, 1, 2], + }) + edges = pd.DataFrame({ + "s": ["p1", "p1", "p2", "p2", "p3", "p3"], + "d": ["aa", "bb", "aa", "bb", "aa", "bb"], + }) + return graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + +@pytest.mark.parametrize( + "predicate,expected", + [ + # filter_dict is mutated as earlier atoms push, so a previously-pushed string + # predicate must not green-light merging a raw string behind it. Checking only the + # existing side made this order-dependent. + ("p.name CONTAINS 'al' AND p.name = 'alice'", [{"n": 4}]), + ("p.name STARTS WITH 'al' AND p.name = 'alice'", [{"n": 4}]), + ("p.name CONTAINS 'a' AND p.name CONTAINS 'l'", [{"n": 8}]), + ("p.name CONTAINS 'al'", [{"n": 4}]), + # Numeric merges are representable and must still push. + ("p.age > 20 AND p.age = 30", [{"n": 4}]), + ], +) +def test_connected_join_string_predicate_merge_matches_cypher(predicate: str, expected: Any) -> None: + query = f"MATCH (p)-[]->(a), (p)-[]->(b) WHERE {predicate} RETURN count(p) AS n" + + result = _real_string_merge_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == expected + + def test_connected_join_dtype_classes_handles_non_pandas_dtypes() -> None: # `pd.api.types.is_numeric_dtype(pl.Int64())` returns False rather than raising, so # filter_by_dict's helpers never reach their fallback and a polars column would look From b53d871c1dd7a5dc10d0e6b8a9b4697f15d7e680 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 19:35:26 -0700 Subject: [PATCH 16/36] fix(gfql): fail closed on dtypes that change class when the engine materializes The gate classifies the dtype the planner sees, but filter_by_dict validates the frame the executor materializes, and those disagree for polars Decimal -- numeric to the planner ("decimal" token), object to pandas, which the validator calls string: polars Decimal column, WHERE p.age > 25 master n=8 -> GFQLTypeError E302 polars Decimal column, WHERE p.age = 26 master n=4 -> GFQLSchemaError E302 Master answered these correctly because it never pushed. Categorical/Enum split the same way (polars Categorical -> pandas category, which classifies as neither). Drop decimal/categorical/enum from the token lists so they fall through to fail closed. The answer stays correct via the residual and only pushdown is lost -- the same trade pandas `category` already makes. The robust fix is to read dtypes after materialization so planner and validator judge the same frame; that is a larger change than this rung. Adds the first end-to-end polars test that actually pushes: every prior polars assertion was negative (asserting a rejected predicate), which is why this went unseen. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- bin/ci_cypher_surface_guard_baseline.json | 2 +- graphistry/compute/gfql/cypher/lowering.py | 10 ++++- .../compute/gfql/cypher/test_lowering.py | 44 ++++++++++++++++++- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/bin/ci_cypher_surface_guard_baseline.json b/bin/ci_cypher_surface_guard_baseline.json index 008feb8916..c6de4f5279 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": 9248 + "lowering_py_max_lines": 9254 } diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index 02503e436c..c3ffbb66b8 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7863,10 +7863,16 @@ def _connected_join_dtype_classes(dtype: Any) -> Tuple[bool, bool]: # "str". Matching those as scalars pushes a numeric predicate onto a container # column, which raises where the residual answers. return False, False - if any(token in text for token in ("bool", "int", "float", "double", "decimal")): + if any(token in text for token in ("bool", "int", "float", "double")): return True, False - if any(token in text for token in ("string", "utf8", "object", "categorical", "enum", "str")): + if any(token in text for token in ("string", "utf8", "object", "str")): return False, True + # Deliberately absent: decimal, categorical, enum. We classify the dtype the PLANNER + # sees, but `filter_by_dict` validates the frame the executor materializes, and those + # disagree for these: polars Decimal -> pandas object (numeric here, string there) and + # polars Categorical -> pandas category (string here, neither there). Failing closed + # costs pushdown and keeps the answer, which is the same trade pandas `category` + # already makes. return False, False diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index bdc5889fdb..2154e4886c 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15266,10 +15266,14 @@ def __str__(self) -> str: (_FakeDtype("Int64"), (True, False)), (_FakeDtype("Float64"), (True, False)), (_FakeDtype("Boolean"), (True, False)), - (_FakeDtype("Decimal(38,10)"), (True, False)), + # Decimal / Categorical / Enum classify differently before and after the engine + # materializes the frame (polars Decimal -> pandas object, Categorical -> category), + # and the validator judges the materialized one. Fail closed rather than guess. + (_FakeDtype("Decimal(38,10)"), (False, False)), + (_FakeDtype("Categorical(ordering='physical')"), (False, False)), + (_FakeDtype("Enum(categories=['a'])"), (False, False)), (_FakeDtype("String"), (False, True)), (_FakeDtype("Utf8"), (False, True)), - (_FakeDtype("Categorical(ordering='physical')"), (False, True)), (_FakeDtype("i-am-not-a-dtype", kind="i"), (True, False)), # Container dtypes embed their element type, so a substring match reads # `interval[int64, right]` / `List(Int64)` as numeric and `struct` as string. @@ -15406,6 +15410,9 @@ def test_connected_join_dtype_classes_handles_non_pandas_dtypes() -> None: assert _connected_join_dtype_classes(pl.Int64()) == (True, False) assert _connected_join_dtype_classes(pl.Float64()) == (True, False) assert _connected_join_dtype_classes(pl.String()) == (False, True) + # polars Decimal -> pandas object at execution, so the planner must not call it numeric + assert _connected_join_dtype_classes(pl.Decimal(10, 2)) == (False, False) + assert _connected_join_dtype_classes(pl.Categorical()) == (False, False) assert _connected_join_dtype_classes(pd.Series([1]).dtype) == (True, False) assert _connected_join_dtype_classes(pd.Series(["a"]).dtype) == (False, True) # Unrecognized dtype must fail closed rather than be treated as pushable. @@ -15426,6 +15433,39 @@ def test_connected_join_polars_nodes_match_pandas_gate_verdicts() -> None: assert _connected_join_dtype_admits("==", 26, string_dtype) is False +@pytest.mark.parametrize( + "predicate,expected", + [ + # Decimal: planner sees polars Decimal, executor materializes pandas object. Pushing + # on the planner's view raised where the residual answers. + ("p.age > 25", [{"n": 8}]), + ("p.age >= 26", [{"n": 8}]), + ("p.age != 26", [{"n": 8}]), + ("p.age = 26", [{"n": 4}]), + # A plain polars numeric column must still push and answer. + ("p.n2 >= 26", [{"n": 8}]), + ], +) +def test_connected_join_polars_decimal_matches_master(predicate: str, expected: Any) -> None: + pl = pytest.importorskip("polars") + + nodes = pl.DataFrame({ + "id": ["p1", "p2", "p3", "x1", "y1"], + "age": pl.Series([25, 26, 30, 1, 2], dtype=pl.Int64()).cast(pl.Decimal(10, 2)), + "n2": pl.Series([25, 26, 30, 1, 2], dtype=pl.Int64()), + }) + edges = pl.DataFrame({ + "s": ["p1", "p1", "p2", "p2", "p3", "p3"], + "d": ["x1", "y1", "x1", "y1", "x1", "y1"], + }) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + query = f"MATCH (p)-->(a), (p)-->(b) WHERE {predicate} RETURN count(p) AS n" + + result = g.gfql(query)._nodes + records = result.to_dict(orient="records") if hasattr(result, "to_dict") else result.to_dicts() + assert records == expected + + def test_connected_join_polars_backed_graph_matches_pandas_results() -> None: # The whole dtype gate was inert on polars-typed nodes: `p.age = 'foo'` pushed and # raised where a pandas-backed graph correctly answers []. From ba8c2cb95ac837edf5b9b8ff095765ce3612338d Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 20:12:35 -0700 Subject: [PATCH 17/36] fix(gfql): classify the dtypes the executor will actually filter Arrow-backed columns raised on queries master answers: ArrowDtype(dictionary(int32,int64)), WHERE p.year > 2000 master n=2 -> GFQLTypeError E302 ArrowDtype(bool), WHERE p.flag >= 1 master n=2 -> GFQLTypeError E302 Reachable from pd.read_parquet(dtype_backend='pyarrow') and any dictionary-encoded column. Two independent causes, and the earlier token patches only papered over the second: The gate fell back to kind/text whenever both filter_by_dict helpers returned False, but those helpers fall back only on `except`. pd.api.types.is_numeric_dtype returns False without raising both for a dtype pandas cannot parse and for one it authoritatively knows is not numeric, so the trigger cannot tell those apart -- and overriding an authoritative False is what raised here, since the executor re-applies it. With pandas nodes df_to_engine is a no-op, so planner and executor held the identical dtype object and still disagreed. Drop the fallback and defer to the helpers, so the two agree by construction. That alone would lose all pushdown on polars, whose dtypes satisfy neither helper. So classify the frame the executor materializes rather than the one the caller passed: df_to_engine(nodes.head(0), ...) preserves dtype classes, and polars Int64 -> int64 pushes while Decimal -> object correctly does not. This subsumes the decimal/categorical/enum and container token lists, which are removed: polars List materializes to object, so an ordering push is declined without a special case. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- bin/ci_cypher_surface_guard_baseline.json | 2 +- graphistry/compute/gfql/cypher/lowering.py | 59 ++----- graphistry/compute/gfql_unified.py | 21 ++- .../compute/gfql/cypher/test_lowering.py | 146 +++++------------- 4 files changed, 67 insertions(+), 161 deletions(-) diff --git a/bin/ci_cypher_surface_guard_baseline.json b/bin/ci_cypher_surface_guard_baseline.json index c6de4f5279..2d5e01ab1e 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": 9254 + "lowering_py_max_lines": 9217 } diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index c3ffbb66b8..b8f508a0d3 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7821,59 +7821,22 @@ def _connected_join_expr_literal_value(node: ExprNode) -> Tuple[bool, Optional[C _CONNECTED_JOIN_STRING_OPS = frozenset({"contains", "starts_with", "ends_with", "regex"}) _CONNECTED_JOIN_ORDERING_OPS = frozenset({"!=", "<", "<=", ">", ">="}) -# Container dtypes whose text embeds an element type, e.g. `interval[int64, right]`, -# `List(Int64)`, `struct`. Never scalar-comparable, so they must never push. -_CONNECTED_JOIN_CONTAINER_DTYPE_TOKENS = ( - "interval", - "struct", - "list", - "array", - "map", - "binary", - "void", - "record", -) - - def _connected_join_dtype_classes(dtype: Any) -> Tuple[bool, bool]: - """Classify `dtype` as (numeric, string), across pandas / cuDF / polars. + """Classify `dtype` as (numeric, string) using the validator's own helpers, verbatim. - `filter_by_dict`'s helpers only reach their kind/text fallback when `pd.api.types` - RAISES. For a polars dtype it returns False instead, so a polars column would come back - neither-numeric-nor-string and the caller would read that as "safe to push". Re-derive - from `kind`/text when both say False, and let the caller fail closed if it stays unknown. + `_node_dtypes_for_pushdown` hands us the dtype the executor will actually filter on, so + asking `filter_by_dict`'s helpers is asking the executor itself -- planner and validator + agree by construction, and anything they both decline fails closed at the caller. + + Deliberately no kind/text fallback. `pd.api.types.is_numeric_dtype` returns False without + raising both for a dtype pandas cannot parse and for one it authoritatively knows is not + numeric, so a "both False" fallback cannot tell those apart -- and overriding the + authoritative answer is what made Arrow bool/dictionary columns raise where the residual + answers. """ from graphistry.compute.filter_by_dict import _is_numeric_dtype_safe, _is_string_dtype_safe - is_numeric_col = bool(_is_numeric_dtype_safe(dtype)) - is_string_col = bool(_is_string_dtype_safe(dtype)) - if is_numeric_col or is_string_col: - return is_numeric_col, is_string_col - kind = getattr(dtype, "kind", None) - if isinstance(kind, str) and kind in {"b", "i", "u", "f", "c"}: - return True, False - text = "" - try: - text = str(dtype).lower() - except Exception: - return False, False - if any(token in text for token in _CONNECTED_JOIN_CONTAINER_DTYPE_TOKENS): - # Container dtypes must be tested first: their text embeds their element type, so - # `interval[int64, right]` and `List(Int64)` contain "int" and `struct` contains - # "str". Matching those as scalars pushes a numeric predicate onto a container - # column, which raises where the residual answers. - return False, False - if any(token in text for token in ("bool", "int", "float", "double")): - return True, False - if any(token in text for token in ("string", "utf8", "object", "str")): - return False, True - # Deliberately absent: decimal, categorical, enum. We classify the dtype the PLANNER - # sees, but `filter_by_dict` validates the frame the executor materializes, and those - # disagree for these: polars Decimal -> pandas object (numeric here, string there) and - # polars Categorical -> pandas category (string here, neither there). Failing closed - # costs pushdown and keeps the answer, which is the same trade pandas `category` - # already makes. - return False, False + return bool(_is_numeric_dtype_safe(dtype)), bool(_is_string_dtype_safe(dtype)) def _connected_join_dtype_admits(op: str, value: Any, dtype: Any) -> bool: diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index 16b9ecf2e7..4a8b9b0f53 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -1341,7 +1341,10 @@ def detect_query_type(query: Any) -> QueryType: return "single" -def _node_dtypes_for_pushdown(g: Plottable) -> Optional[Mapping[str, Any]]: +def _node_dtypes_for_pushdown( + g: Plottable, + engine: Union[EngineAbstract, str] = EngineAbstract.AUTO, +) -> Optional[Mapping[str, Any]]: """Column dtypes used to decide whether a WHERE atom may be pushed into a filter_dict. A pushed filter is schema-validated against the real column while the row residual @@ -1350,14 +1353,16 @@ def _node_dtypes_for_pushdown(g: Plottable) -> Optional[Mapping[str, Any]]: value-type-only decision. """ nodes = getattr(g, "_nodes", None) - dtypes = getattr(nodes, "dtypes", None) - columns = getattr(nodes, "columns", None) - if dtypes is None or columns is None: + if nodes is None: return None try: - # zip rather than `dtypes.items()`: pandas/cuDF expose a column-indexed Series but - # polars exposes a plain list, and `.items()` there would silently yield no schema. - mapping = {str(col): dtype for col, dtype in zip(list(columns), list(dtypes))} + # Classify the frame the EXECUTOR will filter, not the one the caller handed in. + # `filter_by_dict` validates post-materialization dtypes, so judging the pre-conversion + # frame lets the two disagree (polars Decimal reads numeric here, object there). + # `head(0)` preserves dtype classes, so an empty probe is enough. + probe = df_to_engine(nodes.head(0), resolve_engine(cast(Any, engine), nodes), warn=False) + # zip rather than `.items()`: pandas/cuDF expose a column-indexed Series, polars a list. + mapping = {str(col): dtype for col, dtype in zip(list(probe.columns), list(probe.dtypes))} except Exception: return None return mapping or None @@ -1680,7 +1685,7 @@ def gfql(self: Plottable, query, language=language, params=params, - node_dtypes=_node_dtypes_for_pushdown(self), + node_dtypes=_node_dtypes_for_pushdown(self, engine), ) except GFQLValidationError as exc: _fire_postcompile_policy( diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 2154e4886c..7e6571a662 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15241,83 +15241,10 @@ def test_connected_join_out_of_range_int_still_reports_range_error(predicate: st _real_bool_shape_graph().gfql(query) -class _FakeDtype: - """Stands in for a non-pandas dtype (polars/cuDF) without importing that engine.""" - - def __init__(self, text: str, kind: Optional[str] = None) -> None: - self._text = text - if kind is not None: - self.kind = kind - - def __str__(self) -> str: - return self._text - - -class _UnprintableDtype: - def __str__(self) -> str: - raise RuntimeError("dtype has no text form") - - @pytest.mark.parametrize( - "dtype,expected", - [ - # `pd.api.types` returns False (not raises) for these, so classification has to - # fall back to kind/text or the gate would read them as "safe to push". - (_FakeDtype("Int64"), (True, False)), - (_FakeDtype("Float64"), (True, False)), - (_FakeDtype("Boolean"), (True, False)), - # Decimal / Categorical / Enum classify differently before and after the engine - # materializes the frame (polars Decimal -> pandas object, Categorical -> category), - # and the validator judges the materialized one. Fail closed rather than guess. - (_FakeDtype("Decimal(38,10)"), (False, False)), - (_FakeDtype("Categorical(ordering='physical')"), (False, False)), - (_FakeDtype("Enum(categories=['a'])"), (False, False)), - (_FakeDtype("String"), (False, True)), - (_FakeDtype("Utf8"), (False, True)), - (_FakeDtype("i-am-not-a-dtype", kind="i"), (True, False)), - # Container dtypes embed their element type, so a substring match reads - # `interval[int64, right]` / `List(Int64)` as numeric and `struct` as string. - # They are never scalar-comparable and must fail closed. - (_FakeDtype("interval[int64, right]"), (False, False)), - (_FakeDtype("List(Int64)"), (False, False)), - (_FakeDtype("Array(Float64, 2)"), (False, False)), - (_FakeDtype("struct"), (False, False)), - (_FakeDtype("Struct({'a': Int64})"), (False, False)), - (_FakeDtype("Binary"), (False, False)), - # Unrecognized must fail closed, not be guessed at. - (_FakeDtype("Date"), (False, False)), - (_FakeDtype("Duration(time_unit='us')"), (False, False)), - (_UnprintableDtype(), (False, False)), - ], -) -def test_connected_join_dtype_classes_falls_back_for_non_pandas_dtypes(dtype: Any, expected: Any) -> None: - assert _connected_join_dtype_classes(dtype) == expected - - -@pytest.mark.parametrize( - "op,value,dtype", - [ - ("==", "foo", _FakeDtype("Date")), - (">=", 26, _FakeDtype("Date")), - ("contains", "o", _FakeDtype("Date")), - ], + "predicate,expected", + [("p.iv > 1", []), ("p.iv >= 1", []), ("p.iv <> 1", [{"n": 8}]), ("p.iv = 1", [])], ) -def test_connected_join_unknown_dtype_never_pushes(op: str, value: Any, dtype: Any) -> None: - assert _connected_join_dtype_admits(op, value, dtype) is False - - -def test_connected_join_fake_dtype_verdicts_match_pandas() -> None: - numeric_fake, string_fake = _FakeDtype("Int64"), _FakeDtype("String") - numeric_pd, string_pd = pd.Series([1]).dtype, pd.Series(["a"]).dtype - - for numeric, string in [(numeric_pd, string_pd), (numeric_fake, string_fake)]: - assert _connected_join_dtype_admits("==", "foo", numeric) is False - assert _connected_join_dtype_admits(">=", 26, numeric) is True - assert _connected_join_dtype_admits("contains", "o", string) is True - assert _connected_join_dtype_admits("==", 26, string) is False - - -@pytest.mark.parametrize("predicate,expected", [("p.iv > 1", []), ("p.iv >= 1", []), ("p.iv <> 1", [{"n": 8}]), ("p.iv = 1", [])]) def test_connected_join_interval_column_matches_master(predicate: str, expected: Any) -> None: # `interval[int64, right]` contains "int": classified numeric, a comparison pushed onto # it raised a raw ValueError out of pandas where the residual answers correctly. @@ -15401,48 +15328,59 @@ def test_connected_join_string_predicate_merge_matches_cypher(predicate: str, ex assert result._nodes.to_dict(orient="records") == expected -def test_connected_join_dtype_classes_handles_non_pandas_dtypes() -> None: - # `pd.api.types.is_numeric_dtype(pl.Int64())` returns False rather than raising, so - # filter_by_dict's helpers never reach their fallback and a polars column would look - # neither-numeric-nor-string -- which the gate would read as "safe to push". +def test_node_dtypes_for_pushdown_reports_materialized_dtypes() -> None: + # The executor filters the materialized frame, so classifying the caller's polars dtypes + # would both miss pushdown (polars Int64 satisfies neither helper) and risk disagreeing + # with the validator. Report what the executor will actually see. pl = pytest.importorskip("polars") - assert _connected_join_dtype_classes(pl.Int64()) == (True, False) - assert _connected_join_dtype_classes(pl.Float64()) == (True, False) - assert _connected_join_dtype_classes(pl.String()) == (False, True) - # polars Decimal -> pandas object at execution, so the planner must not call it numeric - assert _connected_join_dtype_classes(pl.Decimal(10, 2)) == (False, False) - assert _connected_join_dtype_classes(pl.Categorical()) == (False, False) - assert _connected_join_dtype_classes(pd.Series([1]).dtype) == (True, False) - assert _connected_join_dtype_classes(pd.Series(["a"]).dtype) == (False, True) - # Unrecognized dtype must fail closed rather than be treated as pushable. - assert _connected_join_dtype_classes(pl.Date()) == (False, False) - assert _connected_join_dtype_admits("==", "foo", pl.Date()) is False - - -def test_connected_join_polars_nodes_match_pandas_gate_verdicts() -> None: - pl = pytest.importorskip("polars") + nodes = pl.DataFrame({ + "id": ["a", "b"], + "age": pl.Series([1, 2], dtype=pl.Int64()), + "s": pl.Series(["x", "y"]), + }) + g = graphistry.nodes(nodes, "id").edges(pl.DataFrame({"s": ["a"], "d": ["b"]}), "s", "d") - for numeric_dtype, string_dtype in [ - (pd.Series([1]).dtype, pd.Series(["a"]).dtype), - (pl.Int64(), pl.String()), + dtypes = _node_dtypes_for_pushdown(g) + assert dtypes is not None + # Materialized to pandas, so the numeric column classifies as numeric and still pushes. + assert _connected_join_dtype_classes(dtypes["age"]) == (True, False) + assert _connected_join_dtype_classes(dtypes["s"]) == (False, True) + # The raw polars dtype satisfies neither helper, which is why it must not be used. + assert _connected_join_dtype_classes(pl.Int64()) == (False, False) + + +def test_connected_join_dtype_classes_defers_to_the_live_validator() -> None: + # The gate must not second-guess filter_by_dict: it validates whatever + # `_node_dtypes_for_pushdown` classified, so any disagreement turns a correct answer into + # a type error. Asking its own helpers makes them agree by construction. + from graphistry.compute.filter_by_dict import _is_numeric_dtype_safe, _is_string_dtype_safe + import pyarrow as pa + + for dtype in [ + pd.Series([1]).dtype, + pd.Series(["a"]).dtype, + pd.Series([True]).dtype, + pd.ArrowDtype(pa.bool_()), + pd.ArrowDtype(pa.dictionary(pa.int32(), pa.int64())), + pd.CategoricalDtype(["a"]), + pd.IntervalDtype("int64"), ]: - assert _connected_join_dtype_admits("==", "foo", numeric_dtype) is False - assert _connected_join_dtype_admits(">=", 26, numeric_dtype) is True - assert _connected_join_dtype_admits("contains", "o", string_dtype) is True - assert _connected_join_dtype_admits("==", 26, string_dtype) is False + assert _connected_join_dtype_classes(dtype) == ( + bool(_is_numeric_dtype_safe(dtype)), + bool(_is_string_dtype_safe(dtype)), + ) @pytest.mark.parametrize( "predicate,expected", [ - # Decimal: planner sees polars Decimal, executor materializes pandas object. Pushing - # on the planner's view raised where the residual answers. + # Decimal materializes to pandas object, so ordering must not push; the residual answers. ("p.age > 25", [{"n": 8}]), ("p.age >= 26", [{"n": 8}]), ("p.age != 26", [{"n": 8}]), ("p.age = 26", [{"n": 4}]), - # A plain polars numeric column must still push and answer. + # A plain polars numeric column materializes to int64 and must still push and answer. ("p.n2 >= 26", [{"n": 8}]), ], ) From ebd94daef641decbe5bd34d584f3ac0b17d31d18 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 20:38:04 -0700 Subject: [PATCH 18/36] fix(gfql): fail closed when the schema can't be read, and read frames without head() pyarrow.Table nodes are supported and master executes against them, but pa.Table has no .head(), so the dtype probe raised, the bare except swallowed it, and the gate returned None. None means "no graph, use value-type rules", so it pushed dtype-blind -- the exact bug class the gate exists to prevent: arrow nodes, WHERE p.name = 5 master [] -> GFQLSchemaError Two conflations, both fixed: None answered both "there is no graph" and "we could not read the graph we have". The first is safe: a bare compile_cypher() has no dtypes and value-type rules apply. The second must fail closed. Now only a missing _nodes returns None; a schema we failed to read returns an empty mapping, which fails every column lookup and falls back to the residual. `return mapping or None` also turned an empty read into the unsafe answer. The probe assumed .head(). Fall back to the frame itself when absent -- df_to_engine converts a pa.Table to pandas, so the gate now reads real dtypes for arrow nodes. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql_unified.py | 14 +++++--- .../compute/gfql/cypher/test_lowering.py | 33 +++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index 4a8b9b0f53..20bf40c987 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -1354,18 +1354,22 @@ def _node_dtypes_for_pushdown( """ nodes = getattr(g, "_nodes", None) if nodes is None: + # No graph at all: the caller falls back to value-type rules, which is what a bare + # `compile_cypher()` gets. Distinct from failing to read a graph we do have -- + # returning None for that would push dtype-blind, the very bug this gate exists for. return None try: # Classify the frame the EXECUTOR will filter, not the one the caller handed in. # `filter_by_dict` validates post-materialization dtypes, so judging the pre-conversion # frame lets the two disagree (polars Decimal reads numeric here, object there). - # `head(0)` preserves dtype classes, so an empty probe is enough. - probe = df_to_engine(nodes.head(0), resolve_engine(cast(Any, engine), nodes), warn=False) + probe = nodes.head(0) if hasattr(nodes, "head") else nodes + probe = df_to_engine(probe, resolve_engine(cast(Any, engine), nodes), warn=False) # zip rather than `.items()`: pandas/cuDF expose a column-indexed Series, polars a list. - mapping = {str(col): dtype for col, dtype in zip(list(probe.columns), list(probe.dtypes))} + return {str(col): dtype for col, dtype in zip(list(probe.columns), list(probe.dtypes))} except Exception: - return None - return mapping or None + # We have a graph but could not read its schema, so we know nothing about any column. + # An empty mapping fails every column lookup, which fails closed to the residual. + return {} def _compile_string_query( diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 7e6571a662..8054d2efe8 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15350,6 +15350,39 @@ def test_node_dtypes_for_pushdown_reports_materialized_dtypes() -> None: assert _connected_join_dtype_classes(pl.Int64()) == (False, False) +def test_node_dtypes_for_pushdown_reads_frames_without_head() -> None: + # pyarrow.Table has no .head(); the probe raised, was swallowed, and returned None -- + # which means "no graph, use value-type rules" and pushes dtype-blind, the exact bug the + # gate exists to stop. Absent graph and unreadable schema must not be conflated. + pa = pytest.importorskip("pyarrow") + + nodes = pa.table({"id": ["a", "b", "c"], "name": ["ann", "bob", "cid"], "age": [30, 40, 50]}) + edges = pa.table({"s": ["a", "a", "b", "b"], "d": ["b", "c", "a", "c"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + dtypes = _node_dtypes_for_pushdown(g) + assert dtypes is not None + assert _connected_join_dtype_classes(dtypes["name"]) == (False, True) + assert _connected_join_dtype_classes(dtypes["age"]) == (True, False) + + +def test_arrow_table_nodes_match_master_results() -> None: + pa = pytest.importorskip("pyarrow") + + nodes = pa.table({"id": ["a", "b", "c"], "name": ["ann", "bob", "cid"], "age": [30, 40, 50]}) + edges = pa.table({"s": ["a", "a", "b", "b"], "d": ["b", "c", "a", "c"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + def run(predicate: str) -> Any: + query = f"MATCH (p)-[]->(x), (p)-[]->(y) WHERE {predicate} RETURN count(p) AS n" + out = g.gfql(query)._nodes + return out.to_dict(orient="records") if hasattr(out, "to_dict") else out.to_dicts() + + assert run("p.name = 5") == [] + assert run("p.name = 'ann'") == [{"n": 4}] + assert run("p.age >= 40") == [{"n": 4}] + + def test_connected_join_dtype_classes_defers_to_the_live_validator() -> None: # The gate must not second-guess filter_by_dict: it validates whatever # `_node_dtypes_for_pushdown` classified, so any disagreement turns a correct answer into From 6b0d2a496eeefed8e805e6b00aaaf209bcaaa412 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 20:45:51 -0700 Subject: [PATCH 19/36] fix(gfql): classify the converted frame, not an empty probe polars -> pandas conversion is data-dependent, so an empty probe reports a different class than the real thing: a nullable Boolean is `bool` at head(0) and `object` once converted. The gate called it numeric while the executor filtered an object column: polars nodes, nullable bool, WHERE p.flag > 0 master n=4 -> GFQLTypeError The claim that head(0) preserves dtype classes was wrong. Nullable Int64 widens int64 -> float64, which stays numeric and is harmless, but Boolean crosses the class boundary. Convert the whole frame instead of guessing which conversions are data-dependent. For pandas nodes df_to_engine is identity and costs nothing; for others it is one conversion the executor performs anyway. No heuristic to get wrong. The old test asserted probe dtypes in isolation and would pass with this bug present. It is replaced by one that compares against the full conversion -- the property that actually failed -- and asserts the empty probe disagrees. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql_unified.py | 8 ++- .../compute/gfql/cypher/test_lowering.py | 56 ++++++++++++++----- 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index 20bf40c987..db9d88bc9a 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -1362,8 +1362,12 @@ def _node_dtypes_for_pushdown( # Classify the frame the EXECUTOR will filter, not the one the caller handed in. # `filter_by_dict` validates post-materialization dtypes, so judging the pre-conversion # frame lets the two disagree (polars Decimal reads numeric here, object there). - probe = nodes.head(0) if hasattr(nodes, "head") else nodes - probe = df_to_engine(probe, resolve_engine(cast(Any, engine), nodes), warn=False) + # + # Convert the whole frame, not an empty probe: polars -> pandas is DATA-dependent, so + # `head(0)` reports a different class than the real conversion for a nullable Boolean + # (`bool` empty, `object` with a null in it). For pandas nodes this is identity and + # costs nothing; for others it is one conversion the executor performs anyway. + probe = df_to_engine(nodes, resolve_engine(cast(Any, engine), nodes), warn=False) # zip rather than `.items()`: pandas/cuDF expose a column-indexed Series, polars a list. return {str(col): dtype for col, dtype in zip(list(probe.columns), list(probe.dtypes))} except Exception: diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 8054d2efe8..b77f232ec2 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15328,26 +15328,52 @@ def test_connected_join_string_predicate_merge_matches_cypher(predicate: str, ex assert result._nodes.to_dict(orient="records") == expected -def test_node_dtypes_for_pushdown_reports_materialized_dtypes() -> None: - # The executor filters the materialized frame, so classifying the caller's polars dtypes - # would both miss pushdown (polars Int64 satisfies neither helper) and risk disagreeing - # with the validator. Report what the executor will actually see. +def test_node_dtypes_for_pushdown_matches_the_full_conversion() -> None: + # polars -> pandas is DATA-dependent: an empty probe reports `bool` for a nullable Boolean + # while the real conversion yields `object`. The gate must report what the executor sees, + # so compare against the full conversion rather than asserting probe dtypes in isolation. pl = pytest.importorskip("polars") + from graphistry.Engine import df_to_engine, resolve_engine nodes = pl.DataFrame({ - "id": ["a", "b"], - "age": pl.Series([1, 2], dtype=pl.Int64()), - "s": pl.Series(["x", "y"]), + "id": ["a", "b", "c", "d"], + "flag": pl.Series([True, False, None, True], dtype=pl.Boolean), + "age": pl.Series([1, 2, None, 4], dtype=pl.Int64), + "name": pl.Series(["w", "x", "y", "z"]), }) - g = graphistry.nodes(nodes, "id").edges(pl.DataFrame({"s": ["a"], "d": ["b"]}), "s", "d") + edges = pl.DataFrame({"s": ["a", "a"], "d": ["b", "c"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") - dtypes = _node_dtypes_for_pushdown(g) - assert dtypes is not None - # Materialized to pandas, so the numeric column classifies as numeric and still pushes. - assert _connected_join_dtype_classes(dtypes["age"]) == (True, False) - assert _connected_join_dtype_classes(dtypes["s"]) == (False, True) - # The raw polars dtype satisfies neither helper, which is why it must not be used. - assert _connected_join_dtype_classes(pl.Int64()) == (False, False) + reported = _node_dtypes_for_pushdown(g) + assert reported is not None + executed = df_to_engine(nodes, resolve_engine("auto", nodes), warn=False) + for column, dtype in zip(list(executed.columns), list(executed.dtypes)): + assert _connected_join_dtype_classes(reported[str(column)]) == _connected_join_dtype_classes(dtype) + # The empty probe disagrees on the nullable bool, which is why it cannot be used. + empty = df_to_engine(nodes.head(0), resolve_engine("auto", nodes), warn=False) + empty_dtypes = dict(zip([str(name) for name in empty.columns], list(empty.dtypes))) + assert _connected_join_dtype_classes(empty_dtypes["flag"]) != _connected_join_dtype_classes(reported["flag"]) + + +@pytest.mark.parametrize( + "predicate,expected", + [("p.flag > 0", [{"n": 4}]), ("p.flag = true", [{"n": 4}]), ("p.age >= 2", [{"n": 4}])], +) +def test_connected_join_polars_nullable_columns_match_master(predicate: str, expected: Any) -> None: + pl = pytest.importorskip("polars") + + nodes = pl.DataFrame({ + "id": ["a", "b", "c", "d"], + "flag": pl.Series([True, False, None, True], dtype=pl.Boolean), + "age": pl.Series([1, 2, None, 4], dtype=pl.Int64), + }) + edges = pl.DataFrame({"s": ["a", "a", "b", "b"], "d": ["b", "c", "a", "c"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + query = f"MATCH (p)-[]->(x), (p)-[]->(y) WHERE {predicate} RETURN count(p) AS n" + + out = g.gfql(query)._nodes + records = out.to_dict(orient="records") if hasattr(out, "to_dict") else out.to_dicts() + assert records == expected def test_node_dtypes_for_pushdown_reads_frames_without_head() -> None: From a7f51a08c10d211bb61a3f334c5eb6e3a4d9bd7d Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 21:05:27 -0700 Subject: [PATCH 20/36] perf(gfql): read node dtypes only when pushdown asks for them Reading dtypes converts the whole nodes frame, and the gate was evaluated eagerly as a call argument for every string gfql(). Only connected-join pushdown consumes it, and that path needs an aggregate RETURN, so most queries paid for a frame that was immediately discarded: 2M-row polars nodes, MATCH (n) WHERE n.age > 900 RETURN n 187ms -> 307ms (+64%) The conversion is linear (~65ms per million polars rows) and hits every query on a polars-backed lane. pandas is unaffected -- df_to_engine is identity there. The comment claiming it was "one conversion the executor performs anyway" was wrong: the probe's frame is discarded and the executor converts again, so it was 2x, not free. Return a lazy Mapping that materializes on first lookup and caches. Callers are unchanged, queries that never reach pushdown pay ~0.01ms, and the full-frame read -- which is what makes the dtype classes correct -- still happens whenever a decision actually needs it. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql_unified.py | 58 +++++++++++++++---- .../compute/gfql/cypher/test_lowering.py | 31 ++++++++++ 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index db9d88bc9a..d601d45e9a 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -1341,23 +1341,60 @@ def detect_query_type(query: Any) -> QueryType: return "single" +class _LazyNodeDtypes(Mapping[str, Any]): + """Node dtypes, materialized on first lookup. + + Reading them costs a full engine conversion, and only connected-join pushdown ever asks -- + a path most queries never reach. Computing eagerly charged every string query for a frame + it then discarded (~65ms per million polars rows). Stay a Mapping so callers are unchanged. + """ + + def __init__(self, g: Plottable, engine: Union[EngineAbstract, str]) -> None: + self._g = g + self._engine = engine + self._resolved: Optional[Mapping[str, Any]] = None + + def _materialize(self) -> Mapping[str, Any]: + if self._resolved is None: + self._resolved = _read_node_dtypes(self._g, self._engine) + return self._resolved + + def __getitem__(self, key: str) -> Any: + return self._materialize()[key] + + def __iter__(self) -> Any: + return iter(self._materialize()) + + def __len__(self) -> int: + return len(self._materialize()) + + def _node_dtypes_for_pushdown( g: Plottable, engine: Union[EngineAbstract, str] = EngineAbstract.AUTO, ) -> Optional[Mapping[str, Any]]: - """Column dtypes used to decide whether a WHERE atom may be pushed into a filter_dict. + """Node dtypes for the pushdown gate, or None when there is no graph to read.""" + if getattr(g, "_nodes", None) is None: + # No graph: the caller falls back to value-type rules, as a bare compile_cypher() does. + # Distinct from failing to read a graph we have, which must fail closed. + return None + return _LazyNodeDtypes(g, engine) + + +def _read_node_dtypes( + g: Plottable, + engine: Union[EngineAbstract, str] = EngineAbstract.AUTO, +) -> Mapping[str, Any]: + """Column dtypes the executor will filter on, for the pushdown gate. A pushed filter is schema-validated against the real column while the row residual - evaluates leniently, so the planner needs dtypes to avoid turning a correct empty - result into a type error. Returns None when unavailable, which keeps the - value-type-only decision. + evaluates leniently, so the planner needs dtypes to avoid turning a correct empty result + into a type error. An unreadable schema yields an empty mapping, which fails every column + lookup and so falls back to the residual. """ nodes = getattr(g, "_nodes", None) if nodes is None: - # No graph at all: the caller falls back to value-type rules, which is what a bare - # `compile_cypher()` gets. Distinct from failing to read a graph we do have -- - # returning None for that would push dtype-blind, the very bug this gate exists for. - return None + return {} try: # Classify the frame the EXECUTOR will filter, not the one the caller handed in. # `filter_by_dict` validates post-materialization dtypes, so judging the pre-conversion @@ -1365,8 +1402,9 @@ def _node_dtypes_for_pushdown( # # Convert the whole frame, not an empty probe: polars -> pandas is DATA-dependent, so # `head(0)` reports a different class than the real conversion for a nullable Boolean - # (`bool` empty, `object` with a null in it). For pandas nodes this is identity and - # costs nothing; for others it is one conversion the executor performs anyway. + # (`bool` empty, `object` with a null in it). Identity for pandas; for other engines + # this costs a real conversion, which is why the caller defers it until a pushdown + # decision actually needs it. probe = df_to_engine(nodes, resolve_engine(cast(Any, engine), nodes), warn=False) # zip rather than `.items()`: pandas/cuDF expose a column-indexed Series, polars a list. return {str(col): dtype for col, dtype in zip(list(probe.columns), list(probe.dtypes))} diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index b77f232ec2..8cd2700513 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15328,6 +15328,37 @@ def test_connected_join_string_predicate_merge_matches_cypher(predicate: str, ex assert result._nodes.to_dict(orient="records") == expected +def test_node_dtypes_for_pushdown_defers_the_conversion() -> None: + # Reading dtypes costs a full engine conversion, and only connected-join pushdown asks -- + # a path most queries never reach. Computing eagerly charged every string query for a + # frame it discarded. Nothing may convert until a lookup actually happens. + pl = pytest.importorskip("polars") + + converted = [] + nodes = pl.DataFrame({"id": ["a", "b"], "age": pl.Series([1, 2], dtype=pl.Int64)}) + g = graphistry.nodes(nodes, "id").edges(pl.DataFrame({"s": ["a"], "d": ["b"]}), "s", "d") + + import graphistry.compute.gfql_unified as unified + + original = unified._read_node_dtypes + + def spy(*args: Any, **kwargs: Any) -> Any: + converted.append(1) + return original(*args, **kwargs) + + unified._read_node_dtypes = spy + try: + dtypes = unified._node_dtypes_for_pushdown(g) + assert dtypes is not None + assert converted == [] # built, but nothing read yet + assert _connected_join_dtype_classes(dtypes["age"]) == (True, False) + assert converted == [1] # first lookup materialized + _ = dtypes["id"] + assert converted == [1] # and it is cached + finally: + unified._read_node_dtypes = original + + def test_node_dtypes_for_pushdown_matches_the_full_conversion() -> None: # polars -> pandas is DATA-dependent: an empty probe reports `bool` for a nullable Boolean # while the real conversion yields `object`. The gate must report what the executor sees, From c48227a66093e35e8faa251ecf8c48e5691b6d73 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 21:10:33 -0700 Subject: [PATCH 21/36] test(gfql): pin that an unreadable schema fails closed Reverting the empty-mapping fallback to None survived all 1275 tests: the two are observationally equivalent end-to-end, since a frame whose schema cannot be read also fails execution the same way either way. But the distinction is load-bearing -- None means "no graph, use value-type rules" and pushes dtype-blind -- so assert it directly. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- .../compute/gfql/cypher/test_lowering.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 8cd2700513..e007d2ec43 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15328,6 +15328,27 @@ def test_connected_join_string_predicate_merge_matches_cypher(predicate: str, ex assert result._nodes.to_dict(orient="records") == expected +def test_node_dtypes_for_pushdown_fails_closed_on_unreadable_nodes() -> None: + # An unreadable schema must yield an empty mapping, not None: None means "no graph, use + # value-type rules" and would push dtype-blind. Pinned directly because the two are + # observationally equivalent end-to-end (such frames fail execution either way). + class _Unreadable: + columns = ["id"] + + @property + def dtypes(self) -> Any: + raise RuntimeError("schema unavailable") + + g = graphistry.nodes(pd.DataFrame({"id": ["a"]}), "id").edges( + pd.DataFrame({"s": ["a"], "d": ["a"]}), "s", "d" + ) + object.__setattr__(g, "_nodes", _Unreadable()) + + dtypes = _node_dtypes_for_pushdown(g) + assert dtypes is not None, "unreadable schema must not fall back to dtype-blind pushing" + assert dict(dtypes) == {} + + def test_node_dtypes_for_pushdown_defers_the_conversion() -> None: # Reading dtypes costs a full engine conversion, and only connected-join pushdown asks -- # a path most queries never reach. Computing eagerly charged every string query for a From 8761f6706f33d505f7b106c8d7eb742409406344 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 21:28:10 -0700 Subject: [PATCH 22/36] fix(gfql): don't push onto a bool column the join will widen Numeric comparisons against a pandas bool column raised on queries master answers: MATCH (i)-->(p), (p)-->(c) WHERE p.flag > 0 master n=2 -> GFQLTypeError (same for >=, <>, and = 1; only = true survived) The gate reads the source nodes frame, but the connected-join executor filters a joined one, and an unmatched row introduces NaN. bool cannot hold NaN, so pandas widens it to object -- numeric to the gate, string to the validator. So "classify the frame the executor filters" was still not what the code did: it classified the frame before the join. bool is the only common dtype whose widening crosses the class boundary; int64 -> float64 stays numeric and keeps pushing. Decline bool rather than model the join's type algebra -- the residual answers, and guessing at conversions is what produced most of the defects here. The single-pattern path fails the same way on master, so the defect is pre-existing; this join shape was immune only because it never pushed. Existing bool tests covered equality only, and the polars nullable-Boolean test passes because that dtype converts to object and is declined -- neither exercised a pushed numeric op on a native bool. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- bin/ci_cypher_surface_guard_baseline.json | 2 +- graphistry/compute/gfql/cypher/lowering.py | 18 ++++++++++ .../compute/gfql/cypher/test_lowering.py | 36 +++++++++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/bin/ci_cypher_surface_guard_baseline.json b/bin/ci_cypher_surface_guard_baseline.json index 2d5e01ab1e..11e83ed638 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": 9217 + "lowering_py_max_lines": 9235 } diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index b8f508a0d3..d5b2f0b699 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -7839,6 +7839,22 @@ def _connected_join_dtype_classes(dtype: Any) -> Tuple[bool, bool]: return bool(_is_numeric_dtype_safe(dtype)), bool(_is_string_dtype_safe(dtype)) +def _connected_join_dtype_widens(dtype: Any) -> bool: + """Whether the join can change `dtype`'s class before the filter runs. + + The gate reads the source nodes frame, but the executor filters a joined one, and an + unmatched row introduces NaN. `bool` cannot hold NaN so pandas widens it to `object` -- + numeric to the gate, string to the validator. `int64` widens to `float64`, which stays + numeric and is harmless; `bool` is the only common dtype that crosses the boundary. + """ + import pandas as _pd + + try: + return bool(_pd.api.types.is_bool_dtype(dtype)) and not bool(_pd.api.types.is_extension_array_dtype(dtype)) + except Exception: + return False + + def _connected_join_dtype_admits(op: str, value: Any, dtype: Any) -> bool: """Whether pushing `op`/`value` onto a column of `dtype` matches residual semantics. @@ -7852,6 +7868,8 @@ def _connected_join_dtype_admits(op: str, value: Any, dtype: Any) -> bool: correct on non-pandas dtypes. `compute/validate_schema.py` looks similar but is dead code -- only a docs test imports it -- and it disagrees on `bool`. """ + if _connected_join_dtype_widens(dtype): + return False is_numeric_col, is_string_col = _connected_join_dtype_classes(dtype) if not is_numeric_col and not is_string_col: # Unrecognized dtype: fail closed. Pushing on a guess is how a correct empty diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index e007d2ec43..e5ac849759 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15328,6 +15328,42 @@ def test_connected_join_string_predicate_merge_matches_cypher(predicate: str, ex assert result._nodes.to_dict(orient="records") == expected +def _real_pandas_bool_graph() -> Plottable: + nodes = pd.DataFrame({"id": ["a", "b", "c", "d", "e"], "flag": [True, False, True, False, True], "age": [1, 2, 3, 4, 5]}) + edges = pd.DataFrame({"s": ["a", "b", "c", "a", "b"], "d": ["b", "c", "d", "c", "e"]}) + return graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + +@pytest.mark.parametrize( + "predicate,expected", + [ + # The gate reads the source frame but the executor filters a joined one, where an + # unmatched row introduces NaN and pandas widens bool -> object: numeric to the gate, + # string to the validator. Declining keeps the residual's answer. + ("p.flag > 0", [{"n": 2}]), + ("p.flag >= 1", [{"n": 2}]), + ("p.flag <> 0", [{"n": 2}]), + ("p.flag = 1", [{"n": 2}]), + ("p.flag = true", [{"n": 2}]), + # int64 widens to float64, which stays numeric, so this must still push and answer. + ("p.age >= 2", [{"n": 4}]), + ], +) +def test_connected_join_bool_column_survives_join_widening(predicate: str, expected: Any) -> None: + query = f"MATCH (i)-->(p), (p)-->(c) WHERE {predicate} RETURN count(p) AS n" + + result = _real_pandas_bool_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == expected + + +def test_connected_join_bool_dtype_never_pushes() -> None: + # bool cannot hold NaN, so the join widens it out of its class. int64 -> float64 stays + # numeric and must keep pushing. + assert _connected_join_dtype_admits(">", 0, pd.Series([True]).dtype) is False + assert _connected_join_dtype_admits("==", True, pd.Series([True]).dtype) is False + assert _connected_join_dtype_admits(">", 0, pd.Series([1]).dtype) is True + + def test_node_dtypes_for_pushdown_fails_closed_on_unreadable_nodes() -> None: # An unreadable schema must yield an empty mapping, not None: None means "no graph, use # value-type rules" and would push dtype-blind. Pinned directly because the two are From 666af9c265ba3ebb46d3bbb52109ce8dedc1ffa6 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 21:55:47 -0700 Subject: [PATCH 23/36] fix(gfql): don't push string ops onto object columns holding non-strings A bool column with a null is object, and string predicates are admitted on dtype alone -- by this gate and by filter_by_dict alike -- so .str failed on the values and leaked a raw exception where master raised a GFQL error: flag=[True,False,None,True], WHERE p.flag CONTAINS 'a' master GFQLValidationError -> builtins.AttributeError: Can only use .str accessor... Both checks are dtype-level while .str fails at the value level, so "planner and validator agree by construction" held and still failed. Declining `object` is not the fix: pandas stores ordinary strings there, so that would delete CONTAINS/STARTS WITH entirely. Instead omit object columns whose infer_dtype is a homogeneous non-string kind. An absent column has nothing to look up, so the gate fails closed and the residual answers, as it did before. Mixed and empty contents are untouched -- they already work. Real string columns keep pushing: p.name CONTAINS 'o' still answers where master cannot render the clause at all. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql_unified.py | 40 ++++++++++++++++++- .../compute/gfql/cypher/test_lowering.py | 39 ++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index d601d45e9a..cd293c7891 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -1381,6 +1381,39 @@ def _node_dtypes_for_pushdown( return _LazyNodeDtypes(g, engine) +def _object_column_holds_non_strings(frame: Any, column: str, dtype: Any) -> bool: + """Whether `column` is an object column whose values are not strings. + + `object` says nothing about contents: pandas stores ordinary strings that way, and also a + bool column that acquired a null. String predicates are admitted on dtype alone -- by this + gate and by `filter_by_dict` alike -- but `.str` fails on the values, leaking a raw + AttributeError. Dropping the column leaves the gate with nothing to look up, so it fails + closed and the residual answers. Mixed and empty contents are left alone: they work today. + """ + import pandas as _pd + + try: + if not _pd.api.types.is_object_dtype(dtype): + return False + inferred = _pd.api.types.infer_dtype(frame[column], skipna=True) + except Exception: + return False + return inferred in { + "boolean", + "integer", + "floating", + "decimal", + "complex", + "date", + "datetime", + "datetime64", + "timedelta", + "timedelta64", + "period", + "bytes", + } + + def _read_node_dtypes( g: Plottable, engine: Union[EngineAbstract, str] = EngineAbstract.AUTO, @@ -1407,7 +1440,12 @@ def _read_node_dtypes( # decision actually needs it. probe = df_to_engine(nodes, resolve_engine(cast(Any, engine), nodes), warn=False) # zip rather than `.items()`: pandas/cuDF expose a column-indexed Series, polars a list. - return {str(col): dtype for col, dtype in zip(list(probe.columns), list(probe.dtypes))} + dtypes = {str(col): dtype for col, dtype in zip(list(probe.columns), list(probe.dtypes))} + return { + col: dtype + for col, dtype in dtypes.items() + if not _object_column_holds_non_strings(probe, col, dtype) + } except Exception: # We have a graph but could not read its schema, so we know nothing about any column. # An empty mapping fails every column lookup, which fails closed to the residual. diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index e5ac849759..9ecafe2254 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15364,6 +15364,45 @@ def test_connected_join_bool_dtype_never_pushes() -> None: assert _connected_join_dtype_admits(">", 0, pd.Series([1]).dtype) is True +def _real_object_content_graph() -> Plottable: + nodes = pd.DataFrame({ + "id": ["n1", "n2", "n3", "n4"], + "flag": [True, False, None, True], # bool + null -> object, values are not strings + "name": ["ann", "bob", "cid", "dee"], + "mix": ["a", 1, None, "c"], + "num": [1, 2, 3, 4], + }) + edges = pd.DataFrame({"s": ["n1", "n2", "n3", "n1"], "d": ["n2", "n3", "n4", "n3"]}) + return graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + +def test_connected_join_string_op_on_non_string_object_column_stays_typed() -> None: + # `object` says nothing about contents: strings live there, and so does a bool column that + # acquired a null. Pushing a string op on dtype alone let `.str` fail on the values and leak + # a raw AttributeError where master raised a GFQL error. + query = "MATCH (i)-->(p), (p)-->(c) WHERE p.flag CONTAINS 'a' RETURN count(p) AS n" + + with pytest.raises(GFQLValidationError): + _real_object_content_graph().gfql(query) + + +@pytest.mark.parametrize( + "predicate,expected", + [ + # Real string columns are object too, and must keep pushing -- master cannot render these. + ("p.name CONTAINS 'o'", [{"n": 1}]), + ("p.name STARTS WITH 'a'", []), + ("p.mix CONTAINS 'a'", []), + ("p.num >= 2", [{"n": 3}]), + ], +) +def test_connected_join_object_columns_still_answer(predicate: str, expected: Any) -> None: + query = f"MATCH (i)-->(p), (p)-->(c) WHERE {predicate} RETURN count(p) AS n" + + result = _real_object_content_graph().gfql(query) + assert result._nodes.to_dict(orient="records") == expected + + def test_node_dtypes_for_pushdown_fails_closed_on_unreadable_nodes() -> None: # An unreadable schema must yield an empty mapping, not None: None means "no graph, use # value-type rules" and would push dtype-blind. Pinned directly because the two are From dd47a5aa6add47537e956a235e0380caf7af0ee4 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 22:14:19 -0700 Subject: [PATCH 24/36] fix(gfql): mirror pandas' own .str rule instead of enumerating rejects The omit-set listed kinds to reject and missed three that .str also rejects, so they still leaked a raw AttributeError where master raised a GFQL error: score = [1.5, 2.5, None, 4.5] (mixed-integer-float) -> builtins.AttributeError clock = [time(1), time(2), None, time(3)] (time) -> builtins.AttributeError object column of pd.Interval (interval) -> builtins.AttributeError mixed-integer-float is the sharp one: it is what a float column with a None infers as, while np.nan infers as floating. So the previous commit caught "a bool column that acquired a null" and missed the float column that acquired one -- its own motivating case, one dtype over. StringMethods._validate admits exactly {string, empty, bytes, mixed, mixed-integer}. Keep those and omit the rest, minus bytes, which str.contains forbids. Mirroring the rule fixes all three and fails closed on kinds pandas adds later; an enumerated denylist can do neither. Also repairs the totality premise of the full-conversion test, which the previous commit broke: the map is partial by design now, and the test asserted every column was present. It failed only under system python3 -- the polars lane skips it in CI, so 77/77 stayed green with a broken test. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql_unified.py | 30 +++++-------- .../compute/gfql/cypher/test_lowering.py | 45 ++++++++++++++++++- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index cd293c7891..23c68b49c5 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -1382,13 +1382,20 @@ def _node_dtypes_for_pushdown( def _object_column_holds_non_strings(frame: Any, column: str, dtype: Any) -> bool: - """Whether `column` is an object column whose values are not strings. + """Whether `column` is an object column whose values `.str` would reject. `object` says nothing about contents: pandas stores ordinary strings that way, and also a - bool column that acquired a null. String predicates are admitted on dtype alone -- by this - gate and by `filter_by_dict` alike -- but `.str` fails on the values, leaking a raw + numeric column that acquired a `None`. String predicates are admitted on dtype alone -- by + this gate and by `filter_by_dict` alike -- but `.str` fails on the values, leaking a raw AttributeError. Dropping the column leaves the gate with nothing to look up, so it fails - closed and the residual answers. Mixed and empty contents are left alone: they work today. + closed and the residual answers. + + Mirror pandas' own rule rather than enumerate what to reject: `StringMethods._validate` + admits exactly {string, empty, bytes, mixed, mixed-integer}. Enumerating the complement + misses cases (`mixed-integer-float` -- which is what a float column with a `None` infers as, + unlike `np.nan`, which infers as `floating`) and cannot fail closed on kinds pandas adds + later. `bytes` is excluded here because `str.contains` forbids it even though the accessor + admits it. """ import pandas as _pd @@ -1398,20 +1405,7 @@ def _object_column_holds_non_strings(frame: Any, column: str, dtype: Any) -> boo inferred = _pd.api.types.infer_dtype(frame[column], skipna=True) except Exception: return False - return inferred in { - "boolean", - "integer", - "floating", - "decimal", - "complex", - "date", - "datetime", - "datetime64", - "timedelta", - "timedelta64", - "period", - "bytes", - } + return inferred not in {"string", "empty", "mixed", "mixed-integer"} def _read_node_dtypes( diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 9ecafe2254..985f3af032 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15376,6 +15376,38 @@ def _real_object_content_graph() -> Plottable: return graphistry.nodes(nodes, "id").edges(edges, "s", "d") +@pytest.mark.parametrize( + "column", + [ + # `.str` admits only {string, empty, bytes, mixed, mixed-integer}; everything else must + # be omitted. A float column with a None infers as mixed-integer-float -- unlike np.nan, + # which infers as floating -- so enumerating what to reject misses it. + "score", + "clock", + "flag", + ], +) +def test_connected_join_string_op_on_non_str_kinds_stays_typed(column: str) -> None: + query = f"MATCH (i)-->(p), (p)-->(c) WHERE p.{column} CONTAINS 'a' RETURN count(p) AS n" + + with pytest.raises(GFQLValidationError): + _real_infer_kind_graph().gfql(query) + + +def _real_infer_kind_graph() -> Plottable: + import datetime + + nodes = pd.DataFrame({"id": ["n1", "n2", "n3", "n4"]}) + nodes["score"] = pd.Series([1.5, 2.5, None, 4.5], dtype=object) # mixed-integer-float + nodes["clock"] = pd.Series( + [datetime.time(1, 0), datetime.time(2, 0), None, datetime.time(3, 0)], dtype=object + ) # time + nodes["flag"] = pd.Series([True, False, None, True], dtype=object) # boolean + nodes["name"] = ["ann", "bob", "cid", "dee"] # string -- must still push + edges = pd.DataFrame({"s": ["n1", "n2", "n3", "n1"], "d": ["n2", "n3", "n4", "n1"]}) + return graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + def test_connected_join_string_op_on_non_string_object_column_stays_typed() -> None: # `object` says nothing about contents: strings live there, and so does a bool column that # acquired a null. Pushing a string op on dtype alone let `.str` fail on the values and leak @@ -15474,12 +15506,21 @@ def test_node_dtypes_for_pushdown_matches_the_full_conversion() -> None: reported = _node_dtypes_for_pushdown(g) assert reported is not None executed = df_to_engine(nodes, resolve_engine("auto", nodes), warn=False) + # The map is deliberately partial: object columns whose values `.str` would reject are + # omitted so the gate fails closed on them. Every column it DOES report must agree with + # the executed frame. for column, dtype in zip(list(executed.columns), list(executed.dtypes)): + if str(column) not in reported: + continue assert _connected_join_dtype_classes(reported[str(column)]) == _connected_join_dtype_classes(dtype) - # The empty probe disagrees on the nullable bool, which is why it cannot be used. + # `flag` is polars Boolean with a null -> pandas object holding bools -> omitted. + assert "flag" not in reported + assert {"id", "age", "name"} <= set(reported) + # The empty probe would have called the nullable bool numeric, which is why it cannot be + # used: the real conversion yields object-holding-bools, which is omitted instead. empty = df_to_engine(nodes.head(0), resolve_engine("auto", nodes), warn=False) empty_dtypes = dict(zip([str(name) for name in empty.columns], list(empty.dtypes))) - assert _connected_join_dtype_classes(empty_dtypes["flag"]) != _connected_join_dtype_classes(reported["flag"]) + assert _connected_join_dtype_classes(empty_dtypes["flag"]) == (True, False) @pytest.mark.parametrize( From bec4060e677210014a0524d59623cc74220fbb5f Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 22:33:57 -0700 Subject: [PATCH 25/36] test(gfql): pin the bytes carve-out in the string-op gate Adding "bytes" back to the allowlist survived all 1292 tests, yet it is load-bearing: without the carve-out, an object column of bytes leaks a raw TypeError where master raises a GFQL error. The subtlety is two-layer. bytes passes StringMethods._validate, so mirroring that allowlist verbatim looks right -- but the methods themselves reject it via @forbid_nonstring_types, so str.contains still fails. Nothing pinned the difference. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- .../tests/compute/gfql/cypher/test_lowering.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 985f3af032..4e5818103b 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15394,6 +15394,20 @@ def test_connected_join_string_op_on_non_str_kinds_stays_typed(column: str) -> N _real_infer_kind_graph().gfql(query) +def test_connected_join_bytes_column_stays_typed() -> None: + # bytes passes StringMethods._validate but the methods themselves forbid it via + # @forbid_nonstring_types, so it must be omitted despite being in pandas' accessor + # allowlist. Without the carve-out this leaks a raw TypeError. + nodes = pd.DataFrame({"id": ["n1", "n2", "n3", "n4"]}) + nodes["raw"] = pd.Series([b"abc", b"def", None, b"ghi"], dtype=object) + edges = pd.DataFrame({"s": ["n1", "n2", "n3", "n1"], "d": ["n2", "n3", "n4", "n1"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + query = "MATCH (i)-->(p), (p)-->(c) WHERE p.raw CONTAINS 'a' RETURN count(p) AS n" + + with pytest.raises(GFQLValidationError): + g.gfql(query) + + def _real_infer_kind_graph() -> Plottable: import datetime From d161d84c38617fdbc70645108588cf21b5c1f77b Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 23:07:49 -0700 Subject: [PATCH 26/36] test(gfql): pin the floating and integer omit-set members Admitting "floating" or "integer" back into the keep-set survives all 1293 tests, yet each reintroduces the raw AttributeError the omit-set exists to prevent -- an object column of plain floats or ints is the most trivially constructible case of it. The existing test parametrized only mixed-integer-float, time, and boolean; bytes got its own test. The two simplest kinds were pinned by nothing. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/tests/compute/gfql/cypher/test_lowering.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 4e5818103b..3497eb49c7 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15385,6 +15385,8 @@ def _real_object_content_graph() -> Plottable: "score", "clock", "flag", + "plain_float", + "plain_int", ], ) def test_connected_join_string_op_on_non_str_kinds_stays_typed(column: str) -> None: @@ -15417,6 +15419,8 @@ def _real_infer_kind_graph() -> Plottable: [datetime.time(1, 0), datetime.time(2, 0), None, datetime.time(3, 0)], dtype=object ) # time nodes["flag"] = pd.Series([True, False, None, True], dtype=object) # boolean + nodes["plain_float"] = pd.Series([1.5, 2.5, 3.5, 4.5], dtype=object) # floating + nodes["plain_int"] = pd.Series([1, 2, 3, 4], dtype=object) # integer nodes["name"] = ["ann", "bob", "cid", "dee"] # string -- must still push edges = pd.DataFrame({"s": ["n1", "n2", "n3", "n1"], "d": ["n2", "n3", "n4", "n1"]}) return graphistry.nodes(nodes, "id").edges(edges, "s", "d") From e93d549c48ab215848ef554be7a363fe9aa6b191 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Thu, 16 Jul 2026 23:57:41 -0700 Subject: [PATCH 27/36] fix(gfql): admit only object kinds that survive subsetting mixed and mixed-integer passed pandas' accessor rule on the source frame, but the pushed filter runs on the join's candidate subset -- and those kinds are not closed under it. Drop the strings and they collapse to integer/floating/boolean, which .str rejects: value=["alpha",10,20,30,"beta"], p-candidates hold only numbers WHERE p.value CONTAINS 'a' -> builtins.AttributeError (raw) Master never raises there, but only because it never applies the filter -- its n=2 is wrong. This is the third defect from one root: the gate reasons about a frame the executor does not filter. Pre-conversion dtypes, then post-join widening, now post-subset content. So admit only what no subset can invalidate: string and empty. A subset of strings is string or empty, both accepted. Mixed columns now decline to a typed error, matching master, which cannot render CONTAINS at all. Pure string columns keep pushing and answering where master rejects them. The durable fix is to make the pushed predicate itself value-safe, which would also repair master's single-pattern leak; that reaches into shared predicates and belongs in its own change. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql_unified.py | 18 +++++++++++------- .../tests/compute/gfql/cypher/test_lowering.py | 17 ++++++++++++++++- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index 23c68b49c5..64e0b4d093 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -1390,12 +1390,16 @@ def _object_column_holds_non_strings(frame: Any, column: str, dtype: Any) -> boo AttributeError. Dropping the column leaves the gate with nothing to look up, so it fails closed and the residual answers. - Mirror pandas' own rule rather than enumerate what to reject: `StringMethods._validate` - admits exactly {string, empty, bytes, mixed, mixed-integer}. Enumerating the complement - misses cases (`mixed-integer-float` -- which is what a float column with a `None` infers as, - unlike `np.nan`, which infers as `floating`) and cannot fail closed on kinds pandas adds - later. `bytes` is excluded here because `str.contains` forbids it even though the accessor - admits it. + Keep only the kinds that stay valid under SUBSETTING. `StringMethods._validate` admits + {string, empty, bytes, mixed, mixed-integer}, but it runs on the frame it is handed -- and + the frame we inspect is the source, while the pushed filter runs on the join's candidate + subset. `string` and `empty` survive that (a subset of strings is string or empty), but + `mixed`/`mixed-integer` do not: drop the strings and they collapse to integer/floating/ + boolean, which `.str` rejects. `bytes` is excluded separately -- it passes `_validate` yet + `str.contains` forbids it. + + Mirroring a rule is not enough when we evaluate it against a different frame than the + executor does, so admit only what no subset can invalidate. """ import pandas as _pd @@ -1405,7 +1409,7 @@ def _object_column_holds_non_strings(frame: Any, column: str, dtype: Any) -> boo inferred = _pd.api.types.infer_dtype(frame[column], skipna=True) except Exception: return False - return inferred not in {"string", "empty", "mixed", "mixed-integer"} + return inferred not in {"string", "empty"} def _read_node_dtypes( diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 3497eb49c7..94b8fecba5 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15396,6 +15396,21 @@ def test_connected_join_string_op_on_non_str_kinds_stays_typed(column: str) -> N _real_infer_kind_graph().gfql(query) +def test_connected_join_mixed_column_stays_typed() -> None: + # `mixed`/`mixed-integer` pass pandas' accessor rule on the SOURCE frame but are not closed + # under subsetting: the executor filters the join's candidates, and dropping the strings + # leaves integers that `.str` rejects. Declining matches master, which cannot render + # CONTAINS at all. + nodes = pd.DataFrame({"id": ["u1", "u2", "u3", "u4", "u5"]}) + nodes["value"] = pd.Series(["alpha", 10, 20, 30, "beta"], dtype=object) # mixed-integer + edges = pd.DataFrame({"s": ["u1", "u2", "u5"], "d": ["u2", "u3", "u2"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + query = "MATCH (i)-->(p), (p)-->(c) WHERE p.value CONTAINS 'a' RETURN count(p) AS n" + + with pytest.raises(GFQLValidationError): + g.gfql(query) + + def test_connected_join_bytes_column_stays_typed() -> None: # bytes passes StringMethods._validate but the methods themselves forbid it via # @forbid_nonstring_types, so it must be omitted despite being in pandas' accessor @@ -15442,7 +15457,7 @@ def test_connected_join_string_op_on_non_string_object_column_stays_typed() -> N # Real string columns are object too, and must keep pushing -- master cannot render these. ("p.name CONTAINS 'o'", [{"n": 1}]), ("p.name STARTS WITH 'a'", []), - ("p.mix CONTAINS 'a'", []), + ("p.num >= 2", [{"n": 3}]), ], ) From 9005a3fdf5ade9d8f41fe66c144afa302e86d6fc Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Fri, 17 Jul 2026 00:36:24 -0700 Subject: [PATCH 28/36] test(gfql): pin the empty member of the string-op keep-set Dropping "empty" from the keep-set discriminates zero tests across the whole 4,530-test gfql suite, yet it is load-bearing: an all-null object column infers `empty`, and `.str` handles it, so CONTAINS correctly returns [] (3VL: NULL CONTAINS 'a' is NULL, no rows). Drop it and those queries silently revert to master's unsupported error. Its sibling "string" causes 15 failures when dropped -- the asymmetry is what exposed the gap. "empty" is also load-bearing for the subset-closure argument itself: a subset of strings is string or empty. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- .../compute/gfql/cypher/test_lowering.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 94b8fecba5..278d26af1d 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15396,6 +15396,26 @@ def test_connected_join_string_op_on_non_str_kinds_stays_typed(column: str) -> N _real_infer_kind_graph().gfql(query) +def _real_all_null_object_graph() -> Plottable: + nodes = pd.DataFrame({"id": ["n1", "n2", "n3", "n4"]}) + nodes["note"] = pd.Series([None] * 4, dtype=object) # infers `empty` + edges = pd.DataFrame({"s": ["n1", "n2", "n3", "n1"], "d": ["n2", "n3", "n4", "n3"]}) + return graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + +@pytest.mark.parametrize( + "predicate", + ["p.note CONTAINS 'a'", "p.note STARTS WITH 'a'", "p.note ENDS WITH 'a'", "p.note =~ 'a.*'"], +) +def test_connected_join_empty_object_column_still_answers(predicate: str) -> None: + # `empty` (an all-null object column) is subset-closed and `.str` handles it, so it must keep + # pushing: 3VL says NULL CONTAINS 'a' is NULL, hence no rows. Dropping `empty` from the + # keep-set silently reverts these to master's unsupported error, and nothing else catches it. + query = f"MATCH (i)-->(p), (p)-->(c) WHERE {predicate} RETURN count(p) AS n" + + assert _real_all_null_object_graph().gfql(query)._nodes.to_dict(orient="records") == [] + + def test_connected_join_mixed_column_stays_typed() -> None: # `mixed`/`mixed-integer` pass pandas' accessor rule on the SOURCE frame but are not closed # under subsetting: the executor filters the join's candidates, and dropping the strings From 77df4f798e2de9532a3c22870b33194671484866 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Fri, 17 Jul 2026 01:17:52 -0700 Subject: [PATCH 29/36] fix(gfql): keep the aggregate's schema when a pushed filter empties a pattern A pushed filter that emptied a pattern returned a schema-less frame and skipped post_join_chain -- where count(p) AS n lives -- so the RETURN column vanished: WHERE p.s_col = 'zzz' (string, pushed) cols=[] <- master: cols=['n'] WHERE p.m_col = 'zzz' (mixed, not pushed) cols=['n'] Same query shape, same emptiness; the only discriminator was whether the dtype gate admitted the column, so consumers doing df["n"] hit a KeyError on one and not the other. Both early-returns are byte-identical on master. The regression is reachability: master never pushed into the pattern, so the filter ran post-join and the aggregate survived. This is the fourth defect from that one root -- the pushdown reaching code paths master never exercised. An emptied pattern still carries its columns, so let it flow: the joins propagate empty-with-schema and post_join_chain runs on empty input, which is what produces the column. Projections were never affected; they don't take this path. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql_unified.py | 8 +++-- .../compute/gfql/cypher/test_lowering.py | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index 64e0b4d093..345f6f66fc 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -493,11 +493,15 @@ def _apply_connected_match_join( ) pattern_result = _chain_dispatch(base_graph, with_rows, dispatch_engine, policy, context) pattern_rows = cast(Optional[DataFrameT], pattern_result._nodes) - if pattern_rows is None or len(pattern_rows) == 0: + if pattern_rows is None: out = base_graph.bind() out._nodes = df_ctor() out._edges = df_ctor() return out + # An emptied pattern still carries its columns, so let it flow: the joins propagate + # empty-with-schema and `post_join_chain` still runs. Short-circuiting here skipped that + # chain and dropped the aggregate's RETURN column, which only became reachable once + # predicates started pushing into the pattern. pattern_rows = cast(DataFrameT, pattern_rows[_binding_join_columns(pattern_rows)]) if joined_rows is None: joined_rows = pattern_rows @@ -526,7 +530,7 @@ def _apply_connected_match_join( engine=requested_engine, ) - if joined_rows is None or len(joined_rows) == 0: + if joined_rows is None: out = base_graph.bind() out._nodes = df_ctor() out._edges = df_ctor() diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 278d26af1d..cec597cdbd 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15396,6 +15396,42 @@ def test_connected_join_string_op_on_non_str_kinds_stays_typed(column: str) -> N _real_infer_kind_graph().gfql(query) +@pytest.mark.parametrize( + "predicate", + [ + # A pushed filter that empties the pattern must still run post_join_chain, or the + # aggregate's RETURN column vanishes. `s_col` pushes and `m_col` does not, so the two + # must agree -- that discriminator is what exposed the inconsistency. + "p.s_col = 'zzz'", + "p.m_col = 'zzz'", + ], +) +def test_connected_join_empty_pushed_pattern_keeps_aggregate_schema(predicate: str) -> None: + nodes = pd.DataFrame({ + "id": ["n1", "n2", "n3", "n4"], + "s_col": pd.Series(["a", "b", "c", "d"], dtype=object), # string -> pushes + "m_col": pd.Series(["a", 1, "b", "c"], dtype=object), # mixed-integer -> declines + }) + edges = pd.DataFrame({"s": ["n1", "n2", "n3"], "d": ["n2", "n3", "n4"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + query = f"MATCH (p)-[]->(q), (p)-[]->(r) WHERE {predicate} RETURN count(p) AS n" + + result = g.gfql(query)._nodes + assert list(result.columns) == ["n"] + + +def test_connected_join_non_empty_pushed_pattern_still_answers() -> None: + nodes = pd.DataFrame({ + "id": ["n1", "n2", "n3", "n4"], + "s_col": pd.Series(["a", "b", "c", "d"], dtype=object), + }) + edges = pd.DataFrame({"s": ["n1", "n2", "n3"], "d": ["n2", "n3", "n4"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + query = "MATCH (p)-[]->(q), (p)-[]->(r) WHERE p.s_col = 'a' RETURN count(p) AS n" + + assert g.gfql(query)._nodes.to_dict(orient="records") == [{"n": 1}] + + def _real_all_null_object_graph() -> Plottable: nodes = pd.DataFrame({"id": ["n1", "n2", "n3", "n4"]}) nodes["note"] = pd.Series([None] * 4, dtype=object) # infers `empty` From 03d1f9d7963c3bf2a388ea7fefaf2985ff7019e8 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Fri, 17 Jul 2026 01:30:21 -0700 Subject: [PATCH 30/36] Revert "fix(gfql): keep the aggregate's schema when a pushed filter empties a pattern" This reverts commit 77df4f79. Its central claim -- "an emptied pattern still carries its columns, so let it flow" -- is false for edge aliases, and I shipped it without checking. The `rows` binding-op emits no edge-alias payload at 0 rows, so `e1.w` and the `e1.e1` marker vanish. `joined_alias_columns` recovers node aliases via its `suffix == "id"` fallback -- which is why count(p)/count(q) passed and the claim looked true -- but edge aliases have no `.id` and are only recoverable from the marker the emptied pattern never emits. post_join_chain then dereferences a column that isn't there: MATCH (p)-[e1]->(q), (p)-[e2]->(r) WHERE p.i_col = 999 RETURN count(e1) AS n master cols=['n'] rows=0 -> GFQLTypeError So the commit made the very thing it set out to preserve strictly worse: master returns the ['n'] schema, and it raised. A MEDIUM schema drop was traded for a HIGH failure on valid queries. Reverting restores the prior behaviour, which reinstates the original schema drop (#25). That defect stands, recorded, with its real fix: make the `rows` binding-op emit the full binding schema at 0 rows, so the invariant is established rather than assumed. Restoring the early-return is not that fix -- it just stops making things worse. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql_unified.py | 8 ++--- .../compute/gfql/cypher/test_lowering.py | 36 ------------------- 2 files changed, 2 insertions(+), 42 deletions(-) diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index 345f6f66fc..64e0b4d093 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -493,15 +493,11 @@ def _apply_connected_match_join( ) pattern_result = _chain_dispatch(base_graph, with_rows, dispatch_engine, policy, context) pattern_rows = cast(Optional[DataFrameT], pattern_result._nodes) - if pattern_rows is None: + if pattern_rows is None or len(pattern_rows) == 0: out = base_graph.bind() out._nodes = df_ctor() out._edges = df_ctor() return out - # An emptied pattern still carries its columns, so let it flow: the joins propagate - # empty-with-schema and `post_join_chain` still runs. Short-circuiting here skipped that - # chain and dropped the aggregate's RETURN column, which only became reachable once - # predicates started pushing into the pattern. pattern_rows = cast(DataFrameT, pattern_rows[_binding_join_columns(pattern_rows)]) if joined_rows is None: joined_rows = pattern_rows @@ -530,7 +526,7 @@ def _apply_connected_match_join( engine=requested_engine, ) - if joined_rows is None: + if joined_rows is None or len(joined_rows) == 0: out = base_graph.bind() out._nodes = df_ctor() out._edges = df_ctor() diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index cec597cdbd..278d26af1d 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15396,42 +15396,6 @@ def test_connected_join_string_op_on_non_str_kinds_stays_typed(column: str) -> N _real_infer_kind_graph().gfql(query) -@pytest.mark.parametrize( - "predicate", - [ - # A pushed filter that empties the pattern must still run post_join_chain, or the - # aggregate's RETURN column vanishes. `s_col` pushes and `m_col` does not, so the two - # must agree -- that discriminator is what exposed the inconsistency. - "p.s_col = 'zzz'", - "p.m_col = 'zzz'", - ], -) -def test_connected_join_empty_pushed_pattern_keeps_aggregate_schema(predicate: str) -> None: - nodes = pd.DataFrame({ - "id": ["n1", "n2", "n3", "n4"], - "s_col": pd.Series(["a", "b", "c", "d"], dtype=object), # string -> pushes - "m_col": pd.Series(["a", 1, "b", "c"], dtype=object), # mixed-integer -> declines - }) - edges = pd.DataFrame({"s": ["n1", "n2", "n3"], "d": ["n2", "n3", "n4"]}) - g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") - query = f"MATCH (p)-[]->(q), (p)-[]->(r) WHERE {predicate} RETURN count(p) AS n" - - result = g.gfql(query)._nodes - assert list(result.columns) == ["n"] - - -def test_connected_join_non_empty_pushed_pattern_still_answers() -> None: - nodes = pd.DataFrame({ - "id": ["n1", "n2", "n3", "n4"], - "s_col": pd.Series(["a", "b", "c", "d"], dtype=object), - }) - edges = pd.DataFrame({"s": ["n1", "n2", "n3"], "d": ["n2", "n3", "n4"]}) - g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") - query = "MATCH (p)-[]->(q), (p)-[]->(r) WHERE p.s_col = 'a' RETURN count(p) AS n" - - assert g.gfql(query)._nodes.to_dict(orient="records") == [{"n": 1}] - - def _real_all_null_object_graph() -> Plottable: nodes = pd.DataFrame({"id": ["n1", "n2", "n3", "n4"]}) nodes["note"] = pd.Series([None] * 4, dtype=object) # infers `empty` From 5a1dc463dc735f05b68befcacc5c70dfb5e7cce5 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Fri, 17 Jul 2026 01:48:35 -0700 Subject: [PATCH 31/36] fix(gfql): fail closed when an object column's values can't be read The except path returned False, keeping the column -- the opposite of what its own docstring claimed and of what its sibling _read_node_dtypes does (returns {} for a schema it cannot read). An object column we cannot inspect tells us nothing about whether .str would reject its values, so keeping it pushes blind. Split the two failure modes: a dtype check that raises means the column isn't ours to judge (keep); an unreadable object column means omit and let the residual answer. Latent rather than live -- the reachable paths (duplicate column labels, non-str labels) are already outside the supported envelope or unreachable through the parser. Fixed because "fail closed on what you cannot verify" is the rule the rest of this gate follows. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql_unified.py | 13 ++++++++++--- .../tests/compute/gfql/cypher/test_lowering.py | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index 64e0b4d093..fa163f8a7f 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -1404,11 +1404,18 @@ def _object_column_holds_non_strings(frame: Any, column: str, dtype: Any) -> boo import pandas as _pd try: - if not _pd.api.types.is_object_dtype(dtype): - return False - inferred = _pd.api.types.infer_dtype(frame[column], skipna=True) + is_object = bool(_pd.api.types.is_object_dtype(dtype)) except Exception: return False + if not is_object: + return False + try: + inferred = _pd.api.types.infer_dtype(frame[column], skipna=True) + except Exception: + # It IS an object column and we cannot read its values, so we cannot tell whether + # `.str` would reject them. Omit it and let the residual answer, matching + # `_read_node_dtypes`, which returns {} for a schema it cannot read. + return True return inferred not in {"string", "empty"} diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 278d26af1d..e7d4c8637e 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15396,6 +15396,24 @@ def test_connected_join_string_op_on_non_str_kinds_stays_typed(column: str) -> N _real_infer_kind_graph().gfql(query) +def test_object_column_holds_non_strings_fails_closed_when_unreadable() -> None: + # An object column whose values cannot be read tells us nothing about whether `.str` would + # reject them, so omit it and let the residual answer -- matching `_read_node_dtypes`, which + # returns {} for a schema it cannot read. Keeping it would push blind. + from graphistry.compute.gfql_unified import _object_column_holds_non_strings + + class _UnreadableFrame: + def __getitem__(self, key: str) -> Any: + raise RuntimeError("cannot read column") + + object_dtype = pd.Series(["a"], dtype=object).dtype + assert _object_column_holds_non_strings(_UnreadableFrame(), "x", object_dtype) is True + # Non-object dtypes are not this helper's concern, and real strings still push. + assert _object_column_holds_non_strings(pd.DataFrame({"x": [1]}), "x", pd.Series([1]).dtype) is False + strings = pd.DataFrame({"x": pd.Series(["a", "b"], dtype=object)}) + assert _object_column_holds_non_strings(strings, "x", strings["x"].dtype) is False + + def _real_all_null_object_graph() -> Plottable: nodes = pd.DataFrame({"id": ["n1", "n2", "n3", "n4"]}) nodes["note"] = pd.Series([None] * 4, dtype=object) # infers `empty` From 045b3fedbe9b1e29a9759d83f41d79415c66eb53 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Fri, 17 Jul 2026 09:17:46 -0700 Subject: [PATCH 32/36] fix(gfql): emit the full binding schema at 0 rows, then flow empties through #25 root fix. A pushed filter that emptied a connected-join pattern returned a schema-less frame and skipped post_join_chain, dropping the aggregate's RETURN column. #27 showed why removing the early-returns alone (77df4f79, reverted) is wrong: the `rows` binding-op drops edge-alias columns at 0 rows -- `_gfql_connected_bindings_state` assembles the schema incrementally, so an emptied hop returns before adding edge-alias and later-node columns. Node aliases survive via the row-frame merge's empty lookup; edge aliases have no `.id` fallback, so count(e1) on an emptied pattern raised. Fix the root: at the end of the bindings row-frame build, when 0 rows, add the declared alias.alias markers and edge-alias columns (derivable from ops + the base edge frame). That makes the emptied frame carry its full schema -- instrumented: the rows op now emits all 13 columns at 0 rows, matching the non-empty shape. Only then remove the two early-returns in _apply_connected_match_join so the empty-with-schema frame flows through post_join_chain. Verified vs master: count(e1)/count(DISTINCT e1)/edge-in-WHERE on an emptied pattern, plus the #25 node case, all match master (cols=['n'], rows=0); non-empty controls unchanged; OPTIONAL MATCH (shares the rows op) and polars unchanged. Both halves mutation-verified. The 0-row bare aggregate still returns rows=0 where Cypher wants one row n=0 -- pre-existing on master, unchanged here, recorded as a separate latent finding. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql/row/pipeline.py | 37 +++++++++++++++++++ graphistry/compute/gfql_unified.py | 7 +++- .../compute/gfql/cypher/test_lowering.py | 36 ++++++++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/graphistry/compute/gfql/row/pipeline.py b/graphistry/compute/gfql/row/pipeline.py index 836bbf3097..bf8a805fc8 100644 --- a/graphistry/compute/gfql/row/pipeline.py +++ b/graphistry/compute/gfql/row/pipeline.py @@ -3787,6 +3787,41 @@ def _gfql_connected_bindings_row_table_from_ops( setattr(out, "_gfql_rows_edge_aliases", edge_aliases) return out + def _gfql_add_missing_binding_columns( + self, bindings: DataFrameT, ops: Sequence["ASTObject"] + ) -> DataFrameT: + """Ensure an EMPTY bindings frame still carries every declared alias column. + + The walk assembles the schema incrementally, so an emptied hop returns a frame that + never acquired the columns later steps would have added -- edge-alias columns + (``e1.w``) and ``alias.alias`` markers. Node aliases survive via the row-frame merge's + empty lookup, but edge aliases have no ``.id`` fallback, so ``count(e1)`` / + ``e1.w``-in-WHERE dereferenced a missing column. Declared columns are derivable from + ``ops`` + the base edge frame, so add whatever is missing here (0-row only, no effect + on non-empty output). See #25/#27. + """ + from graphistry.compute.ast import ASTEdge, ASTNode + + base_graph = self._gfql_base_graph() + edges = base_graph._edges if base_graph is not None else None + src_col = base_graph._source if base_graph is not None else None + dst_col = base_graph._destination if base_graph is not None else None + missing: List[str] = [] + for op in ops: + alias = getattr(op, "_name", None) + if not isinstance(alias, str): + continue + missing.append(f"{alias}.{alias}") # the alias.alias marker + if isinstance(op, ASTEdge) and edges is not None: + for col in edges.columns: + if col in (src_col, dst_col): + continue + missing.append(f"{alias}.{col}") + for col in missing: + if col not in bindings.columns: + bindings[col] = self._gfql_broadcast_scalar(bindings, None) + return bindings + def _gfql_connected_bindings_row_frame_from_state( self, ops: Sequence["ASTObject"], @@ -3848,6 +3883,8 @@ def _gfql_connected_bindings_row_frame_from_state( drop_cols = ["__current__"] 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) return bindings def _gfql_shortest_path_scalar_native( diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index fa163f8a7f..af6c4d5076 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -493,11 +493,14 @@ def _apply_connected_match_join( ) pattern_result = _chain_dispatch(base_graph, with_rows, dispatch_engine, policy, context) pattern_rows = cast(Optional[DataFrameT], pattern_result._nodes) - if pattern_rows is None or len(pattern_rows) == 0: + if pattern_rows is None: out = base_graph.bind() out._nodes = df_ctor() out._edges = df_ctor() return out + # The rows op now emits the full binding schema even at 0 rows (#25), so an emptied + # pattern carries its columns and flows through post_join_chain -- which is where the + # aggregate RETURN lives. Short-circuiting here dropped that column. pattern_rows = cast(DataFrameT, pattern_rows[_binding_join_columns(pattern_rows)]) if joined_rows is None: joined_rows = pattern_rows @@ -526,7 +529,7 @@ def _apply_connected_match_join( engine=requested_engine, ) - if joined_rows is None or len(joined_rows) == 0: + if joined_rows is None: out = base_graph.bind() out._nodes = df_ctor() out._edges = df_ctor() diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index e7d4c8637e..885656da4a 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15396,6 +15396,42 @@ def test_connected_join_string_op_on_non_str_kinds_stays_typed(column: str) -> N _real_infer_kind_graph().gfql(query) +def _real_edge_alias_graph() -> Plottable: + nodes = pd.DataFrame({ + "id": ["n1", "n2", "n3", "n4"], + "s_col": pd.Series(["a", "b", "c", "d"], dtype=object), + "i_col": pd.Series([1, 2, 3, 4], dtype="int64"), + }) + edges = pd.DataFrame({ + "s": ["n1", "n2", "n3"], "d": ["n2", "n3", "n4"], "w": pd.Series([1, 2, 3], dtype="int64"), + }) + return graphistry.nodes(nodes, "id").edges(edges, "s", "d") + + +@pytest.mark.parametrize( + "predicate,ret", + [ + # A pushed filter that empties the pattern must still run post_join_chain, so the + # aggregate RETURN column survives -- including for edge aliases (#27), which have no + # `.id` fallback and raised until the rows op emitted the full binding schema at 0 rows. + ("p.i_col = 999", "count(e1) AS n"), + ("p.s_col = 'zzz'", "count(e1) AS n"), + ("p.s_col = 'zzz' AND e1.w > 0", "count(p) AS n"), + ("p.s_col = 'zzz'", "count(p) AS n"), + ], +) +def test_connected_join_empty_pattern_keeps_edge_alias_aggregate(predicate: str, ret: str) -> None: + query = f"MATCH (p)-[e1]->(q), (p)-[e2]->(r) WHERE {predicate} RETURN {ret}" + result = _real_edge_alias_graph().gfql(query)._nodes + assert list(result.columns) == ["n"] + assert len(result) == 0 + + +def test_connected_join_non_empty_edge_alias_aggregate_answers() -> None: + query = "MATCH (p)-[e1]->(q), (p)-[e2]->(r) WHERE e1.w > 0 RETURN count(p) AS n" + assert _real_edge_alias_graph().gfql(query)._nodes.to_dict(orient="records") == [{"n": 3}] + + def test_object_column_holds_non_strings_fails_closed_when_unreadable() -> None: # An object column whose values cannot be read tells us nothing about whether `.str` would # reject them, so omit it and let the residual answer -- matching `_read_node_dtypes`, which From 9e5c9c701769df9e90cb216cdb325d1795e31dd3 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Fri, 17 Jul 2026 09:31:11 -0700 Subject: [PATCH 33/36] fix(gfql): carry source dtype when filling edge-alias columns at 0 rows The #25 schema fill used an untyped None broadcast, so an emptied edge-alias column came back object where the non-empty path and master give int64: WHERE a.v > 99999 RETURN sum(e1.w) master int64 [] -> object [] Values were always correct, but the object dtype upcast the aggregate and escaped a 0-row branch into a non-empty result via UNION ALL (the concatenated column became object). Fill from a typed empty slice of the source edge frame instead, mirroring the node path's typed lookup; the alias.alias marker has no source dtype and keeps the None broadcast. sum/avg/max/count over an emptied edge-alias pattern, and the UNION ALL escape, now match master's dtype exactly. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql/row/pipeline.py | 17 ++++++++++++----- .../tests/compute/gfql/cypher/test_lowering.py | 17 +++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/graphistry/compute/gfql/row/pipeline.py b/graphistry/compute/gfql/row/pipeline.py index bf8a805fc8..85bc3f9f56 100644 --- a/graphistry/compute/gfql/row/pipeline.py +++ b/graphistry/compute/gfql/row/pipeline.py @@ -3806,20 +3806,27 @@ def _gfql_add_missing_binding_columns( edges = base_graph._edges if base_graph is not None else None src_col = base_graph._source if base_graph is not None else None dst_col = base_graph._destination if base_graph is not None else None - missing: List[str] = [] + # (column_name, source_series) — carry the source dtype so an edge property like + # `e1.w` stays int64 at 0 rows instead of an untyped object column (which would upcast + # a sum/avg and escape via UNION ALL). Mirrors the node path's typed empty slice. The + # `alias.alias` marker has no source dtype, so None-broadcast is fine for it. + missing: List[Tuple[str, Optional[Any]]] = [] for op in ops: alias = getattr(op, "_name", None) if not isinstance(alias, str): continue - missing.append(f"{alias}.{alias}") # the alias.alias marker + missing.append((f"{alias}.{alias}", None)) # the alias.alias marker if isinstance(op, ASTEdge) and edges is not None: for col in edges.columns: if col in (src_col, dst_col): continue - missing.append(f"{alias}.{col}") - for col in missing: + missing.append((f"{alias}.{col}", edges[col])) + for col, source in missing: if col not in bindings.columns: - bindings[col] = self._gfql_broadcast_scalar(bindings, None) + if source is not None: + bindings[col] = source.iloc[0:0].reindex(bindings.index) + else: + bindings[col] = self._gfql_broadcast_scalar(bindings, None) return bindings def _gfql_connected_bindings_row_frame_from_state( diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 885656da4a..b2be5a3bd9 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15427,6 +15427,23 @@ def test_connected_join_empty_pattern_keeps_edge_alias_aggregate(predicate: str, assert len(result) == 0 +@pytest.mark.parametrize( + "ret,dtype", + [ + # The 0-row schema fill must carry the source dtype: an untyped None gives object, + # which upcasts sum/avg and escapes via UNION ALL. Edge property must stay int64/float. + ("sum(e1.w) AS c", "int64"), + ("avg(e1.w) AS c", "float64"), + ("max(e1.w) AS c", "int64"), + ], +) +def test_connected_join_empty_edge_aggregate_keeps_numeric_dtype(ret: str, dtype: str) -> None: + query = f"MATCH (a)-[e1]->(b), (b)-[e2]->(c) WHERE a.i_col > 99999 RETURN {ret}" + result = _real_edge_alias_graph().gfql(query)._nodes + assert len(result) == 0 + assert str(result["c"].dtype) == dtype + + def test_connected_join_non_empty_edge_alias_aggregate_answers() -> None: query = "MATCH (p)-[e1]->(q), (p)-[e2]->(r) WHERE e1.w > 0 RETURN count(p) AS n" assert _real_edge_alias_graph().gfql(query)._nodes.to_dict(orient="records") == [{"n": 3}] From b790223047fd7eccc987405632cee6aa261ac2f5 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Fri, 17 Jul 2026 10:10:47 -0700 Subject: [PATCH 34/36] fix(gfql): widen downstream node int columns to float at 0 rows Companion to the edge-column dtype fix. A non-anchor node reaches the row via a hop whose unmatched rows introduce NaN, so the non-empty connected join widens its integer columns to float64. The 0-row path sourced them from the base node frame (int64), so an emptied sum(b.i_col) returned int64 where the non-empty run and master give float64 -- observable through UNION ALL, the same class as the edge-column bug: WHERE a.i_col = 999 RETURN sum(b.i_col) master float64 [] -> int64 [] At 0 rows, cast the integer columns of every node op after the first to float64. The anchor never NaN-widens (stays int); float columns are unchanged. sum/max over an emptied non-anchor node property, and the UNION ALL escape, now match master's dtype. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql/row/pipeline.py | 18 +++++++++++++ .../compute/gfql/cypher/test_lowering.py | 27 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/graphistry/compute/gfql/row/pipeline.py b/graphistry/compute/gfql/row/pipeline.py index 85bc3f9f56..eec613c54c 100644 --- a/graphistry/compute/gfql/row/pipeline.py +++ b/graphistry/compute/gfql/row/pipeline.py @@ -3827,6 +3827,24 @@ def _gfql_add_missing_binding_columns( bindings[col] = source.iloc[0:0].reindex(bindings.index) else: bindings[col] = self._gfql_broadcast_scalar(bindings, None) + # Downstream node aliases (every node op after the first) reach the row via a hop + # whose unmatched rows introduce NaN, so the non-empty path widens their integer + # columns to float. The 0-row path sourced them from the base node frame (int), so + # match that widening or an emptied `sum(b.i)` returns int64 where the non-empty run + # and master give float64 -- observable through UNION ALL (#31). + import pandas as _pd + + node_ops = [op for op in ops if isinstance(op, ASTNode)] + for op in node_ops[1:]: + alias = getattr(op, "_name", None) + if not isinstance(alias, str): + continue + for col in [c for c in bindings.columns if c == alias or str(c).startswith(f"{alias}.")]: + try: + if _pd.api.types.is_integer_dtype(bindings[col].dtype): + bindings[col] = bindings[col].astype("float64") + except Exception: + continue return bindings def _gfql_connected_bindings_row_frame_from_state( diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index b2be5a3bd9..ab5302afd7 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15444,6 +15444,33 @@ def test_connected_join_empty_edge_aggregate_keeps_numeric_dtype(ret: str, dtype assert str(result["c"].dtype) == dtype +@pytest.mark.parametrize( + "ret,dtype", + [ + # A downstream (non-anchor) node reaches the row via a hop whose NaN widens its + # integer columns to float in the non-empty run; the 0-row path must match, or an + # emptied sum(b.iv) returns int64 and escapes via UNION ALL (#31). The anchor never + # NaN-widens, so it stays int; float columns are unchanged. + ("sum(b.iv) AS c", "float64"), + ("max(b.iv) AS c", "float64"), + ("sum(a.iv) AS c", "int64"), + ("sum(b.fv) AS c", "float64"), + ], +) +def test_connected_join_empty_node_aggregate_matches_nonempty_dtype(ret: str, dtype: str) -> None: + nodes = pd.DataFrame({ + "id": ["n1", "n2", "n3", "n4"], + "iv": pd.Series([1, 2, 3, 4], dtype="int64"), + "fv": pd.Series([1.0, 2.0, 3.0, 4.0], dtype="float64"), + }) + edges = pd.DataFrame({"s": ["n1", "n2", "n3"], "d": ["n2", "n3", "n4"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + query = f"MATCH (a)-[e1]->(b), (b)-[e2]->(c) WHERE a.iv = 999 RETURN {ret}" + result = g.gfql(query)._nodes + assert len(result) == 0 + assert str(result["c"].dtype) == dtype + + def test_connected_join_non_empty_edge_alias_aggregate_answers() -> None: query = "MATCH (p)-[e1]->(q), (p)-[e2]->(r) WHERE e1.w > 0 RETURN count(p) AS n" assert _real_edge_alias_graph().gfql(query)._nodes.to_dict(orient="records") == [{"n": 3}] From 07720830026d89ef3ed2f7df4c148dbb706fd0ad Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Fri, 17 Jul 2026 10:26:56 -0700 Subject: [PATCH 35/36] fix(gfql): keep extension dtypes + widen bool at 0-row connected join Wave 37 caught two 0-row schema-fidelity defects in the downstream-node widening added for #31: - Finding 1 (regression): `is_integer_dtype` returns True for nullable `Int64`, so `astype("float64")` collapsed it to numpy float64 -- diverging from both the non-empty run (keeps `Int64`, avg -> `Float64`) and master. Guard the cast to numpy int/uint only, skipping extension dtypes, which hold NA natively and are never widened by the hop. - Finding 2: a downstream node's numpy `bool` column is upcast to `object` by the hop's NaN-introducing left-join in the non-empty path, but the 0-row path left it as raw `bool`. Widen numpy bool -> object to match; nullable `boolean` (extension) stays put via the same guard. Both escape via UNION ALL, both were uncovered by the existing tests (plain int64/float64 only). Add nullable-Int64 and bool/boolean regression tests, mutation-verified against non-empty and master. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- graphistry/compute/gfql/row/pipeline.py | 19 +++++-- .../compute/gfql/cypher/test_lowering.py | 57 +++++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/graphistry/compute/gfql/row/pipeline.py b/graphistry/compute/gfql/row/pipeline.py index eec613c54c..6c774ff86c 100644 --- a/graphistry/compute/gfql/row/pipeline.py +++ b/graphistry/compute/gfql/row/pipeline.py @@ -3828,10 +3828,14 @@ def _gfql_add_missing_binding_columns( else: bindings[col] = self._gfql_broadcast_scalar(bindings, None) # Downstream node aliases (every node op after the first) reach the row via a hop - # whose unmatched rows introduce NaN, so the non-empty path widens their integer - # columns to float. The 0-row path sourced them from the base node frame (int), so - # match that widening or an emptied `sum(b.i)` returns int64 where the non-empty run - # and master give float64 -- observable through UNION ALL (#31). + # whose unmatched rows introduce NaN, so the non-empty path widens the columns that + # cannot hold NaN: numpy int -> float64, numpy bool -> object. The 0-row path sourced + # them from the base node frame (int/bool), so match that widening or an emptied + # `sum(b.i)`/`max(b.bv)` returns int64/bool where the non-empty run and master give + # float64/object -- observable through UNION ALL (#31, Wave 37 Finding 2). Extension + # dtypes (`Int64`, `boolean`) hold NA natively and stay put in the non-empty path, so + # they must NOT be touched here (widening them re-introduced the divergence -- Wave 37 + # Finding 1). import pandas as _pd node_ops = [op for op in ops if isinstance(op, ASTNode)] @@ -3841,8 +3845,13 @@ def _gfql_add_missing_binding_columns( continue for col in [c for c in bindings.columns if c == alias or str(c).startswith(f"{alias}.")]: try: - if _pd.api.types.is_integer_dtype(bindings[col].dtype): + dtype = bindings[col].dtype + if _pd.api.types.is_extension_array_dtype(dtype): + continue + if dtype.kind in ("i", "u"): bindings[col] = bindings[col].astype("float64") + elif dtype.kind == "b": + bindings[col] = bindings[col].astype("object") except Exception: continue return bindings diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index ab5302afd7..097a762de9 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15471,6 +15471,63 @@ def test_connected_join_empty_node_aggregate_matches_nonempty_dtype(ret: str, dt assert str(result["c"].dtype) == dtype +@pytest.mark.parametrize( + "ret,dtype", + [ + # A nullable *extension* Int64 on a downstream node stays Int64 through the non-empty + # hop's left-join (avg -> Float64); the 0-row widening must NOT collapse it to numpy + # float64. `pd.api.types.is_integer_dtype(Int64)` is True, so a naive int check + # over-reaches and diverges from both the non-empty run and master -- observable via + # UNION ALL, the same escape as #31. This is the regression Wave 37 caught. + ("sum(b.iv) AS c", "Int64"), + ("max(b.iv) AS c", "Int64"), + ("min(b.iv) AS c", "Int64"), + ("avg(b.iv) AS c", "Float64"), + ], +) +def test_connected_join_empty_node_aggregate_keeps_nullable_int(ret: str, dtype: str) -> None: + nodes = pd.DataFrame({ + "id": ["n1", "n2", "n3", "n4"], + "iv": pd.Series([1, 2, None, 4], dtype="Int64"), + }) + edges = pd.DataFrame({"s": ["n1", "n2", "n3"], "d": ["n2", "n3", "n4"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + query = f"MATCH (a)-[e1]->(b), (b)-[e2]->(c) WHERE a.iv = 999 RETURN {ret}" + empty = g.gfql(query)._nodes + assert len(empty) == 0 + assert str(empty["c"].dtype) == dtype + # The empty schema must match the non-empty run of the identical query. + full_query = f"MATCH (a)-[e1]->(b), (b)-[e2]->(c) RETURN {ret}" + full = g.gfql(full_query)._nodes + assert str(full["c"].dtype) == dtype + + +@pytest.mark.parametrize( + "bv_dtype,expected", + [ + # numpy bool cannot hold the hop's left-join NaN, so the non-empty path widens a + # downstream node's bool column to object; the 0-row path must match, or an emptied + # max(b.bv) returns raw bool where non-empty and master give object (Wave 37 Finding 2). + ("bool", "object"), + # Nullable `boolean` holds NA natively and stays `boolean` in the non-empty path, so it + # must be left untouched -- widening it would be the Finding-1 over-reach in bool form. + ("boolean", "boolean"), + ], +) +def test_connected_join_empty_node_bool_aggregate_matches_nonempty(bv_dtype: str, expected: str) -> None: + nodes = pd.DataFrame({ + "id": ["n1", "n2", "n3", "n4"], + "bv": pd.Series([True, False, True, False], dtype=bv_dtype), + }) + edges = pd.DataFrame({"s": ["n1", "n2", "n3"], "d": ["n2", "n3", "n4"]}) + g = graphistry.nodes(nodes, "id").edges(edges, "s", "d") + empty = g.gfql("MATCH (a)-[e1]->(b), (b)-[e2]->(c) WHERE a.id = 'zzz' RETURN max(b.bv) AS c")._nodes + assert len(empty) == 0 + assert str(empty["c"].dtype) == expected + full = g.gfql("MATCH (a)-[e1]->(b), (b)-[e2]->(c) RETURN max(b.bv) AS c")._nodes + assert str(full["c"].dtype) == expected + + def test_connected_join_non_empty_edge_alias_aggregate_answers() -> None: query = "MATCH (p)-[e1]->(q), (p)-[e2]->(r) WHERE e1.w > 0 RETURN count(p) AS n" assert _real_edge_alias_graph().gfql(query)._nodes.to_dict(orient="records") == [{"n": 3}] From 7f2e5d7bd1d7a3d95a761670460e70de3294dc8c Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Fri, 17 Jul 2026 14:58:29 -0700 Subject: [PATCH 36/36] refactor(gfql): type + relocate connected-join pushdown internals Type/architecture cleanup from code review of the connected-join predicate pushdown. All changes are typing or relocation -- zero runtime-behavior change (1336 cypher+filter_by_dict tests pass; connected-join answers unchanged). - Operators: retype the pushdown gate's `op: str` to the existing `WhereOp` Literal (ast.py); narrow `_connected_join_literal_op` -> `Optional[WhereOp]`; delete both `cast(str, predicate.op)` widenings (predicate.op is already WhereOp). value/dtype/node_dtypes params typed via new aliases. - Aliases: add `DType` (engine-polymorphic column dtype, honestly Any) and `NodeDtypes = Mapping[str, DType]` to compute/typing.py; use across the gate, gfql_unified, and the relocated helpers. - Relocation: move `_LazyNodeDtypes`, `_node_dtypes_for_pushdown`, `_object_column_holds_non_strings`, `_read_node_dtypes` from the gfql_unified dispatch file to compute/filter_by_dict.py -- their true home (it owns the `_is_*_dtype_safe` validators these use and already imports the engine deps). No circular import. Test imports + the `_read_node_dtypes` monkeypatch retarget to filter_by_dict. - Dynamic access: `getattr(g, "_nodes", None)` -> `g._nodes` (Plottable._nodes is a declared attribute) in the relocated helpers. - Deprecated path: `_compile_string_query` now calls `compile_cypher_query` directly instead of threading a private `_node_dtypes` through the deprecated `compile_cypher`; that param is removed, restoring compile_cypher's compat surface (api.py now identical to master). - Surface-guard baseline bumped 9235 -> 9237 for the two new import lines. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UVsx9Fk3BDFvaZXhDvgv5V --- bin/ci_cypher_surface_guard_baseline.json | 2 +- graphistry/compute/filter_by_dict.py | 120 ++++++++++++++++- graphistry/compute/gfql/cypher/api.py | 3 +- graphistry/compute/gfql/cypher/lowering.py | 34 ++--- graphistry/compute/gfql_unified.py | 126 +----------------- graphistry/compute/typing.py | 8 +- .../compute/gfql/cypher/test_lowering.py | 18 +-- 7 files changed, 160 insertions(+), 151 deletions(-) diff --git a/bin/ci_cypher_surface_guard_baseline.json b/bin/ci_cypher_surface_guard_baseline.json index 11e83ed638..72fc38aa20 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": 9235 + "lowering_py_max_lines": 9237 } diff --git a/graphistry/compute/filter_by_dict.py b/graphistry/compute/filter_by_dict.py index 1a28b71b02..837d4f9096 100644 --- a/graphistry/compute/filter_by_dict.py +++ b/graphistry/compute/filter_by_dict.py @@ -1,11 +1,11 @@ -from typing import Any, Dict, Optional, Tuple, Union +from typing import Any, Dict, Mapping, Optional, Tuple, Union, cast import pandas as pd from graphistry.Engine import EngineAbstract, df_to_engine, resolve_engine from graphistry.util import setup_logger from graphistry.Plottable import Plottable from .predicates.ASTPredicate import ASTPredicate -from .typing import DataFrameT +from .typing import DataFrameT, DType, NodeDtypes logger = setup_logger(__name__) @@ -219,3 +219,119 @@ def filter_edges_by_dict(self: Plottable, filter_dict: Optional[dict] = None, en """ edges2 = filter_by_dict(self._edges, filter_dict, engine) return self.edges(edges2) + + +class _LazyNodeDtypes(Mapping[str, DType]): + """Node dtypes, materialized on first lookup. + + Reading them costs a full engine conversion, and only connected-join pushdown ever asks -- + a path most queries never reach. Computing eagerly charged every string query for a frame + it then discarded (~65ms per million polars rows). Stay a Mapping so callers are unchanged. + """ + + def __init__(self, g: Plottable, engine: Union[EngineAbstract, str]) -> None: + self._g = g + self._engine = engine + self._resolved: Optional[NodeDtypes] = None + + def _materialize(self) -> NodeDtypes: + if self._resolved is None: + self._resolved = _read_node_dtypes(self._g, self._engine) + return self._resolved + + def __getitem__(self, key: str) -> DType: + return self._materialize()[key] + + def __iter__(self) -> Any: + return iter(self._materialize()) + + def __len__(self) -> int: + return len(self._materialize()) + + +def _node_dtypes_for_pushdown( + g: Plottable, + engine: Union[EngineAbstract, str] = EngineAbstract.AUTO, +) -> Optional[NodeDtypes]: + """Node dtypes for the pushdown gate, or None when there is no graph to read.""" + if g._nodes is None: + # No graph: the caller falls back to value-type rules, as a bare compile_cypher() does. + # Distinct from failing to read a graph we have, which must fail closed. + return None + return _LazyNodeDtypes(g, engine) + + +def _object_column_holds_non_strings(frame: DataFrameT, column: str, dtype: DType) -> bool: + """Whether `column` is an object column whose values `.str` would reject. + + `object` says nothing about contents: pandas stores ordinary strings that way, and also a + numeric column that acquired a `None`. String predicates are admitted on dtype alone -- by + this gate and by `filter_by_dict` alike -- but `.str` fails on the values, leaking a raw + AttributeError. Dropping the column leaves the gate with nothing to look up, so it fails + closed and the residual answers. + + Keep only the kinds that stay valid under SUBSETTING. `StringMethods._validate` admits + {string, empty, bytes, mixed, mixed-integer}, but it runs on the frame it is handed -- and + the frame we inspect is the source, while the pushed filter runs on the join's candidate + subset. `string` and `empty` survive that (a subset of strings is string or empty), but + `mixed`/`mixed-integer` do not: drop the strings and they collapse to integer/floating/ + boolean, which `.str` rejects. `bytes` is excluded separately -- it passes `_validate` yet + `str.contains` forbids it. + + Mirroring a rule is not enough when we evaluate it against a different frame than the + executor does, so admit only what no subset can invalidate. + """ + import pandas as _pd + + try: + is_object = bool(_pd.api.types.is_object_dtype(dtype)) + except Exception: + return False + if not is_object: + return False + try: + inferred = _pd.api.types.infer_dtype(frame[column], skipna=True) + except Exception: + # It IS an object column and we cannot read its values, so we cannot tell whether + # `.str` would reject them. Omit it and let the residual answer, matching + # `_read_node_dtypes`, which returns {} for a schema it cannot read. + return True + return inferred not in {"string", "empty"} + + +def _read_node_dtypes( + g: Plottable, + engine: Union[EngineAbstract, str] = EngineAbstract.AUTO, +) -> NodeDtypes: + """Column dtypes the executor will filter on, for the pushdown gate. + + A pushed filter is schema-validated against the real column while the row residual + evaluates leniently, so the planner needs dtypes to avoid turning a correct empty result + into a type error. An unreadable schema yields an empty mapping, which fails every column + lookup and so falls back to the residual. + """ + nodes = g._nodes + if nodes is None: + return {} + try: + # Classify the frame the EXECUTOR will filter, not the one the caller handed in. + # `filter_by_dict` validates post-materialization dtypes, so judging the pre-conversion + # frame lets the two disagree (polars Decimal reads numeric here, object there). + # + # Convert the whole frame, not an empty probe: polars -> pandas is DATA-dependent, so + # `head(0)` reports a different class than the real conversion for a nullable Boolean + # (`bool` empty, `object` with a null in it). Identity for pandas; for other engines + # this costs a real conversion, which is why the caller defers it until a pushdown + # decision actually needs it. + probe = df_to_engine(nodes, resolve_engine(cast(Any, engine), nodes), warn=False) + # zip rather than `.items()`: pandas/cuDF expose a column-indexed Series, polars a list. + dtypes = {str(col): dtype for col, dtype in zip(list(probe.columns), list(probe.dtypes))} + return { + col: dtype + for col, dtype in dtypes.items() + if not _object_column_holds_non_strings(probe, col, dtype) + } + except Exception: + # We have a graph but could not read its schema, so we know nothing about any column. + # An empty mapping fails every column lookup, which fails closed to the residual. + return {} diff --git a/graphistry/compute/gfql/cypher/api.py b/graphistry/compute/gfql/cypher/api.py index 733167a38b..c7b660adf4 100644 --- a/graphistry/compute/gfql/cypher/api.py +++ b/graphistry/compute/gfql/cypher/api.py @@ -81,7 +81,6 @@ def compile_cypher( query: str, *, params: Optional[Mapping[str, Any]] = None, - _node_dtypes: Optional[Mapping[str, Any]] = None, _warn_deprecated: bool = True, ) -> Union[CompiledCypherQuery, CompiledCypherUnionQuery, CompiledCypherGraphQuery]: """Deprecated compatibility helper for inspecting compiled Cypher internals. @@ -111,4 +110,4 @@ def compile_cypher( stacklevel=2, ) parsed = parse_cypher(query) - return compile_cypher_query(parsed, params=params, node_dtypes=_node_dtypes) + return compile_cypher_query(parsed, params=params) diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index d5b2f0b699..7f1b56fbd2 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -109,9 +109,11 @@ SourceSpan, UnwindClause, WhereClause, + WhereOp, WherePredicate, WherePatternPredicate, ) +from graphistry.compute.typing import DType, NodeDtypes from graphistry.compute.gfql.cypher._boolean_expr_text import boolean_expr_to_text from graphistry.compute.gfql.cypher.expression_text import ( cypher_literal_expr_text as _cypher_literal_expr_text, @@ -7773,20 +7775,20 @@ def _next_id() -> int: ) -def _connected_join_literal_op(op: str, *, reverse: bool = False) -> Optional[str]: +def _connected_join_literal_op(op: str, *, reverse: bool = False) -> Optional[WhereOp]: normalized = "==" if op == "=" else op if normalized not in {"==", "!=", "<", "<=", ">", ">="}: return None if not reverse: - return normalized - return { + return cast(WhereOp, normalized) + return cast(WhereOp, { "==": "==", "!=": "!=", "<": ">", "<=": ">=", ">": "<", ">=": "<=", - }[normalized] + }[normalized]) def _connected_join_expr_property_ref( @@ -7855,7 +7857,7 @@ def _connected_join_dtype_widens(dtype: Any) -> bool: return False -def _connected_join_dtype_admits(op: str, value: Any, dtype: Any) -> bool: +def _connected_join_dtype_admits(op: WhereOp, value: Optional[Union[str, int, float, bool]], dtype: DType) -> bool: """Whether pushing `op`/`value` onto a column of `dtype` matches residual semantics. A pushed `filter_dict` is schema-validated against the real column, while the row @@ -7890,12 +7892,12 @@ def _connected_join_dtype_admits(op: str, value: Any, dtype: Any) -> bool: def _connected_join_pushable_value( - op: str, + op: WhereOp, value: Optional[CypherLiteral], *, params: Optional[Mapping[str, Any]], column: Optional[str] = None, - node_dtypes: Optional[Mapping[str, Any]] = None, + node_dtypes: Optional[NodeDtypes] = None, ) -> bool: """Whether `op`/`value` survives the trip into a node `filter_dict`. @@ -7949,7 +7951,7 @@ def _connected_join_pushable_value( return _connected_join_dtype_admits(op, resolved, node_dtypes[column]) -def _connected_join_mergeable_filter(target: ASTNode, prop: str, *, op: str, resolved: Any) -> bool: +def _connected_join_mergeable_filter(target: ASTNode, prop: str, *, op: WhereOp, resolved: Optional[Union[str, int, float, bool]]) -> bool: """Whether pushing `op`/`resolved` onto `prop` can merge with what is already there. `_merge_filter_predicates` wraps raw scalars with `comparison.eq`, which serializes to @@ -7980,10 +7982,10 @@ def _apply_connected_join_node_filter( alias_targets_by_pattern: Sequence[Mapping[str, ASTObject]], *, prop_ref: PropertyRef, - op: str, + op: WhereOp, value: Optional[CypherLiteral], params: Optional[Mapping[str, Any]], - node_dtypes: Optional[Mapping[str, Any]] = None, + node_dtypes: Optional[NodeDtypes] = None, ) -> bool: if not _connected_join_pushable_value( op, value, params=params, column=prop_ref.property, node_dtypes=node_dtypes @@ -8025,7 +8027,7 @@ def _pushdown_connected_join_atom_filter( alias_targets: Mapping[str, ASTObject], *, params: Optional[Mapping[str, Any]], - node_dtypes: Optional[Mapping[str, Any]] = None, + node_dtypes: Optional[NodeDtypes] = None, ) -> bool: try: node = _parse_row_expr( @@ -8076,7 +8078,7 @@ def _pushdown_connected_join_where_filters( alias_targets: Mapping[str, ASTObject], *, params: Optional[Mapping[str, Any]], - node_dtypes: Optional[Mapping[str, Any]] = None, + node_dtypes: Optional[NodeDtypes] = None, ) -> Optional[List[ExpressionText]]: if where is None: return [] @@ -8101,7 +8103,7 @@ def _pushdown_connected_join_where_filters( _apply_connected_join_node_filter( alias_targets_by_pattern, prop_ref=predicate.left, - op=cast(str, predicate.op), + op=predicate.op, value=cast(Optional[CypherLiteral], predicate.right), params=params, node_dtypes=node_dtypes, @@ -8148,7 +8150,7 @@ def _walk(expr_node: BooleanExpr) -> None: pushed = _apply_connected_join_node_filter( alias_targets_by_pattern, prop_ref=predicate.left, - op=cast(str, predicate.op), + op=predicate.op, value=cast(Optional[CypherLiteral], predicate.right), params=params, node_dtypes=node_dtypes, @@ -8215,7 +8217,7 @@ def _compile_connected_match_join( *, params: Optional[Mapping[str, Any]] = None, semantic_entity_kinds: Optional[Mapping[str, Literal["node", "edge", "scalar"]]] = None, - node_dtypes: Optional[Mapping[str, Any]] = None, + node_dtypes: Optional[NodeDtypes] = None, ) -> CompiledCypherQuery: clause = query.matches[0] pattern_chains: List[Chain] = [] @@ -8812,7 +8814,7 @@ def compile_cypher_query( query: Union[CypherQuery, CypherUnionQuery, CypherGraphQuery], *, params: Optional[Mapping[str, Any]] = None, - node_dtypes: Optional[Mapping[str, Any]] = None, + node_dtypes: Optional[NodeDtypes] = None, ) -> Union[CompiledCypherQuery, CompiledCypherUnionQuery, CompiledCypherGraphQuery]: from graphistry.compute.gfql.cypher import projection_planning as _projection diff --git a/graphistry/compute/gfql_unified.py b/graphistry/compute/gfql_unified.py index af6c4d5076..179c14c98c 100644 --- a/graphistry/compute/gfql_unified.py +++ b/graphistry/compute/gfql_unified.py @@ -27,7 +27,7 @@ parse_where_json, ) from graphistry.compute.exceptions import ErrorCode, GFQLValidationError -from graphistry.compute.gfql.cypher.api import compile_cypher +from graphistry.compute.gfql.cypher.parser import parse_cypher from graphistry.compute.gfql.cypher.lowering import ( ConnectedMatchJoinPlan, CompiledCypherGraphQuery, @@ -35,7 +35,9 @@ CompiledCypherUnionQuery, CompiledGraphResidualFilter, ConnectedOptionalMatchPlan, + compile_cypher_query, ) +from graphistry.compute.filter_by_dict import _node_dtypes_for_pushdown from graphistry.compute.gfql.cypher.reentry.execution import ( REENTRY_DUPLICATE_CARRIED_ROWS_REASON as _REENTRY_DUPLICATE_CARRIED_ROWS_REASON, REENTRY_WHOLE_ROW_SUGGESTION as _REENTRY_WHOLE_ROW_SUGGESTION, @@ -69,7 +71,7 @@ from graphistry.compute.gfql.passes import DEFAULT_LOGICAL_PASSES, DEFAULT_TIER2_PASSES, PassManager from graphistry.compute.gfql.row.pipeline import _RowPipelineAdapter, is_row_pipeline_call from graphistry.compute.gfql.search_any import search_any_mask -from graphistry.compute.typing import DataFrameT, SeriesT +from graphistry.compute.typing import DataFrameT, SeriesT, NodeDtypes from graphistry.compute.util.generate_safe_column_name import generate_safe_column_name from graphistry.compute.validate.validate_schema import validate_chain_schema from graphistry.compute.gfql_validate import gfql_validate as gfql_preflight_validate @@ -1344,128 +1346,12 @@ def detect_query_type(query: Any) -> QueryType: return "single" -class _LazyNodeDtypes(Mapping[str, Any]): - """Node dtypes, materialized on first lookup. - - Reading them costs a full engine conversion, and only connected-join pushdown ever asks -- - a path most queries never reach. Computing eagerly charged every string query for a frame - it then discarded (~65ms per million polars rows). Stay a Mapping so callers are unchanged. - """ - - def __init__(self, g: Plottable, engine: Union[EngineAbstract, str]) -> None: - self._g = g - self._engine = engine - self._resolved: Optional[Mapping[str, Any]] = None - - def _materialize(self) -> Mapping[str, Any]: - if self._resolved is None: - self._resolved = _read_node_dtypes(self._g, self._engine) - return self._resolved - - def __getitem__(self, key: str) -> Any: - return self._materialize()[key] - - def __iter__(self) -> Any: - return iter(self._materialize()) - - def __len__(self) -> int: - return len(self._materialize()) - - -def _node_dtypes_for_pushdown( - g: Plottable, - engine: Union[EngineAbstract, str] = EngineAbstract.AUTO, -) -> Optional[Mapping[str, Any]]: - """Node dtypes for the pushdown gate, or None when there is no graph to read.""" - if getattr(g, "_nodes", None) is None: - # No graph: the caller falls back to value-type rules, as a bare compile_cypher() does. - # Distinct from failing to read a graph we have, which must fail closed. - return None - return _LazyNodeDtypes(g, engine) - - -def _object_column_holds_non_strings(frame: Any, column: str, dtype: Any) -> bool: - """Whether `column` is an object column whose values `.str` would reject. - - `object` says nothing about contents: pandas stores ordinary strings that way, and also a - numeric column that acquired a `None`. String predicates are admitted on dtype alone -- by - this gate and by `filter_by_dict` alike -- but `.str` fails on the values, leaking a raw - AttributeError. Dropping the column leaves the gate with nothing to look up, so it fails - closed and the residual answers. - - Keep only the kinds that stay valid under SUBSETTING. `StringMethods._validate` admits - {string, empty, bytes, mixed, mixed-integer}, but it runs on the frame it is handed -- and - the frame we inspect is the source, while the pushed filter runs on the join's candidate - subset. `string` and `empty` survive that (a subset of strings is string or empty), but - `mixed`/`mixed-integer` do not: drop the strings and they collapse to integer/floating/ - boolean, which `.str` rejects. `bytes` is excluded separately -- it passes `_validate` yet - `str.contains` forbids it. - - Mirroring a rule is not enough when we evaluate it against a different frame than the - executor does, so admit only what no subset can invalidate. - """ - import pandas as _pd - - try: - is_object = bool(_pd.api.types.is_object_dtype(dtype)) - except Exception: - return False - if not is_object: - return False - try: - inferred = _pd.api.types.infer_dtype(frame[column], skipna=True) - except Exception: - # It IS an object column and we cannot read its values, so we cannot tell whether - # `.str` would reject them. Omit it and let the residual answer, matching - # `_read_node_dtypes`, which returns {} for a schema it cannot read. - return True - return inferred not in {"string", "empty"} - - -def _read_node_dtypes( - g: Plottable, - engine: Union[EngineAbstract, str] = EngineAbstract.AUTO, -) -> Mapping[str, Any]: - """Column dtypes the executor will filter on, for the pushdown gate. - - A pushed filter is schema-validated against the real column while the row residual - evaluates leniently, so the planner needs dtypes to avoid turning a correct empty result - into a type error. An unreadable schema yields an empty mapping, which fails every column - lookup and so falls back to the residual. - """ - nodes = getattr(g, "_nodes", None) - if nodes is None: - return {} - try: - # Classify the frame the EXECUTOR will filter, not the one the caller handed in. - # `filter_by_dict` validates post-materialization dtypes, so judging the pre-conversion - # frame lets the two disagree (polars Decimal reads numeric here, object there). - # - # Convert the whole frame, not an empty probe: polars -> pandas is DATA-dependent, so - # `head(0)` reports a different class than the real conversion for a nullable Boolean - # (`bool` empty, `object` with a null in it). Identity for pandas; for other engines - # this costs a real conversion, which is why the caller defers it until a pushdown - # decision actually needs it. - probe = df_to_engine(nodes, resolve_engine(cast(Any, engine), nodes), warn=False) - # zip rather than `.items()`: pandas/cuDF expose a column-indexed Series, polars a list. - dtypes = {str(col): dtype for col, dtype in zip(list(probe.columns), list(probe.dtypes))} - return { - col: dtype - for col, dtype in dtypes.items() - if not _object_column_holds_non_strings(probe, col, dtype) - } - except Exception: - # We have a graph but could not read its schema, so we know nothing about any column. - # An empty mapping fails every column lookup, which fails closed to the residual. - return {} - - def _compile_string_query( query: str, *, language: Optional[Literal["cypher", "gremlin"]], params: Optional[Mapping[str, Any]], - node_dtypes: Optional[Mapping[str, Any]] = None, + node_dtypes: Optional[NodeDtypes] = None, ) -> Any: query_language = language or "cypher" if query_language != "cypher": @@ -1477,7 +1363,7 @@ def _compile_string_query( suggestion="Use language='cypher' for now; Gremlin string compilation is not implemented yet.", language="gfql", ) - return compile_cypher(query, params=params, _node_dtypes=node_dtypes, _warn_deprecated=False) + return compile_cypher_query(parse_cypher(query), params=params, node_dtypes=node_dtypes) def _compile_value_repr(value: Any) -> str: diff --git a/graphistry/compute/typing.py b/graphistry/compute/typing.py index d17805fc0b..a72c287274 100644 --- a/graphistry/compute/typing.py +++ b/graphistry/compute/typing.py @@ -1,5 +1,5 @@ import pandas as pd -from typing import Any, Protocol, TYPE_CHECKING, Tuple, TypeVar, Union +from typing import Any, Mapping, Protocol, TYPE_CHECKING, Tuple, TypeVar, Union # TODO stubs for Union[cudf.DataFrame, dask.DataFrame, ..] at checking time if TYPE_CHECKING: @@ -13,6 +13,12 @@ IndexT = Any DomainT = Any +# Engine-polymorphic column dtype: numpy dtype / pandas ExtensionDtype / polars DataType. +# Honestly Any -- the concrete type is engine-dependent and only ever passed to dtype-inspection +# helpers that accept Any and fail closed. +DType = Any +NodeDtypes = Mapping[str, DType] + # Type variable for return type preservation in predicates T = TypeVar('T') diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 097a762de9..6a11c5cadb 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -38,13 +38,13 @@ ) from graphistry.plugins.networkx.policy import NETWORKX_SCIPY_EXTRA_REQUIREMENTS, NETWORKX_VERSION_SPEC, SCIPY_VERSION_SPEC from graphistry.compute.gfql.cypher.ast import ExpressionText, OrderByClause, OrderItem, ReturnClause, ReturnItem, SourceSpan -from graphistry.compute.gfql.cypher.lowering import CompiledCypherExecutionExtras, CompiledCypherGraphQuery +from graphistry.compute.gfql.cypher.lowering import CompiledCypherExecutionExtras, CompiledCypherGraphQuery, compile_cypher_query from graphistry.compute.gfql.cypher.lowering import _logical_plan_route_for_query from graphistry.compute.gfql.frontends.cypher.binder import FrontendBinder from graphistry.compute.gfql.ir.bound_ir import BoundIR, BoundQueryPart, SemanticTable from graphistry.compute.gfql.ir.compilation import PlanContext from graphistry.compute.gfql.ir.logical_plan import CHILD_SLOTS, Filter, PatternMatch, ProcedureCall as LogicalProcedureCall -from graphistry.compute.gfql_unified import _node_dtypes_for_pushdown +from graphistry.compute.filter_by_dict import _node_dtypes_for_pushdown from graphistry.compute.gfql.cypher.lowering import _connected_join_dtype_admits, _connected_join_dtype_classes from graphistry.Plottable import Plottable from graphistry.tests.test_compute import CGFull @@ -15537,7 +15537,7 @@ def test_object_column_holds_non_strings_fails_closed_when_unreadable() -> None: # An object column whose values cannot be read tells us nothing about whether `.str` would # reject them, so omit it and let the residual answer -- matching `_read_node_dtypes`, which # returns {} for a schema it cannot read. Keeping it would push blind. - from graphistry.compute.gfql_unified import _object_column_holds_non_strings + from graphistry.compute.filter_by_dict import _object_column_holds_non_strings class _UnreadableFrame: def __getitem__(self, key: str) -> Any: @@ -15674,17 +15674,17 @@ def test_node_dtypes_for_pushdown_defers_the_conversion() -> None: nodes = pl.DataFrame({"id": ["a", "b"], "age": pl.Series([1, 2], dtype=pl.Int64)}) g = graphistry.nodes(nodes, "id").edges(pl.DataFrame({"s": ["a"], "d": ["b"]}), "s", "d") - import graphistry.compute.gfql_unified as unified + import graphistry.compute.filter_by_dict as filter_by_dict - original = unified._read_node_dtypes + original = filter_by_dict._read_node_dtypes def spy(*args: Any, **kwargs: Any) -> Any: converted.append(1) return original(*args, **kwargs) - unified._read_node_dtypes = spy + filter_by_dict._read_node_dtypes = spy try: - dtypes = unified._node_dtypes_for_pushdown(g) + dtypes = filter_by_dict._node_dtypes_for_pushdown(g) assert dtypes is not None assert converted == [] # built, but nothing read yet assert _connected_join_dtype_classes(dtypes["age"]) == (True, False) @@ -15692,7 +15692,7 @@ def spy(*args: Any, **kwargs: Any) -> Any: _ = dtypes["id"] assert converted == [1] # and it is cached finally: - unified._read_node_dtypes = original + filter_by_dict._read_node_dtypes = original def test_node_dtypes_for_pushdown_matches_the_full_conversion() -> None: @@ -15872,7 +15872,7 @@ def test_connected_join_dtype_schema_selects_pushdown() -> None: assert dtypes is not None def pushed_props(predicate: str) -> set: - compiled = cast(CompiledCypherQuery, compile_cypher(_t1_nullable_query(predicate), _node_dtypes=dtypes)) + compiled = cast(CompiledCypherQuery, compile_cypher_query(parse_cypher(_t1_nullable_query(predicate)), node_dtypes=dtypes)) plan = _compiled_execution_extras(compiled).connected_match_join assert plan is not None return {