nchammas commented on code in PR #57887:
URL: https://github.com/apache/spark/pull/57887#discussion_r3750973068
##########
python/pyspark/errors/utils.py:
##########
@@ -98,20 +98,22 @@ def __init__(self) -> None:
def get_sqlstate(self, errorClass: Optional[str]) -> Optional[str]:
"""
Returns the SQL state for the given error class.
+
+ A sub-class with its own sqlState overrides the main class's; a
sub-class
+ without one inherits the main class's.
Review Comment:
If the intention is to have sub-conditions inherit the parent condition
error state, that's one thing. But allowing a sub-condition to _override_ the
parent error state is another thing.
I think in this PR your intention is just to do the former, correct? If so,
I would update this docstring and the behavior below accordingly.
##########
python/pyspark/errors/tests/test_errors.py:
##########
@@ -118,12 +118,20 @@ def test_sqlstate(self):
error = PySparkRuntimeError(errorClass="APPLICATION_NAME_NOT_SET",
messageParameters={})
self.assertIsNone(error.getSqlState())
+ # Neither the sub-class nor the main class declares a sqlState.
error = PySparkRuntimeError(
errorClass="SESSION_MUTATION_IN_DECLARATIVE_PIPELINE.SET_RUNTIME_CONF",
messageParameters={"method": "set"},
)
self.assertIsNone(error.getSqlState())
+ # A sub-class without its own sqlState inherits the main class's.
+ error = PySparkRuntimeError(
+ errorClass="NEAREST_BY_JOIN.UNSUPPORTED_MODE",
Review Comment:
Does this test ensure that the sub-condition does _not_ have an error state?
Because while that may be true now, it may change in the future and silently
invalidate the purpose of this test
##########
python/pyspark/errors/tests/test_errors.py:
##########
@@ -118,12 +118,20 @@ def test_sqlstate(self):
error = PySparkRuntimeError(errorClass="APPLICATION_NAME_NOT_SET",
messageParameters={})
self.assertIsNone(error.getSqlState())
+ # Neither the sub-class nor the main class declares a sqlState.
error = PySparkRuntimeError(
errorClass="SESSION_MUTATION_IN_DECLARATIVE_PIPELINE.SET_RUNTIME_CONF",
messageParameters={"method": "set"},
)
self.assertIsNone(error.getSqlState())
+ # A sub-class without its own sqlState inherits the main class's.
+ error = PySparkRuntimeError(
+ errorClass="NEAREST_BY_JOIN.UNSUPPORTED_MODE",
Review Comment:
Does this test ensure that the sub-condition does _not_ have an error state?
Because while that may be true now, it may change in the future and silently
invalidate the purpose of this test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
