search

NVIDIA / spark-rapids

Spark RAPIDS plugin - accelerate Apache Spark with GPUs
https://nvidia.github.io/spark-rapids
Apache License 2.0
822 stars 235 forks source link

Fix `string_test` for [databricks] 14.3 #11669

Closed mythrocks closed 2 weeks ago

mythrocks commented 3 weeks ago

Fixes #11535.

This commit fixes the failure of test_startswith and test_endswith in string_test.py on Databricks 14.3.

This is, in effect, a follow-on to the changes introduced in #11247, where test_endswith was skipped for Databricks 4.0, on account of https://issues.apache.org/jira/browse/SPARK-48995. It appears that that bug afflicts Databricks 14.3 as well.

This commit handles both test_startswith and test_endswith for Databricks 14.3 and Apache Spark 4.0.

mythrocks commented 3 weeks ago

Build

mythrocks commented 3 weeks ago

Build

mythrocks commented 2 weeks ago

Is there a tracking issue to have us go back and enable the tests again once this issue is fixed? Do we want to try and fix https://issues.apache.org/jira/browse/SPARK-48995?

That's a good point. I can raise an issue for re-enabling the test. As for fixing SPARK-48995: I've been trying to prioritize a clean run of spark-rapids CI against 350db. Once that's in sight, I can try hitting this Spark bug.

https://issues.apache.org/jira/browse/SPARK-48989 indicates that it is fixed now. Should we update the test to not be skipped anymore?

It doesn't seem to have been pulled into Databricks 14.3. I'll raise a separate issue to unskip the test, once the fix is in.

mythrocks commented 2 weeks ago

Build

mythrocks commented 2 weeks ago

Build

mythrocks commented 2 weeks ago

@revans2, I have raised #11696 to un-skip the starts/ends-with tests, for whenever the NPE is fixed in Apache Spark.

As for https://issues.apache.org/jira/browse/SPARK-48989, I have verified that this skip is safe to remove now. I've un-skipped that test as well.

@revans2, @razajafri: I've had to dismiss the reviews to make these changes. With your approval, I'll check this in after the tests pass.

mythrocks commented 2 weeks ago

Thank you for the reviews, chaps. This change has been merged.