[SPARK-40760][SQL] Migrate type check failures of interval expressions onto error classes #38237
Conversation
MaxGekk
changed the title
|
@cloud-fan @itholic Please, take a look at it. |
| }, | ||
| "WRONG_NUM_ENDPOINTS" : { | ||
| "message" : [ | ||
| "The number of endpoints must be >= 2 to construct intervals." |
There was a problem hiding this comment.
Nit: maybe also include what is current num but this is minor.
|
@cloud-fan Could you approve this PR, please. |
| "The <functionName> accepts only arrays of pair structs, but <childExpr> is of <childType>." | ||
| ] | ||
| }, | ||
| "NON_FOLDABLE_ENDPOINT" : { |
There was a problem hiding this comment.
Can NON_FOLDABLE_INPUT be reused?
| }, | ||
| "UNEXPECTED_ENDPOINT_TYPE" : { | ||
| "message" : [ | ||
| "Endpoints require (numeric or \"TIMESTAMP\" or \"DATE\" or \"TIMESTAMP_NTZ\" or \"INTERVAL YEAR TO MONTH\" or \"INTERVAL DAY TO SECOND\") type." |
There was a problem hiding this comment.
Can UNEXPECTED_INPUT_TYPE be reused?
Or Should we change numeric or \"TIMESTAMP\" or \"DATE\" or \"TIMESTAMP_NTZ\" or \"INTERVAL YEAR TO MONTH\" or \"INTERVAL DAY TO SECOND\" to a placeholder?
| "The <exprName> must be between <valueRange> (current value = <currentValue>)" | ||
| ] | ||
| }, | ||
| "WRONG_NUM_ENDPOINTS" : { |
There was a problem hiding this comment.
Can WRONG_NUM_PARAMS be reused?
There was a problem hiding this comment.
I would leave this as is because this is not about wrong number of parameters but about endpoints in a parameter.
LuciferYang
left a comment
LuciferYang
left a comment
There was a problem hiding this comment.
+1, LGTM if test pass
|
@cloud-fan Could you approve the PR, please. |
|
Merging to master. Thank you, @itholic @LuciferYang @amaliujia for review. |
…s onto error classes ### What changes were proposed in this pull request? In the PR, I propose to add new error sub-classes of the error class `DATATYPE_MISMATCH`, and use it in the case of type check failures of some interval expressions. ### Why are the changes needed? Migration onto error classes unifies Spark SQL error messages, and improves search-ability of errors. ### Does this PR introduce _any_ user-facing change? Yes. The PR changes user-facing error messages. ### How was this patch tested? By running the affected test suites: ``` $ build/sbt "test:testOnly *AnalysisSuite" $ build/sbt "test:testOnly *ExpressionTypeCheckingSuite" $ build/sbt "test:testOnly *ApproxCountDistinctForIntervalsSuite" ``` Closes apache#38237 from MaxGekk/type-check-fails-interval-exprs. Authored-by: Max Gekk <max.gekk@gmail.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>
4 participants
What changes were proposed in this pull request?
In the PR, I propose to add new error sub-classes of the error class
DATATYPE_MISMATCH, and use it in the case of type check failures of some interval expressions.Why are the changes needed?
Migration onto error classes unifies Spark SQL error messages, and improves search-ability of errors.
Does this PR introduce any user-facing change?
Yes. The PR changes user-facing error messages.
How was this patch tested?
By running the affected test suites: