Skip to content

Comments

[SPARK-42845][SQL] Update the error class _LEGACY_ERROR_TEMP_2010 to InternalError#40817

Closed
liang3zy22 wants to merge 2 commits intoapache:masterfrom
liang3zy22:spark42845
Closed

[SPARK-42845][SQL] Update the error class _LEGACY_ERROR_TEMP_2010 to InternalError#40817
liang3zy22 wants to merge 2 commits intoapache:masterfrom
liang3zy22:spark42845

Conversation

@liang3zy22
Copy link
Contributor

@liang3zy22 liang3zy22 commented Apr 17, 2023

What changes were proposed in this pull request?

Update the error class _LEGACY_ERROR_TEMP_2010 to InternalError.

Why are the changes needed?

Fix jira issue SPARK-42845. The original name just a number, update it to InteralError.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Add a test case in QueryExecutionErrorsSuite.

@liang3zy22
Copy link
Contributor Author

I can't find a method to trigger this error_class using sql command. This error_class are only thrown by "mergeExpressions" field of AggregateWindowFunction. But this field isn't called anywhere in all aggregate window function. Is it an internal error?

@liang3zy22
Copy link
Contributor Author

@MaxGekk , any comment?

Copy link
Member

Choose a reason for hiding this comment

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

Can you trigger the error from user space (by SQL statement, for instance)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't find a method to trigger this error_class using SQL statement. This error_class are only thrown by "mergeExpressions" field of AggregateWindowFunction. But this field isn't called anywhere in all aggregate window function. Maybe we can change it to internal error?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can change it to internal error?

Yep, let's do that.

@liang3zy22 liang3zy22 changed the title [SPARK-42845][SQL] Update the error class _LEGACY_ERROR_TEMP_2010 to MERGE_UNSUPPORTED_BY_WINDOW_FUNCTION [SPARK-42845][SQL] Update the error class _LEGACY_ERROR_TEMP_2010 to InternalError Apr 25, 2023
@liang3zy22 liang3zy22 force-pushed the spark42845 branch 2 times, most recently from 29f1638 to faed04c Compare April 26, 2023 02:46
Copy link
Member

Choose a reason for hiding this comment

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

Could you use " instead of """
and please, add the function name. How about:

Suggested change
s"""Aggregate Window Functions do not support merging.""")
s"The aggregate window function ${toSQLId(funcName)} does not support merging.")

Pass the function name:

  override lazy val mergeExpressions =
    throw QueryExecutionErrors.mergeUnsupportedByWindowFunctionError(prettyName)

liang3zy22 added 2 commits May 2, 2023 07:31
Signed-off-by: Liang Yan <ckgppl_yan@sina.cn>
Signed-off-by: Liang Yan <ckgppl_yan@sina.cn>
@MaxGekk
Copy link
Member

MaxGekk commented May 3, 2023

+1, LGTM. Merging to master.
Thank you, @liang3zy22.

@MaxGekk MaxGekk closed this in 0010847 May 3, 2023
@liang3zy22 liang3zy22 deleted the spark42845 branch May 3, 2023 10:27
LuciferYang pushed a commit to LuciferYang/spark that referenced this pull request May 10, 2023
…InternalError

### What changes were proposed in this pull request?

Update the error class _LEGACY_ERROR_TEMP_2010 to InternalError.

### Why are the changes needed?

Fix jira issue [SPARK-42845](https://issues.apache.org/jira/browse/SPARK-42845). The original name just a number, update it to InteralError.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Add a test case in QueryExecutionErrorsSuite.

Closes apache#40817 from liang3zy22/spark42845.

Authored-by: Liang Yan <ckgppl_yan@sina.cn>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants