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: [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]
