-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-40856][SQL] Update the error template of WRONG_NUM_PARAMS #38319
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-40856][SQL] Update the error template of WRONG_NUM_PARAMS #38319
Conversation
| messageParameters = Map("actualNum" -> children.length.toString)) | ||
| messageParameters = Map( | ||
| "functionName" -> prettyName, | ||
| "expectedNum" -> "at least two", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's pass > 1 instead of at least two to don't depend on English. When we will translate error-classes.json to a local language, it would be nice to avoid error messages in different languages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
Can one of the admins verify this patch? |
|
@panbingkun Seems like a connection issue: Could you re-trigger GAs, please. |
Done |
|
+1, LGTM. Merging to master. |
### What changes were proposed in this pull request? The pr aim to Update the error template of WRONG_NUM_PARAMS. ### Why are the changes needed? More general. ### Does this PR introduce _any_ user-facing change? Yes the PR changes user-facing error messages. ### How was this patch tested? - Existed UT. - Pass GA. Closes apache#38319 from panbingkun/update_WRONG_NUM_PARAMS_template. Authored-by: panbingkun <[email protected]> Signed-off-by: Max Gekk <[email protected]>
What changes were proposed in this pull request?
The pr aim to Update the error template of WRONG_NUM_PARAMS.
Why are the changes needed?
More general.
Does this PR introduce any user-facing change?
Yes the PR changes user-facing error messages.
How was this patch tested?