Skip to content

[SPARK-36293][SQL] Refactor fourth set of 20 query execution errors to use error classes#34190

Closed
dchvn wants to merge 2 commits intoapache:masterfrom
dchvn:SPARK-36293
Closed

[SPARK-36293][SQL] Refactor fourth set of 20 query execution errors to use error classes#34190
dchvn wants to merge 2 commits intoapache:masterfrom
dchvn:SPARK-36293

Conversation

@dchvn
Copy link
Contributor

@dchvn dchvn commented Oct 6, 2021

What changes were proposed in this pull request?

Refactor some exceptions in QueryExecutionErrors to use error classes.
There are currently ~350 exceptions in this file; so this PR only focuses on the fourth set of 20.

unableToCreateDatabaseAsFailedToCreateDirectoryError
unableToDropDatabaseAsFailedToDeleteDirectoryError
unableToCreateTableAsFailedToCreateDirectoryError
unableToDeletePartitionPathError
unableToDropTableAsFailedToDeleteDirectoryError
unableToRenameTableAsFailedToRenameDirectoryError
unableToCreatePartitionPathError
unableToRenamePartitionPathError
methodNotImplementedError
tableStatsNotSpecifiedError
unaryMinusCauseOverflowError
binaryArithmeticCauseOverflowError
failedSplitSubExpressionMsg
failedSplitSubExpressionError
failedToCompileMsg
internalCompilerError
compilerError
unsupportedTableChangeError
notADatasourceRDDPartitionError
dataPathNotSpecifiedError

Why are the changes needed?

https://issues.apache.org/jira/browse/SPARK-36094

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass all current tests

@SparkQA
Copy link

SparkQA commented Oct 6, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48378/

@SparkQA
Copy link

SparkQA commented Oct 6, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48378/

@SparkQA
Copy link

SparkQA commented Oct 6, 2021

Test build #143865 has finished for PR 34190 at commit 98ba4b9.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 7, 2021

Test build #143988 has started for PR 34190 at commit 9be6ab3.

@SparkQA
Copy link

SparkQA commented Oct 7, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48456/

@SparkQA
Copy link

SparkQA commented Oct 7, 2021

Test build #143965 has finished for PR 34190 at commit 9be6ab3.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 7, 2021

Test build #143973 has finished for PR 34190 at commit 98ba4b9.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 7, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48456/

@dchvn
Copy link
Contributor Author

dchvn commented Oct 7, 2021

CC @karenfeng, please help me review this one, many thanks!

Copy link
Contributor

@karenfeng karenfeng left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @dchvn! Left some comments; please ping me when you address them.

"CANNOT_DROP_OBJECT_DUE_TO_FAILED_DIRECTORY_DELETION" : {
"message" : [ "Unable to DROP %s %s as failed to delete its directory %s." ]
},
"CANNOT_EVALUATE_EXPRESSION" : {
Copy link
Contributor

Choose a reason for hiding this comment

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

These aren't from this PR, right?

"CANNOT_EVALUATE_EXPRESSION" : {
"message" : [ "Cannot evaluate expression: %s" ]
},
"CANNOT_GENERATE_CODE_FOR_EXPRESSION" : {
Copy link
Contributor

Choose a reason for hiding this comment

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

These aren't from this PR, right?


def failedToCompileMsg(e: Exception): String = {
s"failed to compile: $e"
SparkThrowableHelper.getMessage(
Copy link
Contributor

Choose a reason for hiding this comment

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

@cloud-fan, are all of these compilation failures internal errors?

def notADatasourceRDDPartitionError(split: Partition): Throwable = {
new SparkException(s"[BUG] Not a DataSourceRDDPartition: $split")
new SparkException(
errorClass = "NOT_A_DATA_SOURCE_RDD_PARTITION",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an internal error; let's use INTERNAL_ERROR

"CONCURRENT_QUERY" : {
"message" : [ "Another instance of this query was just started by a concurrent session." ]
},
"DATA_PATH_NOT_SPECIFIED" : {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's call this MISSING_DATA_PATH?


def methodNotImplementedError(methodName: String): Throwable = {
new UnsupportedOperationException(s"$methodName is not implemented")
new SparkUnsupportedOperationException(
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this is actually only used by a dummy implementation and should not be exposed to users. Can you make this an internal error?

def tableStatsNotSpecifiedError(): Throwable = {
new IllegalStateException("table stats must be specified.")
new SparkIllegalStateException(
errorClass = "TABLE_STATISTICS_NOT_SPECIFIED",
Copy link
Contributor

Choose a reason for hiding this comment

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

@cloud-fan, is this an internal error? I can't know a use case where the user encounters this.

"message" : [ "%s cannot be represented as Decimal(%s, %s)." ],
"sqlState" : "22005"
},
"CANNOT_CREATE_OBJECT_DUE_TO_FAILED_DIRECTORY_CREATION" : {
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, what if we just have three error classes: CANNOT_CREATE_DIRECTORY, CANNOT_DELETE_DIRECTORY and CANNOT_RENAME_DIRECTORY? This would be more concise and can be used more generally as well. Then we can make the error message like Cannot %s as failed to create directory %s.

@github-actions
Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Jan 22, 2022
@github-actions github-actions bot closed this Jan 23, 2022
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.

3 participants