-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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-51386][CORE][SQL] Assign name to error conditions _LEGACY_ERROR_TEMP_3300-3302 #50149
Conversation
@@ -1737,6 +1743,12 @@ | |||
], | |||
"sqlState" : "42822" | |||
}, | |||
"GROW_POINTER_ARRAY_OUT_OF_MEMORY" : { |
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.
GROW_POINTER_ARRAY_OUT_OF_MEMORY
-> POINTER_ARRAY_OUT_OF_MEMORY
?
It's because the existing *_OUT_OF_MEMORY has the following naming format.
"HDFS_STORE_PROVIDER_OUT_OF_MEMORY" : { |
"ROCKSDB_STORE_PROVIDER_OUT_OF_MEMORY" : { |
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.
thanks for suggestion, sounds reasonable, updated
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.
+1, LGTM (with one minor comment about naming)
cc @MaxGekk
"message" : [ | ||
"No enough memory for aggregation" | ||
], | ||
"sqlState" : "53200" |
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.
Could you assign sqlState
code from another error class:
"82": "out of memory (could not allocate)", |
For instance, "82001" and so on.
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.
thanks for pointing it out, updated
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.
BTW, I see UNABLE_TO_ACQUIRE_MEMORY uses 53200, should we keep it or update 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.
The error class "53": "Invalid Operand or Inconsistent Specification",
is unrelated to memory issues. I believe that need to change its sqlState
too.
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.
Okay, will do that independently
Merged to master. Thank you, @pan3793 and all. |
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.
late LGTM.
…R_TEMP_3300-3302 ### What changes were proposed in this pull request? Assign name to error conditions `_LEGACY_ERROR_TEMP_3300-3302` with sqlState `82001-82003`, all of them are applicable for `SparkOutOfMemoryError` ### Why are the changes needed? Improve the error framework. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GHA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#50149 from pan3793/SPARK-51386. Authored-by: Cheng Pan <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
…R_TEMP_3300-3302 ### What changes were proposed in this pull request? Assign name to error conditions `_LEGACY_ERROR_TEMP_3300-3302` with sqlState `82001-82003`, all of them are applicable for `SparkOutOfMemoryError` ### Why are the changes needed? Improve the error framework. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GHA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#50149 from pan3793/SPARK-51386. Authored-by: Cheng Pan <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
What changes were proposed in this pull request?
Assign name to error conditions
_LEGACY_ERROR_TEMP_3300-3302
with sqlState82001-82003
, all of them are applicable forSparkOutOfMemoryError
Why are the changes needed?
Improve the error framework.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass GHA.
Was this patch authored or co-authored using generative AI tooling?
No.