-
Notifications
You must be signed in to change notification settings - Fork 240
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
Fix test failure with Spark 3.3 by looking for less specific error message #5185
Fix test failure with Spark 3.3 by looking for less specific error message #5185
Conversation
Signed-off-by: Andy Grove <[email protected]>
1e3cb7a
to
a6193b1
Compare
build |
@@ -788,7 +788,7 @@ def test_div_overflow_exception_when_ansi(expr, ansi_enabled): | |||
assert_gpu_and_cpu_error( | |||
df_fun=lambda spark: _get_div_overflow_df(spark, expr).collect(), | |||
conf=ansi_conf, | |||
error_message='java.lang.ArithmeticException: Overflow in integral divide') | |||
error_message='ArithmeticException: Overflow in integral divide') |
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.
Isn't the point to ensure we're throwing the same exception? As such I'd rather see this test check for the correct exception class based on the Spark version -- if Spark 3.3 throws a different exception for this, then our plugin should throw the same class for the same type of exception on Spark 3.3.
I'm OK if we want to do this as a followup to refine the test if we aren't already doing the correct behavior now. If we are, then this should choose a different message to check based on which Spark we're using as is done in other tests checking exception types.
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.
Yes, that's a good point. I filed #5196 as a follow-on.
Closes #5182
Spark 3.3 changed which exception they throw for a divide that causes an overflow in apache/spark@c2ed15d, causing our tests to fail.
This PR changes the affected test to just look for
ArithmeticException
rather than specificallyjava.lang.ArithmeticException
. This is the same approach used in the other tests.