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-41435][SQL] Change to call invalidFunctionArgumentsError for curdate() when expressions is not empty #38960

Closed
wants to merge 4 commits into from

Conversation

LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Dec 7, 2022

What changes were proposed in this pull request?

This pr change to call invalidFunctionArgumentsError instead of invalidFunctionArgumentNumberError for curdate() when expressions is not empty, then curdate() will throw AnalysisException with error class WRONG_NUM_ARGS when input args it not empty.

Why are the changes needed?

WRONG_NUM_ARGS is a more appropriate error class

Does this PR introduce any user-facing change?

No

How was this patch tested?

Add new test case

@LuciferYang LuciferYang changed the title [SPARK-41435][SQL] Make curdate() throw WRONG_NUM_ARGS when args is not null [SPARK-41435][SQL] Make curdate() throw WRONG_NUM_ARGS when args is not empty Dec 7, 2022
@github-actions github-actions bot added the SQL label Dec 7, 2022
Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

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

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Dec 8, 2022

There are two questions:

  1. Should an internal exception be thrown when validParametersCount.length == 0? This may be triggered by the user action, such as SELECT CAST(1).

  2. select curdate(1) looks like also a user usage error, so I think user should get a user-facing error.

If necessary, I think remove the error class and returning an internal error should be completed in #38940.

@@ -172,7 +172,7 @@ object CurDateExpressionBuilder extends ExpressionBuilder {
CurrentDate()
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
Seq.empty, funcName, expressions.length)
Seq(0), funcName, expressions.length)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To clarify the intention of this pr, I will change this as

throw QueryCompilationErrors.invalidFunctionArgumentsError(funcName, "0", expressions.length)

and update pr description later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@LuciferYang LuciferYang changed the title [SPARK-41435][SQL] Make curdate() throw WRONG_NUM_ARGS when args is not empty [SPARK-41435][SQL] Change to call invalidFunctionArgumentsError for curdate() when expressions is not empty Dec 8, 2022
@MaxGekk
Copy link
Member

MaxGekk commented Dec 8, 2022

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

@MaxGekk MaxGekk closed this in d5e3275 Dec 8, 2022
@LuciferYang
Copy link
Contributor Author

Thanks @MaxGekk

beliefer pushed a commit to beliefer/spark that referenced this pull request Dec 18, 2022
… `curdate()` when `expressions` is not empty

### What changes were proposed in this pull request?
This pr change to call `invalidFunctionArgumentsError` instead of `invalidFunctionArgumentNumberError ` for `curdate()`  when `expressions` is not empty, then `curdate()` will throw `AnalysisException` with error class `WRONG_NUM_ARGS` when input args it not empty.

### Why are the changes needed?
`WRONG_NUM_ARGS` is a more appropriate error class

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

### How was this patch tested?
Add new test case

Closes apache#38960 from LuciferYang/curdate-err-msg.

Authored-by: yangjie01 <yangjie01@baidu.com>
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
Labels
Projects
None yet
2 participants