Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-48556][SQL] Fix incorrect error message pointing to UNSUPPORTED_GROUPING_EXPRESSION #46900

Closed
wants to merge 3 commits into from

Conversation

nikolamand-db
Copy link
Contributor

What changes were proposed in this pull request?

Following sequence of queries produces UNSUPPORTED_GROUPING_EXPRESSION error:

create table t1(a int, b int) using parquet;
select grouping(a), dummy from t1 group by a with rollup; 

However, the appropriate error should point the user to the invalid dummy column name.

Fix the problem by deprioritizing Grouping and GroupingID nodes in plan which were not resolved and thus cause the unwanted error.

Why are the changes needed?

To fix the described issue.

Does this PR introduce any user-facing change?

Yes, it displays proper error message to user instead of misleading one.

How was this patch tested?

Added test to QueryCompilationErrorsSuite.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the SQL label Jun 6, 2024
@nikolamand-db nikolamand-db changed the title [SPARK-48556] Fix incorrect error message pointing to UNSUPPORTED_GROUPING_EXPRESSION [SPARK-48556][SQL] Fix incorrect error message pointing to UNSUPPORTED_GROUPING_EXPRESSION Jun 6, 2024
@@ -267,6 +267,7 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog with QueryErrorsB
// Early checks for column definitions, to produce better error messages
ColumnDefinition.checkColumnDefinitions(operator)

var checkLowPriorityErrors = () => ()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about

var stagedError: Option[() => Unit] = None
...
if (stagedError.isEmpty) stagedError = ...
...
if (stagedError.isDefined) stagedError.get.apply()

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 224ba16 Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants