[SPARK-40810][SQL] Use SparkIllegalArgumentException instead of IllegalArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand #38274
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…alArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand
…alArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand
…alArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand
…lArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand
Contributor
Author
|
cc @MaxGekk |
MaxGekk
reviewed
Oct 17, 2022
Comment on lines
72
to
75
| val e = intercept[SparkIllegalArgumentException] { | ||
| sql(s"CREATE NAMESPACE $ns LOCATION ''") | ||
| } | ||
| assert(e.getMessage.contains("Can not create a Path from an empty string")) | ||
| assert(e.getMessage.contains("Unsupported empty location")) |
Member
There was a problem hiding this comment.
As you are here, could you use checkError().
|
Can one of the admins verify this patch? |
…alArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand
…alArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand
…alArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand
…alArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand
…alArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand
MaxGekk
reviewed
Oct 18, 2022
| "Not enough memory to build and broadcast the table to all worker nodes. As a workaround, you can either disable broadcast by setting <autoBroadcastjoinThreshold> to -1 or increase the spark driver memory by setting <driverMemory> to a higher value<analyzeTblMsg>" | ||
| ] | ||
| }, | ||
| "_LEGACY_ERROR_TEMP_2251" : { |
Member
There was a problem hiding this comment.
Could you assign a name to the error class like: UNSUPPORTED_EMPTY_LOCATION
…alArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand
MaxGekk
approved these changes
Oct 19, 2022
Member
|
+1, LGTM. Merging to master. |
cloud-fan
reviewed
Oct 26, 2022
| }, | ||
| "UNSUPPORTED_EMPTY_LOCATION" : { | ||
| "message" : [ | ||
| "Unsupported empty location." |
Contributor
There was a problem hiding this comment.
empty location is a bit misleading. This is not a location with no files/directories, it's empty string. How about
EMPTY_STRING_AS_LOCATION: Can not reference a location with an empty string.
Member
There was a problem hiding this comment.
@panbingkun Could you open a follow up PR and change this, and address below comment, please.
cloud-fan
reviewed
Oct 26, 2022
| sql(s"ALTER NAMESPACE $ns SET LOCATION ''") | ||
| }.getMessage | ||
| assert(message.contains("Can not create a Path from an empty string")) | ||
| val sqlText = s"ALTER NAMESPACE $ns SET LOCATION ''" |
Contributor
There was a problem hiding this comment.
I think we can move this test to the base suite now, as the behavior is consistent between v1 and v2 tables.
cloud-fan
reviewed
Oct 26, 2022
| } | ||
| assert(e.getMessage.contains("Can not create a Path from an empty string")) | ||
|
|
||
| val sqlText = s"CREATE NAMESPACE $ns LOCATION ''" |
SandishKumarHN
pushed a commit
to SandishKumarHN/spark
that referenced
this pull request
Dec 12, 2022
…alArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand ### What changes were proposed in this pull request? This pr aims to use SparkIllegalArgumentException instead of IllegalArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand. ### Why are the changes needed? When I work on https://issues.apache.org/jira/browse/SPARK-40790, I found when `location` is empty, DDL command(CreateDatabaseCommand & AlterDatabaseSetLocationCommand) throw IllegalArgumentException, it seem not to fit into the new error framework. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existed UT. Closes apache#38274 from panbingkun/setNamespaceLocation_error. Authored-by: panbingkun <[email protected]> Signed-off-by: Max Gekk <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This pr aims to use SparkIllegalArgumentException instead of IllegalArgumentException in CreateDatabaseCommand & AlterDatabaseSetLocationCommand.
Why are the changes needed?
When I work on https://issues.apache.org/jira/browse/SPARK-40790,
I found when
locationis empty, DDL command(CreateDatabaseCommand & AlterDatabaseSetLocationCommand) throw IllegalArgumentException, it seem not to fit into the new error framework.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existed UT.