AlexanderNenninger opened a new issue, #27058: URL: https://github.com/apache/datafusion/issues/27058
### Describe the bug Thanks for DataFusion and its Substrait support! I found this bug while replaying DuckDB's sqllogictest suite through DataFusion's Substrait producer and comparing the results against a [formalization](https://github.com/AlexanderNenninger/substrait-lean) of Substrait plan semantics. The query was `SELECT i % 2 AS k, SUM(i) FROM integers GROUP BY k, k` from `test/sql/aggregate/group/test_group_by.test`. When a grouping key is repeated (`GROUP BY k, k`), the producer writes an `AggregateRel` with two grouping expressions, one per occurrence. Per the spec, that relation outputs one column per grouping expression followed by the measures, so three columns here: `k, k, sum`. The `ProjectRel` above it, though, indexes the aggregate's output as if it had two columns (`k, sum`): its expressions read fields 0 and 1, and its emit `[2, 3]` picks what it takes to be those two expressions. Read per the spec, the plan returns `(sum, k)` under the names `(k, s)`. DataFusion's own consumer reads the plan the same way, so a round trip swaps the columns. Executing the SQL directly is correct; only the Substrait plan is affected. The DataFusion `Aggregate` schema appears to have one column for the repeated key, while the Substrait relation gets one grouping expression per occurrence. ### To Reproduce DataFusion 55.0.0 (Python bindings, `datafusion==55.0.0`, built on DataFusion 55.1.0); the same output on 51.0.0. ```python # uv run --no-project --with datafusion==55.0.0 --with "substrait>=0.37" python DF-4.py import json import datafusion from datafusion import SessionContext from datafusion.substrait import Consumer, Producer from google.protobuf import json_format from substrait.plan_pb2 import Plan print("datafusion", datafusion.__version__) ctx = SessionContext() ctx.sql("CREATE TABLE t (k INT, v INT) AS VALUES (1, 10), (1, 20), (2, 5)").collect() q = "SELECT k, SUM(v) AS s FROM t GROUP BY k, k ORDER BY k" print("direct: ", ctx.sql(q).to_pydict()) p = Producer.to_substrait_plan(ctx.sql(q).logical_plan(), ctx) lp = Consumer.from_substrait_plan(ctx, p) print("round trip:", ctx.create_dataframe_from_logical_plan(lp).to_pydict()) # The relevant part of the Substrait plan msg = Plan(); msg.ParseFromString(p.encode()) plan = json_format.MessageToDict(msg) def walk(o): if isinstance(o, dict): if "project" in o and "aggregate" in o["project"]["input"]: proj = o["project"] agg = proj["input"]["aggregate"] print("aggregate: %d grouping expressions, groupings %s, %d measure(s)" % ( len(agg["groupingExpressions"]), json.dumps(agg["groupings"]), len(agg["measures"]))) print("project above it: expressions read fields %s, emit %s" % ( [e["selection"]["directReference"]["structField"].get("field", 0) for e in proj["expressions"]], proj["common"]["emit"]["outputMapping"])) for v in o.values(): walk(v) elif isinstance(o, list): for v in o: walk(v) walk(plan) ``` Output: ```text datafusion 55.0.0 direct: {'k': [1, 2], 's': [30, 5]} round trip: {'k': [5, 30], 's': [2, 1]} aggregate: 2 grouping expressions, groupings [{"expressionReferences": [0, 1]}], 1 measure(s) project above it: expressions read fields [0, 1], emit [2, 3] ``` The unoptimized logical plan keeps both keys: `Aggregate: groupBy=[[t.k, t.k]], aggr=[[sum(t.v)]]`. ### Expected behavior The round trip returns what the SQL returns, `{'k': [1, 2], 's': [30, 5]}`. Either the producer writes the repeated key once, or it counts every grouping expression it writes when it computes the field indices of the relations above. Repeating a grouping key is legal SQL and doesn't change the result: DuckDB's test expects `GROUP BY k, k` to group by `k` once, and PostgreSQL agrees. ### Additional context - The aggregate's output layout is in the spec: "The list of grouping expressions in declaration order followed by the list of measures in declaration order" ([logical relations, Aggregate operation](https://substrait.io/relations/logical_relations/#aggregate-operation)). - #16140 (fixed by #16189) is the consumer side: it gives each identical grouping expression its own output column, which exposes the shift. - DuckDB's test for `GROUP BY k, k`: https://github.com/duckdb/duckdb/blob/d54cbbe3df17e8e5ce68312f483b4155836112aa/test/sql/aggregate/group/test_group_by.test ### Willingness to contribute I would like to contribute a fix for this bug, but request guidance from the DataFusion community -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
