alamb commented on code in PR #17313: URL: https://github.com/apache/datafusion/pull/17313#discussion_r2325782336
########## datafusion/sql/src/select.rs: ########## @@ -944,15 +963,41 @@ impl<S: ContextProvider> SqlToRel<'_, S> { check_columns_satisfy_exprs( &column_exprs_post_aggr, std::slice::from_ref(&having_expr_post_aggr), - CheckColumnsSatisfyExprsPurpose::HavingMustReferenceAggregate, + CheckColumnsSatisfyExprsPurpose::Aggregate( + CheckColumnsMustReferenceAggregatePurpose::Having, + ), )?; Some(having_expr_post_aggr) } else { None }; - Ok((plan, select_exprs_post_aggr, having_expr_post_aggr)) + // Rewrite the QUALIFY expression to use the columns produced by the + // aggregation. + let qualify_expr_post_aggr = if let Some(qualify_expr) = qualify_expr_opt { Review Comment: Perhaps we can try it in a follow on PR ########## datafusion/sql/tests/cases/diagnostic.rs: ########## @@ -184,7 +184,7 @@ fn test_missing_non_aggregate_in_group_by() -> Result<()> { let diag = do_query(query); assert_snapshot!(diag.message, @"'person.first_name' must appear in GROUP BY clause because it's not an aggregate expression"); assert_eq!(diag.span, Some(spans["a"])); - assert_snapshot!(diag.helps[0].message, @"Either add 'person.first_name' to GROUP BY clause, or use an aggregare function like ANY_VALUE(person.first_name)"); Review Comment: ❤️ -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org