jayzhan211 commented on code in PR #12046: URL: https://github.com/apache/datafusion/pull/12046#discussion_r1720928339
########## datafusion/sql/src/select.rs: ########## @@ -749,6 +750,37 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { .map(|expr| rebase_expr(expr, &aggr_projection_exprs, input)) .collect::<Result<Vec<Expr>>>()?; + // If the having expression is present and the group by expression is not present, + // we can ensure this is an invalid query. Expand the wildcard expression here to + // get a better error message. + let select_exprs_post_aggr = if having_expr_opt.is_some() + && group_by_exprs.is_empty() Review Comment: I think checking again with `check_columns_satisfy_exprs` in `ExpandWildcardRule` make more sense to me ########## datafusion/sql/src/select.rs: ########## @@ -749,6 +750,37 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { .map(|expr| rebase_expr(expr, &aggr_projection_exprs, input)) .collect::<Result<Vec<Expr>>>()?; + // If the having expression is present and the group by expression is not present, + // we can ensure this is an invalid query. Expand the wildcard expression here to + // get a better error message. + let select_exprs_post_aggr = if having_expr_opt.is_some() + && group_by_exprs.is_empty() Review Comment: I think checking again with `check_columns_satisfy_exprs` in `ExpandWildcardRule` makes more sense to me -- 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