-
Notifications
You must be signed in to change notification settings - Fork 28k
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-47158][SQL] Assign proper name and sqlState
to _LEGACY_ERROR_TEMP_(2134|2231)
#45244
Conversation
sqlState
to _LEGACY_ERROR_TEMP_(2134|2231)
sqlState
to _LEGACY_ERROR_TEMP_(2134|2231)
@MaxGekk Could you review this when you find some time? They are currently the most frequent top 2 legacy error classes. |
@itholic Thanks for the ping. I will look at it soon. |
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/TimestampFormatterSuite.scala
Show resolved
Hide resolved
Thanks, @MaxGekk for the review! Just adjusted comments |
sql/api/src/main/scala/org/apache/spark/sql/errors/ExecutionErrors.scala
Outdated
Show resolved
Hide resolved
sql/api/src/main/scala/org/apache/spark/sql/errors/DataTypeErrors.scala
Outdated
Show resolved
Hide resolved
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.
Waiting for CI. @itholic Could you update PR's description according to your recent changes.
Updated PR description and fixed the test. |
Oh, actually they are related. Let me fix it. |
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.
@itholic Could you fix the failed test:
[info] RowSuite:
...
[info] - SPARK-37654: row contains a null at the requested index should return null (1 millisecond)
[info] - access fieldIndex on Row without schema *** FAILED *** (4 milliseconds)
[info] Map("methodName" -> "fieldIndex", "className" -> "Row", "fieldName" -> "`foo`") did not equal Map("methodName" -> "fieldIndex", "className" -> "Row", "fieldName" -> "foo") (SparkFunSuite.scala:362)
+1, LGTM. Merging to master. |
Thanks for the review, @MaxGekk ! |
…R_TEMP_(2134|2231)` ### What changes were proposed in this pull request? This PR proposes to assign proper name and `sqlState` to `_LEGACY_ERROR_TEMP_(2134|2231)` - `_LEGACY_ERROR_TEMP_2134` -> `CANNOT_PARSE_STRING_AS_DATATYPE` - `_LEGACY_ERROR_TEMP_2231` -> `UNSUPPORTED_CALL.FIELD_INDEX` ### Why are the changes needed? To improve error usability ### Does this PR introduce _any_ user-facing change? No API changes, but the user-facing error message will be improved ### How was this patch tested? Added UTs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#45244 from itholic/TOP_LEGACY_ERRORS. Authored-by: Haejoon Lee <haejoon.lee@databricks.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>
…R_TEMP_(2134|2231)` ### What changes were proposed in this pull request? This PR proposes to assign proper name and `sqlState` to `_LEGACY_ERROR_TEMP_(2134|2231)` - `_LEGACY_ERROR_TEMP_2134` -> `CANNOT_PARSE_STRING_AS_DATATYPE` - `_LEGACY_ERROR_TEMP_2231` -> `UNSUPPORTED_CALL.FIELD_INDEX` ### Why are the changes needed? To improve error usability ### Does this PR introduce _any_ user-facing change? No API changes, but the user-facing error message will be improved ### How was this patch tested? Added UTs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#45244 from itholic/TOP_LEGACY_ERRORS. Authored-by: Haejoon Lee <haejoon.lee@databricks.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>
…R_TEMP_(2134|2231)` ### What changes were proposed in this pull request? This PR proposes to assign proper name and `sqlState` to `_LEGACY_ERROR_TEMP_(2134|2231)` - `_LEGACY_ERROR_TEMP_2134` -> `CANNOT_PARSE_STRING_AS_DATATYPE` - `_LEGACY_ERROR_TEMP_2231` -> `UNSUPPORTED_CALL.FIELD_INDEX` ### Why are the changes needed? To improve error usability ### Does this PR introduce _any_ user-facing change? No API changes, but the user-facing error message will be improved ### How was this patch tested? Added UTs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#45244 from itholic/TOP_LEGACY_ERRORS. Authored-by: Haejoon Lee <haejoon.lee@databricks.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>
What changes were proposed in this pull request?
This PR proposes to assign proper name and
sqlState
to_LEGACY_ERROR_TEMP_(2134|2231)
_LEGACY_ERROR_TEMP_2134
->CANNOT_PARSE_STRING_AS_DATATYPE
_LEGACY_ERROR_TEMP_2231
->UNSUPPORTED_CALL.FIELD_INDEX
Why are the changes needed?
To improve error usability
Does this PR introduce any user-facing change?
No API changes, but the user-facing error message will be improved
How was this patch tested?
Added UTs.
Was this patch authored or co-authored using generative AI tooling?
No.