alamb commented on code in PR #11301:
URL: https://github.com/apache/datafusion/pull/11301#discussion_r1667352742
##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -369,14 +369,21 @@ impl AggregateExec {
new_requirement.extend(req);
new_requirement = collapse_lex_req(new_requirement);
- let input_order_mode =
- if indices.len() == groupby_exprs.len() && !indices.is_empty() {
- InputOrderMode::Sorted
- } else if !indices.is_empty() {
- InputOrderMode::PartiallySorted(indices)
- } else {
- InputOrderMode::Linear
- };
+ let indices: Vec<usize> = indices
Review Comment:
I recommend adding a comment here (perhaps copied from the description of
the PR) explaining the subtelty involving grouping sets
##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -2267,4 +2278,94 @@ mod tests {
assert_eq!(new_agg.schema(), aggregate_exec.schema());
Ok(())
}
+
+ #[tokio::test]
+ async fn test_agg_exec_group_by_const() -> Result<()> {
+ let schema = Arc::new(Schema::new(vec![
+ Field::new("a", DataType::Float32, true),
+ Field::new("b", DataType::Float32, true),
+ Field::new("const", DataType::Int32, false),
+ ]));
+
+ let col_a = col("a", &schema)?;
+ let col_b = col("b", &schema)?;
+ let const_expr = Arc::new(Literal::new(ScalarValue::Int32(Some(1))));
+
+ let groups = PhysicalGroupBy::new(
+ vec![
+ (col_a, "a".to_string()),
+ (col_b, "b".to_string()),
+ (const_expr, "const".to_string()),
+ ],
+ vec![
+ (
+ Arc::new(Literal::new(ScalarValue::Float32(None))),
+ "a".to_string(),
+ ),
+ (
+ Arc::new(Literal::new(ScalarValue::Float32(None))),
+ "b".to_string(),
+ ),
+ (
+ Arc::new(Literal::new(ScalarValue::Int32(None))),
+ "const".to_string(),
+ ),
+ ],
+ vec![
+ vec![false, true, true],
+ vec![true, false, true],
+ vec![true, true, false],
+ ],
+ );
+
+ let aggregates: Vec<Arc<dyn AggregateExpr>> =
vec![create_aggregate_expr(
+ count_udaf().as_ref(),
+ &[lit(1)],
+ &[Expr::Literal(ScalarValue::Int32(Some(1)))],
Review Comment:
I think you could make this more concise like this (and ensure the type is
correct)
```suggestion
&[lit(1i32)],
```
--
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]
