Skip to content

Conversation

@itholic
Copy link
Contributor

@itholic itholic commented Oct 6, 2022

What changes were proposed in this pull request?

This PR proposes to migrate 25 execution errors onto temporary error classes with the prefix _LEGACY_ERROR_TEMP_2101 to _LEGACY_ERROR_TEMP_2125.

The error classes are prefixed with _LEGACY_ERROR_TEMP_ indicates the dev-facing error messages, and won't be exposed to end users.

Why are the changes needed?

To speed-up the error class migration.

The migration on temporary error classes allow us to analyze the errors, so we can detect the most popular error classes.

Does this PR introduce any user-facing change?

No

How was this patch tested?

$ build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite"
$ build/sbt "test:testOnly *SQLQuerySuite"

@itholic itholic changed the title [WIP][SPARK-40663][SQL] Migrate execution errors onto error classes: _LEGACY_ERROR_TEMP_2101-2125 [SPARK-40663][SQL] Migrate execution errors onto error classes: _LEGACY_ERROR_TEMP_2101-2125 Oct 8, 2022
@itholic itholic marked this pull request as ready for review October 8, 2022 12:53
@itholic
Copy link
Contributor Author

itholic commented Oct 12, 2022

FYI @MaxGekk @cloud-fan

Test passed!

@MaxGekk
Copy link
Member

MaxGekk commented Oct 12, 2022

@itholic Thank you for the ping. I will review this PR today.

@MaxGekk
Copy link
Member

MaxGekk commented Oct 12, 2022

+1, LGTM. Merging to master.
Thank you, @itholic.

@MaxGekk MaxGekk closed this in c1f54e7 Oct 12, 2022
def cannotParseStatisticAsPercentileError(
stats: String, e: NumberFormatException): Throwable = {
new IllegalArgumentException(s"Unable to parse $stats as a percentile", e)
stats: String, e: NumberFormatException): SparkIllegalArgumentException = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itholic You just ignored e? Seems like some kind of regression. Could you add a constructor to SparkIllegalArgumentException which accepts cause too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out.

Let me address it with #38104 (comment).

@itholic itholic deleted the SPARK-40540-2101-2125 branch April 22, 2023 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants