Improved error for expand wildcard rule

[Jiashu-Hu]

Which issue does this PR close?

• Closes Improved error for expand wildcard rule #15004

Rationale for this change

The current error messages for non-aggregate columns in GROUP BY queries are technically accurate but not user-friendly. They use technical terminology like "non-aggregate values" without clearly explaining the problem or providing actionable solutions. This PR improves these error messages to be more intuitive and helpful, especially for users who might not be familiar with SQL aggregation concepts.

What changes are included in this PR?

added two function in to expand_wildcard_rule.rs:

1. fn validate_columns_in_group_by_or_aggregate(has_group_by: bool, expanded: &Vec<Expr>, group_by_columns: &Vec<Expr>, aggregate_columns: &Vec<Expr>) -> Result<(), DataFusionError>

• Verifies all selected columns appear either in the GROUP BY clause or within aggregate functions
• Returns a clear SchemaError::GroupByColumnNotInSelectList error when validation fails

2. added a new SchemaError type to shows the error message - datafusion_common::SchemaError::GroupByColumnNotInSelectList

In datafusion/sql/src/utils.rs:

• Updated message_prefix() method to provide clearer error message prefixes
• Improved help text in check_column_satisfies_expr() to suggest specific solutions
• for easy understanding as requested in #15004

Are these changes tested?

There are several existing tests that directly evaluate this function during the CICL process, and all of them have been successfully passed.

Are there any user-facing changes?

Yes. Users will now see more intuitive error messages when they encounter GROUP BY validation issues. For example, instead of seeing:

Error during planning: Projection references non-aggregate values: Expression foo.b could not be resolved from available columns: foo.a

They will see:

SchemaError: Column 'foo.b' must appear in the GROUP BY clause or be used in an aggregate function

This change makes errors more understandable and provides clearer guidance on how to fix the issue.

[jayzhan211]

It seems like the error should be handled in the check_columns_satisfy_exprs below with ProjectionMustReferenceAggregate, is it possible to unify these 2?

[Jiashu-Hu]

hey jay, thanks for your review! I've unified these 2 also updated error message in the test file.

[alamb]

Thank you so much @Jiashu-Hu

I also verified that the example in the original reproducer now is much better 😍

DataFusion CLI v46.0.1
> create table foo(a int, b int, c timestamp) as values (1,2,'2025-01-01T12:01:02');

SELECT *
FROM foo
WHERE c >= NOW() - INTERVAL '1 hour'
GROUP BY a;
0 row(s) fetched.
Elapsed 0.037 seconds.

Error during planning: Column in SELECT must be in GROUP BY or an aggregate function: While expanding wildcard, column "foo.b" must appear in the GROUP BY clause or must be part of an aggregate function, currently only "foo.a" appears in the SELECT clause satisfies this requirement

cc @mhilton

[jayzhan211]

Thanks @alamb @Jiashu-Hu

[ozankabak]

This PR seems to have broken main. It is an easy fix (error messages do not match).

@alamb -- Extended tests break very frequently these days. We should prioritize completing the work on running them before merge.

[alamb]

This PR seems to have broken main. It is an easy fix (error messages do not match).

@alamb -- Extended tests break very frequently these days. We should prioritize completing the work on running them before merge.

@ozankabak I think there is a PR waiting on review that would let us more easily run the tests before merge. I am not sure how we would have known to run them in this case though

• Triggering extended tests through PR comment: Run extended tests #15101

I feel like what would be idea is some workflow that actually ran the tests before merge but that didn't need us to babysit the PR -- something like letting a bot merge it or a merge queue