-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Closed
Labels
bugSomething isn't workingSomething isn't working
Description
Describe the bug
Providing empty aggregation functions in a Substrait plan results in an invalid logical plan that fails in the physical transformation step with the following error:
Error: Internal("args should not be empty")
Coming from this line:
return internal_err!("args should not be empty"); |
To Reproduce
Feed this simple SELECT count(*) FROM (SELECT 1))
substrait plan:
substrait plan
```json { "extensionUris": [ { "uri": "https://github.com/substrait-io/substrait/blob/main/extensions/functions_aggregate_generic.yaml" } ], "extensions": [ { "extensionFunction": { "functionAnchor": 185, "name": "count:any" } } ], "relations": [ { "root": { "input": { "aggregate": { "common": { "direct": { } }, "input": { "read": { "common": { "direct": { } }, "baseSchema": { "names": [ "dummy" ], "struct": { "types": [ { "i64": { "nullability": "NULLABILITY_REQUIRED" } } ], "nullability": "NULLABILITY_REQUIRED" } }, "virtualTable": { "values": [ { "fields": [ { "i64": "0", "nullable": false } ] } ] } } }, "groupings": [ { "groupingExpressions": [], "expressionReferences": [] } ], "measures": [ { "measure": { "functionReference": 185, "args": [], "sorts": [], "phase": "AGGREGATION_PHASE_INITIAL_TO_RESULT", "outputType": { "i64": { "nullability": "NULLABILITY_REQUIRED" } }, "invocation": "AGGREGATION_INVOCATION_ALL", "arguments": [], "options": [] } } ], "groupingExpressions": [] } }, "names": [ "count(*)" ] } } ] } ```Or simply add the following line to the Substrait integration tests here and run them.
let ctx = add_plan_schemas_to_ctx(SessionContext::new(), &proto)?;
let plan = from_substrait_plan(&ctx.state(), &proto).await?;
+ ctx.state().create_physical_plan(&plan).await?;
Ok(format!("{}", plan))
}
Expected behavior
The Substrait to DF conversion logic works as before injecting dummy arguments where necessary
Additional context
I suspect that this regression was introduced in this change https://github.com/apache/datafusion/pull/14824/files#diff-474e53672159d74dae38992a914a74eba81b8350ebe161f11d755f06414ed7b4, but not sure if just adding that back will break something else
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working