From ce0979f48fb66005f1539b3b06878b3543d0ef8a Mon Sep 17 00:00:00 2001 From: lmeyerov Date: Tue, 20 Oct 2026 21:47:13 -0700 Subject: [PATCH] fix(gfql): preserve quoted ORDER BY aggregate aliases Rebase accepted PR #2208 source 5922f0a7c4c3a5ecb28c7e091f23420517dcbe9a onto #2207 master bf2edb29822fad6b31f482c7203087325c92c7a1. --- .github/workflows/ci.yml | 6 +- CHANGELOG.md | 1 + graphistry/compute/gfql/cypher/lowering.py | 67 +- .../gfql/cypher/test_backtick_identifiers.py | 637 ++++++++++++++++++ 4 files changed, 698 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 87a19b8cbd..9faeb64f7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1760,9 +1760,9 @@ jobs: needs: [changes, test-minimal-python, generate-lockfiles] if: ${{ ((needs.changes.outputs.python == 'true' && needs.changes.outputs.narrow_python_only != 'true') || needs.changes.outputs.gfql == 'true' || needs.changes.outputs.pandas_compat == 'true' || needs.changes.outputs.core == 'true' || needs.changes.outputs.infra == 'true' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule') && !(needs.changes.outputs.docs_only_latest == 'true' && (github.event_name == 'push' || github.event_name == 'pull_request')) }} runs-on: ubuntu-latest - # The py3.12 coverage cell can exceed 15 minutes including setup and - # artifact work; retain enough margin for a completed test verdict. - timeout-minutes: 20 + # The py3.12 coverage cell can exceed 20 minutes including setup, + # two test phases, and coverage artifact work. + timeout-minutes: 30 strategy: matrix: diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b606a4361..6c32ed6bf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **Chained `compute_igraph` calls preserve custom-named edge attributes**: edge attributes named `source` or `target` are restored to their original column names after a round trip when endpoint bindings use other names, so repeated igraph calls no longer accumulate collision renames or fail on duplicate columns. Attributes that collide with the final endpoint bindings are moved to unique generated names rather than merging with endpoint columns (#2204). +- **GFQL Cypher quoted ORDER BY aliases**: quoted aliases containing spaces or escaped backticks can now order values in pandas, Polars, and cuDF lowering, including aggregate outputs. Quoted output names keep their own binding identity when their text matches another aggregate expression, and unresolved quoted aliases retain structured validation errors (#2205). ## [0.61.0] - 2026-10-18 diff --git a/graphistry/compute/gfql/cypher/lowering.py b/graphistry/compute/gfql/cypher/lowering.py index 513dc28213..76b1a5856d 100644 --- a/graphistry/compute/gfql/cypher/lowering.py +++ b/graphistry/compute/gfql/cypher/lowering.py @@ -169,7 +169,7 @@ from graphistry.compute.gfql.cypher.reentry import naming as _reentry_naming, scope as _reentry_scope from graphistry.compute.gfql.cypher.ast import CypherParams from graphistry.compute.gfql.cypher.where_membership import peel_literal_membership_where -from graphistry.compute.gfql.identifiers import render_cypher_identifier, shortest_path_hops_column +from graphistry.compute.gfql.identifiers import parse_cypher_backtick_identifier, render_cypher_identifier, shortest_path_hops_column @dataclass(frozen=True) @@ -1245,6 +1245,7 @@ def _rewrite(node_in: ExprNode) -> ExprNode: def _rewrite_expr_to_output_names( expr_text: str, *, + available_columns: AbstractSet[str], replacements: Mapping[str, str], params: Optional[Mapping[str, Any]], field: str, @@ -1260,12 +1261,16 @@ def _rewrite_expr_to_output_names( ) def _rewrite(node_in: ExprNode) -> ExprNode: + if isinstance(node_in, Identifier): + if node_in.name in available_columns: + return Identifier(node_in.name, quoted=node_in.quoted) + replacement = replacements.get(node_in.name) + if replacement is not None: + return Identifier(replacement, quoted=node_in.quoted) rendered = _render_expr_node(node_in) replacement = replacements.get(rendered) if replacement is not None: - return Identifier(replacement) - if isinstance(node_in, Identifier): - return Identifier(replacements.get(node_in.name, node_in.name)) + return Identifier(replacement, quoted=True) return _rebuild_expr_node(node_in, rewrite=_rewrite, error_context="output rewrite") return _render_expr_node(_rewrite(node)) @@ -4115,6 +4120,10 @@ def _lower_order_by_clause( ) else: order_key = alias_name + if order_key == alias_name and not ( + plan.whole_row_output_names and alias_name in plan.output_to_expr_source + ): + order_key = render_cypher_identifier(order_key) else: source_alias = ( plan.whole_row_sources.get(alias_name, plan.source_alias) @@ -4223,10 +4232,18 @@ def _lower_order_by_outputs( ) -> ASTObject: keys: List[Tuple[str, str]] = [] for item in clause.items: - order_key = expr_to_output.get(item.expression.text) + order_node = _parse_row_expr(item.expression.text, params=params, field="order_by", line=item.span.line, column=item.span.column) + quoted_alias = order_node.name if isinstance(order_node, Identifier) and order_node.quoted else None + if isinstance(order_node, Identifier) and order_node.name in available_columns: + keys.append((render_cypher_identifier(order_node.name), item.direction)) + continue + order_key: Optional[str] = expr_to_output.get(item.expression.text) + if order_key is None and quoted_alias is not None and quoted_alias in available_columns: + order_key = render_cypher_identifier(quoted_alias) if order_key is None: order_key = _rewrite_expr_to_output_names( item.expression.text, + available_columns=available_columns, replacements=expr_to_output, params=params, field="order_by", @@ -4234,6 +4251,8 @@ def _lower_order_by_outputs( column=item.span.column, ) _reject_order_by_aggregate(item, expr_text=order_key, params=params) + if order_key != item.expression.text and order_key in available_columns: + order_key = render_cypher_identifier(order_key) if order_key not in available_columns: node = _parse_row_expr( order_key, @@ -4913,6 +4932,35 @@ def _lower_match_alias_stage( ) row_steps: List[ASTObject] = [] defer_return_projection = False + order_projection_items = list(plan.projection_items) + order_plan: _ProjectionPlan = plan + if stage.order_by is not None: + order_plan = _projection._plan_with_visible_projected_columns(plan, scope.projected_columns) + order_output_names = _projection._projection_output_names(plan) + for order_item in stage.order_by.items: + try: + order_alias, order_prop = _projection._projection_ref_from_expr( + order_item.expression.text, + alias_targets=scope.alias_targets, + params=params, + field="order_by", + line=order_item.span.line, + column=order_item.span.column, + ) + except GFQLValidationError: + continue + if ( + order_prop is None + and order_alias is not None + and order_alias not in order_output_names + and order_alias in order_plan.available_columns + and order_alias in scope.projected_columns + ): + order_binding = scope.projected_columns[order_alias] + if order_binding.source_name is not None: + order_projection_items.append((order_alias, order_binding.source_name)) + order_output_names.add(order_alias) + order_plan = replace(order_plan, projection_items=order_projection_items) if not plan.whole_row_output_names: projection_fn = with_ if stage.clause.kind == "with" else return_ if ( @@ -4922,7 +4970,7 @@ def _lower_match_alias_stage( ): # Keep pre-existing columns alive for ORDER BY references that are # intentionally not part of the final RETURN projection. - row_steps.append(with_(plan.projection_items, extend=True)) + row_steps.append(with_(order_projection_items, extend=True)) defer_return_projection = True else: row_steps.append(projection_fn(plan.projection_items)) @@ -4971,7 +5019,6 @@ def _lower_match_alias_stage( ) ) if stage.order_by is not None: - order_plan = _projection._plan_with_visible_projected_columns(plan, scope.projected_columns) row_steps.append( _lower_order_by_clause( stage.order_by, @@ -5245,7 +5292,7 @@ def _lower_match_alias_aggregate_stage( expr_to_output, source_expr=agg_spec.source_text, output_name=agg_spec.output_name, - alias_name=agg_spec.output_name, + alias_name=None, ) bindings_row_path = bool(scope.allowed_match_aliases) @@ -5817,7 +5864,7 @@ def _lower_row_column_aggregate_stage( expr_to_output, source_expr=agg_spec.source_text, output_name=agg_spec.output_name, - alias_name=agg_spec.output_name, + alias_name=None, ) row_steps: List[ASTObject] = [] @@ -7470,7 +7517,7 @@ def _lower_general_row_projection( expr_to_output, source_expr=agg_spec.source_text, output_name=agg_spec.output_name, - alias_name=agg_spec.output_name, + alias_name=None, ) if not binding_row_aliases: diff --git a/graphistry/tests/compute/gfql/cypher/test_backtick_identifiers.py b/graphistry/tests/compute/gfql/cypher/test_backtick_identifiers.py index 12d36200f9..b2dc335477 100644 --- a/graphistry/tests/compute/gfql/cypher/test_backtick_identifiers.py +++ b/graphistry/tests/compute/gfql/cypher/test_backtick_identifiers.py @@ -4,6 +4,7 @@ import pytest import graphistry +from graphistry.compute.exceptions import GFQLValidationError from graphistry.compute.gfql.identifiers import CYPHER_RESERVED_WORDS from graphistry.compute.gfql.cypher.parser import parse_cypher from graphistry.compute.gfql.expr_parser import Identifier, parse_expr @@ -90,6 +91,575 @@ def test_backtick_alias_survives_where_order_and_projection(odd_columns_graph): assert out._nodes["out"].tolist() == [20] +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +def test_quoted_return_alias_survives_order(odd_columns_graph, engine): + if engine == "polars": + pytest.importorskip("polars") + + out = odd_columns_graph.gfql( + "MATCH (n) RETURN n.`a-b` AS `sort id` ORDER BY `sort id` DESC", + engine=engine, + ) + + assert out._nodes["sort id"].to_list() == [20, 10] + + +def test_quoted_return_alias_survives_order_on_cudf(): + cudf = pytest.importorskip("cudf") + nodes = cudf.DataFrame({"id": ["a", "b"], "a-b": [10, 20]}) + edges = cudf.DataFrame({"src": ["a", "b"], "dst": ["b", "a"]}) + graph = graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + out = graph.gfql( + "MATCH (n) RETURN n.`a-b` AS `sort id` ORDER BY `sort id` DESC", + engine="cudf", + ) + + assert type(out._nodes).__module__.startswith("cudf") + assert out._nodes["sort id"].to_pandas().tolist() == [20, 10] + + +def test_quoted_output_alias_does_not_collide_with_expression_text(): + nodes = pd.DataFrame({"id": ["a", "b", "c"], "num": [3, 1, 2]}) + edges = pd.DataFrame({"src": ["a", "b", "c"], "dst": ["b", "c", "a"]}) + graph = graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + out = graph.gfql( + "MATCH(a) RETURN a, a.num + 1 AS `(a.num + 1)` " + "ORDER BY `(a.num + 1)` DESC", + engine="pandas", + ) + + assert out._nodes["(a.num + 1)"].tolist() == [4, 3, 2] + + +def test_quoted_output_alias_survives_explicit_with_realias(): + nodes = pd.DataFrame({"id": ["a", "b", "c"], "num": [3, 1, 2]}) + edges = pd.DataFrame({"src": ["a", "b", "c"], "dst": ["b", "c", "a"]}) + graph = graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + out = graph.gfql( + "MATCH(a) WITH a, a.num + 1 AS `(a.num + 1)` " + "WITH a, `(a.num + 1)` AS `(a.num + 1)` " + "ORDER BY `(a.num + 1)` DESC " + "RETURN a, `(a.num + 1)` AS `(a.num + 1)`", + engine="pandas", + ) + + assert out._nodes["(a.num + 1)"].tolist() == [4, 3, 2] + + +def test_quoted_hidden_order_key_survives_order_on_cudf(): + cudf = pytest.importorskip("cudf") + nodes = cudf.DataFrame({"id": ["a", "b", "c"], "score": [3, 1, 2]}) + edges = cudf.DataFrame({"src": ["a"], "dst": ["b"]}) + graph = graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + out = graph.gfql( + "MATCH(n) " + "WITH n.id AS id, n.score AS `sort id` " + "RETURN id ORDER BY `sort id` DESC", + engine="cudf", + ) + + assert type(out._nodes).__module__.startswith("cudf") + assert out._nodes["id"].to_pandas().tolist() == ["a", "c", "b"] + + +def test_quoted_aggregate_alias_survives_order(odd_columns_graph): + out = odd_columns_graph.gfql( + "MATCH (n) RETURN count(*) AS `count value` ORDER BY `count value` DESC", + engine="pandas", + ) + + assert out._nodes["count value"].tolist() == [2] + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize( + ("query", "expected_category", "expected_aggregate"), + [ + ( + "MATCH(n) RETURN n.category AS category, count(*) AS `count(*)` ORDER BY `count(*)` ASC, category ASC", + ["y", "x"], + [1, 3], + ), + ( + "MATCH(n) RETURN n.category AS category, sum(n.num) AS `sum(n.num)` ORDER BY `sum(n.num)` ASC, category ASC", + ["y", "x"], + [5, 6], + ), + ], +) +def test_quoted_aggregate_output_aliases_survive_order( + aggregate_graph, + engine, + query, + expected_category, + expected_aggregate, +): + if engine == "polars": + pytest.importorskip("polars") + + out = aggregate_graph.gfql(query, engine=engine) + + assert out._nodes["category"].to_list() == expected_category + assert out._nodes["count(*)" if "count(*)" in query else "sum(n.num)"].to_list() == expected_aggregate + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize( + "query", + [ + "MATCH(n) RETURN n.category AS category ORDER BY count(*) ASC", + "MATCH(n) RETURN n.category AS category ORDER BY sum(n.num) ASC", + ], +) +def test_unprojected_aggregate_order_is_rejected(boundary_graph, engine, query): + if engine == "polars": + pytest.importorskip("polars") + + with pytest.raises(GFQLValidationError) as raised: + boundary_graph.gfql(query, engine=engine) + + error = raised.value + assert error.code == "unsupported-cypher-query" + assert error.context["field"] == "order_by" + + +@pytest.fixture +def unwind_aggregate_graph(): + nodes = pd.DataFrame({"id": [0]}) + return graphistry.nodes(nodes, "id") + + +@pytest.fixture +def unwind_match_graph(): + nodes = pd.DataFrame({"id": ["a", "b"]}) + edges = pd.DataFrame({"src": ["a"], "dst": ["b"]}) + return graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize( + ("query_prefix", "order_key", "expected_g", "expected_count", "expected_sum"), + [ + ("UNWIND [1, 1, 10] AS x", "`sum(x)`", [10, 1], [1, 2], [10, 2]), + ("UNWIND [1, 1, 10] AS x", "sum(x)", [1, 10], [2, 1], [2, 10]), + ("UNWIND [1, 1, 10] AS x", "`count(*)`", [1, 10], [2, 1], [2, 10]), + ("UNWIND [1, 1, 10] AS x", "count(*)", [10, 1], [1, 2], [10, 2]), + ("MATCH(n) UNWIND [1, 1, 10] AS x", "`sum(x)`", [10, 1], [2, 4], [20, 4]), + ("MATCH(n) UNWIND [1, 1, 10] AS x", "sum(x)", [1, 10], [4, 2], [4, 20]), + ("MATCH(n) UNWIND [1, 1, 10] AS x", "`count(*)`", [1, 10], [4, 2], [4, 20]), + ("MATCH(n) UNWIND [1, 1, 10] AS x", "count(*)", [10, 1], [2, 4], [20, 4]), + ], +) +def test_swapped_aggregate_aliases_across_callers( + unwind_aggregate_graph, + unwind_match_graph, + engine, + query_prefix, + order_key, + expected_g, + expected_count, + expected_sum, +): + if engine == "polars": + pytest.importorskip("polars") + + graph = unwind_match_graph if query_prefix.startswith("MATCH") else unwind_aggregate_graph + query = ( + f"{query_prefix} " + "RETURN x AS g, count(*) AS `sum(x)`, sum(x) AS `count(*)` " + f"ORDER BY {order_key} ASC" + ) + + out = graph.gfql(query, engine=engine) + + assert out._nodes["g"].to_list() == expected_g + assert out._nodes["sum(x)"].to_list() == expected_count + assert out._nodes["count(*)"].to_list() == expected_sum + + +def test_swapped_unwind_aggregate_aliases_on_cudf(): + cudf = pytest.importorskip("cudf") + graph = graphistry.nodes(cudf.DataFrame({"id": [0]}), "id") + + out = graph.gfql( + "UNWIND [1, 1, 10] AS x " + "RETURN x AS g, count(*) AS `sum(x)`, sum(x) AS `count(*)` " + "ORDER BY sum(x) ASC", + engine="cudf", + ) + + assert type(out._nodes).__module__.startswith("cudf") + assert out._nodes["g"].to_pandas().tolist() == [1, 10] + assert out._nodes["sum(x)"].to_pandas().tolist() == [2, 1] + assert out._nodes["count(*)"].to_pandas().tolist() == [2, 10] + + +def test_swapped_match_unwind_aggregate_aliases_on_cudf(): + cudf = pytest.importorskip("cudf") + graph = graphistry.nodes(cudf.DataFrame({"id": ["a", "b"]}), "id").edges( + cudf.DataFrame({"src": ["a"], "dst": ["b"]}), "src", "dst" + ) + + out = graph.gfql( + "MATCH(n) UNWIND [1, 1, 10] AS x " + "RETURN x AS g, count(*) AS `sum(x)`, sum(x) AS `count(*)` " + "ORDER BY `sum(x)` ASC", + engine="cudf", + ) + + assert type(out._nodes).__module__.startswith("cudf") + assert out._nodes["g"].to_pandas().tolist() == [10, 1] + assert out._nodes["sum(x)"].to_pandas().tolist() == [2, 4] + assert out._nodes["count(*)"].to_pandas().tolist() == [20, 4] + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize( + "query", + [ + "UNWIND [1, 1, 10] AS x RETURN x AS g ORDER BY count(*) ASC", + "MATCH(n) UNWIND [1, 1, 10] AS x RETURN n.id AS g ORDER BY count(*) ASC", + ], +) +def test_unprojected_aggregate_order_is_rejected_across_callers( + unwind_aggregate_graph, + unwind_match_graph, + engine, + query, +): + if engine == "polars": + pytest.importorskip("polars") + + graph = unwind_match_graph if query.startswith("MATCH") else unwind_aggregate_graph + + with pytest.raises(GFQLValidationError) as raised: + graph.gfql(query, engine=engine) + + error = raised.value + assert error.code == "unsupported-cypher-query" + assert error.context["field"] == "order_by" + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize( + ("order_key", "expected_category", "expected_count", "expected_sum"), + [ + ("`sum(n.num)`", ["y", "x"], [1, 3], [10, 3]), + ("`count(*)`", ["x", "y"], [3, 1], [3, 10]), + ("count(*)", ["y", "x"], [1, 3], [10, 3]), + ("sum(n.num)", ["x", "y"], [3, 1], [3, 10]), + ], +) +def test_aggregate_output_alias_kinds_keep_independent_sort_order( + aggregate_alias_graph, + engine, + order_key, + expected_category, + expected_count, + expected_sum, +): + if engine == "polars": + pytest.importorskip("polars") + + out = aggregate_alias_graph.gfql( + "MATCH(n) " + "RETURN n.category AS category, count(*) AS `sum(n.num)`, sum(n.num) AS `count(*)` " + f"ORDER BY {order_key} ASC", + engine=engine, + ) + + assert out._nodes["category"].to_list() == expected_category + assert out._nodes["sum(n.num)"].to_list() == expected_count + assert out._nodes["count(*)"].to_list() == expected_sum + + +@pytest.mark.parametrize( + ("order_key", "expected_category", "expected_count", "expected_sum"), + [ + ("`sum(n.num)`", ["y", "x"], [1, 3], [10, 3]), + ("`count(*)`", ["x", "y"], [3, 1], [3, 10]), + ("count(*)", ["y", "x"], [1, 3], [10, 3]), + ("sum(n.num)", ["x", "y"], [3, 1], [3, 10]), + ], +) +def test_aggregate_output_alias_kinds_on_cudf( + order_key, + expected_category, + expected_count, + expected_sum, +): + cudf = pytest.importorskip("cudf") + nodes = cudf.DataFrame( + {"id": ["a", "b", "c", "d"], "num": [1, 1, 1, 10], "category": ["x", "x", "x", "y"]} + ) + edges = cudf.DataFrame({"src": ["a"], "dst": ["b"]}) + graph = graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + out = graph.gfql( + "MATCH(n) " + "RETURN n.category AS category, count(*) AS `sum(n.num)`, sum(n.num) AS `count(*)` " + f"ORDER BY {order_key} ASC", + engine="cudf", + ) + + assert type(out._nodes).__module__.startswith("cudf") + assert out._nodes["category"].to_pandas().tolist() == expected_category + assert out._nodes["sum(n.num)"].to_pandas().tolist() == expected_count + assert out._nodes["count(*)"].to_pandas().tolist() == expected_sum + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize( + ("alias", "order_key", "expected_category", "expected_aggregate"), + [ + ("c", "`c` + 1", ["y", "x"], [1, 3]), + ("the count", "`the count` + 1", ["y", "x"], [1, 3]), + ("sum(n.num)", "`sum(n.num)` + 1", ["y", "x"], [1, 3]), + ("c", "-`c`", ["x", "y"], [3, 1]), + ("the count", "-`the count`", ["x", "y"], [3, 1]), + ("sum(n.num)", "-`sum(n.num)`", ["x", "y"], [3, 1]), + ("c", "abs(`c`)", ["y", "x"], [1, 3]), + ("the count", "abs(`the count`)", ["y", "x"], [1, 3]), + ("sum(n.num)", "abs(`sum(n.num)`)", ["y", "x"], [1, 3]), + ("c", "count(*) + 1", ["y", "x"], [1, 3]), + ("the count", "count(*) + 1", ["y", "x"], [1, 3]), + ("sum(n.num)", "count(*) + 1", ["y", "x"], [1, 3]), + ], +) +def test_compound_aggregate_order_keys_preserve_output_bindings( + aggregate_alias_graph, + engine, + alias, + order_key, + expected_category, + expected_aggregate, +): + if engine == "polars": + pytest.importorskip("polars") + + out = aggregate_alias_graph.gfql( + "MATCH(n) " + f"RETURN n.category AS category, count(*) AS `{alias}` " + f"ORDER BY {order_key} ASC", + engine=engine, + ) + + assert out._nodes["category"].to_list() == expected_category + assert out._nodes[alias].to_list() == expected_aggregate + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize( + ("order_key", "expected_category", "expected_count", "expected_sum"), + [ + ("`sum(n.num)` + 1", ["y", "x"], [1, 3], [10, 3]), + ("`count(*)` + 1", ["x", "y"], [3, 1], [3, 10]), + ("count(*) + 1", ["y", "x"], [1, 3], [10, 3]), + ("sum(n.num) + 1", ["x", "y"], [3, 1], [3, 10]), + ("-`sum(n.num)`", ["x", "y"], [3, 1], [3, 10]), + ("-`count(*)`", ["y", "x"], [1, 3], [10, 3]), + ("-count(*)", ["x", "y"], [3, 1], [3, 10]), + ("-sum(n.num)", ["y", "x"], [1, 3], [10, 3]), + ("abs(`sum(n.num)`)", ["y", "x"], [1, 3], [10, 3]), + ("abs(`count(*)`)", ["x", "y"], [3, 1], [3, 10]), + ("abs(count(*))", ["y", "x"], [1, 3], [10, 3]), + ("abs(sum(n.num))", ["x", "y"], [3, 1], [3, 10]), + ], +) +def test_compound_swapped_aggregate_order_keys_preserve_bindings( + aggregate_alias_graph, + engine, + order_key, + expected_category, + expected_count, + expected_sum, +): + if engine == "polars": + pytest.importorskip("polars") + + out = aggregate_alias_graph.gfql( + "MATCH(n) " + "RETURN n.category AS category, " + "count(*) AS `sum(n.num)`, sum(n.num) AS `count(*)` " + f"ORDER BY {order_key} ASC", + engine=engine, + ) + + assert out._nodes["category"].to_list() == expected_category + assert out._nodes["sum(n.num)"].to_list() == expected_count + assert out._nodes["count(*)"].to_list() == expected_sum + + +@pytest.fixture +def shadowed_aggregate_alias_graph(): + nodes = pd.DataFrame( + { + "id": ["a", "b", "c", "d"], + "num": [1, 1, 1, 10], + "score": [1, 1, 1, 1], + } + ) + edges = pd.DataFrame({"src": ["a"], "dst": ["b"]}) + return graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize( + ("order_key", "expected_group", "expected_aggregate"), + [ + ("v", [10, 1], [1, 3]), + ("`v`", [10, 1], [1, 3]), + ("v + 1", [10, 1], [1, 3]), + ("`v` + 1", [10, 1], [1, 3]), + ("sum(w) + 1", [10, 1], [1, 3]), + ("group_value + 1", [1, 10], [3, 1]), + ], +) +def test_projected_identifier_precedes_aggregate_alias_rewrite( + shadowed_aggregate_alias_graph, + engine, + order_key, + expected_group, + expected_aggregate, +): + if engine == "polars": + pytest.importorskip("polars") + + out = shadowed_aggregate_alias_graph.gfql( + "MATCH(n) " + "WITH n.num AS v, n.score AS w " + "RETURN v AS group_value, sum(w) AS v " + f"ORDER BY {order_key} ASC", + engine=engine, + ) + + assert out._nodes["group_value"].to_list() == expected_group + assert out._nodes["v"].to_list() == expected_aggregate + + +@pytest.mark.parametrize( + "order_key", + ["v", "`v`", "v + 1", "`v` + 1"], +) +def test_projected_identifier_precedes_aggregate_alias_rewrite_on_cudf( + order_key, +): + cudf = pytest.importorskip("cudf") + nodes = cudf.DataFrame( + { + "id": ["a", "b", "c", "d"], + "num": [1, 1, 1, 10], + "score": [1, 1, 1, 1], + } + ) + edges = cudf.DataFrame({"src": ["a"], "dst": ["b"]}) + graph = graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + out = graph.gfql( + "MATCH(n) " + "WITH n.num AS v, n.score AS w " + "RETURN v AS group_value, sum(w) AS v " + f"ORDER BY {order_key} ASC", + engine="cudf", + ) + + assert type(out._nodes).__module__.startswith("cudf") + assert out._nodes["group_value"].to_pandas().tolist() == [10, 1] + assert out._nodes["v"].to_pandas().tolist() == [1, 3] + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize("alias", ["c", "the count"]) +@pytest.mark.parametrize(("factor", "expected"), [(1, ["y", "x"]), (-1, ["x", "y"])]) +def test_quoted_aggregate_order_accepts_parameters( + aggregate_graph, + engine, + alias, + factor, + expected, +): + if engine == "polars": + pytest.importorskip("polars") + + out = aggregate_graph.gfql( + f"MATCH(n) RETURN n.category AS category, count(*) AS `{alias}` " + f"ORDER BY `{alias}` * $factor ASC", + params={"factor": factor}, + engine=engine, + ) + + assert out._nodes["category"].to_list() == expected + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize("alias", ["c", "the count"]) +def test_quoted_aggregate_order_reports_missing_parameter( + aggregate_graph, + engine, + alias, +): + if engine == "polars": + pytest.importorskip("polars") + + with pytest.raises(GFQLValidationError) as raised: + aggregate_graph.gfql( + f"MATCH(n) RETURN n.category AS category, count(*) AS `{alias}` " + f"ORDER BY `{alias}` * $factor ASC", + engine=engine, + ) + + error = raised.value + assert error.code == "missing-required-field" + assert error.context["field"] == "order_by" + assert error.context["value"] == "factor" + + +def test_compound_aggregate_order_key_survives_order_on_cudf(): + cudf = pytest.importorskip("cudf") + nodes = cudf.DataFrame( + {"id": ["a", "b", "c", "d"], "num": [1, 1, 1, 10], "category": ["x", "x", "x", "y"]} + ) + edges = cudf.DataFrame({"src": ["a"], "dst": ["b"]}) + graph = graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + out = graph.gfql( + "MATCH(n) " + "RETURN n.category AS category, count(*) AS `the count` " + "ORDER BY `the count` + 1 ASC", + engine="cudf", + ) + + assert type(out._nodes).__module__.startswith("cudf") + assert out._nodes["category"].to_pandas().tolist() == ["y", "x"] + assert out._nodes["the count"].to_pandas().tolist() == [1, 3] + + +def test_escaped_quoted_alias_survives_order(odd_columns_graph): + out = odd_columns_graph.gfql( + "MATCH (n) RETURN n.`a``b` AS `sort``id` ORDER BY `sort``id` DESC", + engine="pandas", + ) + + assert out._nodes["sort`id"].to_list() == [2, 1] + + +def test_unknown_quoted_order_alias_is_rejected(odd_columns_graph): + with pytest.raises(GFQLValidationError) as raised: + odd_columns_graph.gfql( + "MATCH (n) RETURN n.`a-b` AS x ORDER BY `missing id` DESC", + engine="pandas", + ) + + error = raised.value + assert error.code == "invalid-name-type" + assert error.context["field"] == "identifier" + assert error.context["value"] == "missing id" + + def test_unquoted_identifiers_are_not_quoted(): parsed = parse_expr("n.analysis") assert parsed.value.name == "n" @@ -97,6 +667,73 @@ def test_unquoted_identifiers_are_not_quoted(): assert parsed.property == "analysis" +@pytest.fixture +def boundary_graph(): + nodes = pd.DataFrame({"id": ["a", "b", "c", "d"], "score": [2, None, 2, 1], "a": [1, 5, 2, 4], "b": [0, 0, 0, 0], "category": ["x", "x", "x", "y"]}) + edges = pd.DataFrame({"src": ["a"], "dst": ["b"]}) + return graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + +@pytest.fixture +def aggregate_graph(): + nodes = pd.DataFrame({"id": ["a", "b", "c", "d"], "num": [1, 3, 2, 5], "category": ["x", "x", "x", "y"]}) + edges = pd.DataFrame({"src": ["a"], "dst": ["b"]}) + return graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + +@pytest.fixture +def aggregate_alias_graph(): + nodes = pd.DataFrame( + { + "id": ["a", "b", "c", "d"], + "num": [1, 1, 1, 10], + "category": ["x", "x", "x", "y"], + } + ) + edges = pd.DataFrame({"src": ["a"], "dst": ["b"]}) + return graphistry.nodes(nodes, "id").edges(edges, "src", "dst") + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize( + ("case", "query", "expected"), + [ + ("null_desc", "MATCH(n) RETURN n.id AS id, n.score AS `the id` ORDER BY `the id` DESC, id ASC", ["b", "a", "c", "d"]), + ("null_asc", "MATCH(n) RETURN n.id AS id, n.score AS `the id` ORDER BY `the id` ASC, id ASC", ["d", "a", "c", "b"]), + ("aggregate_asc", "MATCH(n) RETURN n.category AS id, count(*) AS `count value` ORDER BY `count value` ASC", ["y", "x"]), + ("aggregate_desc", "MATCH(n) RETURN n.category AS id, count(*) AS `count value` ORDER BY `count value` DESC", ["x", "y"]), + ("empty", "MATCH(n) WHERE n.score > 100 RETURN n.id AS id, n.score AS `the id` ORDER BY `the id` DESC", []), + ("limit", "MATCH(n) RETURN n.id AS id, n.score AS `the id` ORDER BY `the id` DESC, id ASC LIMIT 2", ["b", "a"]), + ("expression_alias", "MATCH(n) RETURN n.id AS id, n.a AS a, n.b AS b, n.score AS `a-b` ORDER BY `a-b` DESC, id ASC", ["b", "a", "c", "d"]), + ("expression_key", "MATCH(n) RETURN n.id AS id, n.a AS a, n.b AS b, n.score AS `a-b` ORDER BY a-b DESC", ["b", "d", "c", "a"]), + ], +) +def test_quoted_order_boundaries(boundary_graph, engine, case, query, expected): + if engine == "polars": + pytest.importorskip("polars") + + out = boundary_graph.gfql(query, engine=engine) + + assert out._nodes["id"].to_list() == expected + + +@pytest.mark.parametrize("engine", ["pandas", "polars"]) +@pytest.mark.parametrize( + "query", + [ + "MATCH(n) WITH n.id AS id, n.score AS `hidden value` RETURN id ORDER BY `hidden value` DESC", + "MATCH(n) WITH n, n.score + 1 AS `hidden value` RETURN n.id AS id ORDER BY `hidden value` DESC", + ], +) +def test_quoted_hidden_order_keys_survive_final_projection(boundary_graph, engine, query): + if engine == "polars": + pytest.importorskip("polars") + + out = boundary_graph.gfql(query, engine=engine) + + assert out._nodes["id"].to_list() == ["b", "a", "c", "d"] + + @pytest.mark.parametrize( "query", [