Skip to content

[SPARK-41135][SQL] Rename UNSUPPORTED_EMPTY_LOCATION to INVALID_EMPTY_LOCATION #38650

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.

Sign up for GitHub

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

Closed
itholic wants to merge 13 commits into from
Closed

[SPARK-41135][SQL] Rename UNSUPPORTED_EMPTY_LOCATION to INVALID_EMPTY_LOCATION #38650

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ee1829a
[SPARK-41135][SQL] Rename UNSUPPORTED_EMPTY_LOCATION to INVALID_EMPTY…
itholic Nov 14, 2022
4fdad7f
Merge branch 'master' of https://github.com/apache/spark into SPARK-4…
itholic Nov 14, 2022
d32a32f
resolved the comments
itholic Nov 14, 2022
deb0748
fix
itholic Nov 14, 2022
16fdcdb
Merge branch 'master' of https://github.com/apache/spark into SPARK-4…
itholic Nov 15, 2022
526cf80
fix typing
itholic Nov 15, 2022
43ed19b
Fix typing
itholic Nov 15, 2022
9a2b8e3
fix type
itholic Nov 15, 2022
d73c590
fix test
itholic Nov 16, 2022
104f80e
Merge branch 'master' of https://github.com/apache/spark into SPARK-4…
itholic Nov 16, 2022
d4d5b28
fix location
itholic Nov 17, 2022
71e5c42
fix test
itholic Nov 18, 2022
d1056ce
exclude `null` from the message
itholic Nov 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions core/src/main/resources/error/error-classes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,11 @@
],
"sqlState" : "42000"
},
"INVALID_EMPTY_LOCATION" : {
"message" : [
"The location name cannot be empty string or null, but `<location>` was given."
Copy link
Member

@MaxGekk MaxGekk Nov 18, 2022

Choose a reason for hiding this comment

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

Could you add a test when the location is NULL. I just worry that parameters substitution might not handle this. Please, check this.

Copy link
Contributor Author

@itholic itholic Nov 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Good catch.
I tried to add a test with NULL, but it complains [PARSE_SYNTAX_ERROR] rather than [INVALID_EMPTY_LOCATION].
Let me exclude the "null" from the error message.

]
},
"INVALID_FIELD_NAME" : {
"message" : [
"Field name <fieldName> is invalid: <path> is not a struct."
Expand Down Expand Up @@ -1105,11 +1110,6 @@
}
}
},
"UNSUPPORTED_EMPTY_LOCATION" : {
"message" : [
"Unsupported empty location."
]
},
"UNSUPPORTED_FEATURE" : {
"message" : [
"The feature is not supported:"
Expand Down
6 changes: 3 additions & 3 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2799,10 +2799,10 @@ private[sql] object QueryExecutionErrors extends QueryErrorsBase {
"size" -> elementSize.toString))
}

def unsupportedEmptyLocationError(): SparkIllegalArgumentException = {
def invalidEmptyLocationError(location: String): SparkIllegalArgumentException = {
new SparkIllegalArgumentException(
errorClass = "UNSUPPORTED_EMPTY_LOCATION",
messageParameters = Map.empty)
errorClass = "INVALID_EMPTY_LOCATION",
messageParameters = Map("location" -> location))
}

def malformedProtobufMessageDetectedInMessageParsingError(e: Throwable): Throwable = {
Expand Down
4 changes: 2 additions & 2 deletions sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class ResolveSessionCatalog(val catalogManager: CatalogManager)

case SetNamespaceLocation(DatabaseInSessionCatalog(db), location) if conf.useV1Command =>
if (StringUtils.isEmpty(location)) {
throw QueryExecutionErrors.unsupportedEmptyLocationError()
throw QueryExecutionErrors.invalidEmptyLocationError(location)
}
AlterDatabaseSetLocationCommand(db, location)

Expand Down Expand Up @@ -243,7 +243,7 @@ class ResolveSessionCatalog(val catalogManager: CatalogManager)
val location = c.properties.get(SupportsNamespaces.PROP_LOCATION)
val newProperties = c.properties -- CatalogV2Util.NAMESPACE_RESERVED_PROPERTIES
if (location.isDefined && location.get.isEmpty) {
throw QueryExecutionErrors.unsupportedEmptyLocationError()
throw QueryExecutionErrors.invalidEmptyLocationError(location)
}
CreateDatabaseCommand(name, c.ifNotExists, location, comment, newProperties)

Expand Down
4 changes: 2 additions & 2 deletions ...e/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat

case SetNamespaceLocation(ResolvedNamespace(catalog, ns), location) =>
if (StringUtils.isEmpty(location)) {
throw QueryExecutionErrors.unsupportedEmptyLocationError()
throw QueryExecutionErrors.invalidEmptyLocationError(location)
}
AlterNamespaceSetPropertiesExec(
catalog.asNamespaceCatalog,
Expand All @@ -369,7 +369,7 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
case CreateNamespace(ResolvedNamespace(catalog, ns), ifNotExists, properties) =>
val location = properties.get(SupportsNamespaces.PROP_LOCATION)
if (location.isDefined && location.get.isEmpty) {
throw QueryExecutionErrors.unsupportedEmptyLocationError()
throw QueryExecutionErrors.invalidEmptyLocationError(location)
}
val finalProperties = properties.get(SupportsNamespaces.PROP_LOCATION).map { loc =>
properties + (SupportsNamespaces.PROP_LOCATION -> makeQualifiedDBObjectPath(loc))
Expand Down
2 changes: 1 addition & 1 deletion ...est/scala/org/apache/spark/sql/execution/command/AlterNamespaceSetLocationSuiteBase.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ trait AlterNamespaceSetLocationSuiteBase extends QueryTest with DDLCommandTestUt
exception = intercept[SparkIllegalArgumentException] {
sql(sqlText)
},
errorClass = "UNSUPPORTED_EMPTY_LOCATION",
errorClass = "INVALID_EMPTY_LOCATION",
parameters = Map.empty)
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...core/src/test/scala/org/apache/spark/sql/execution/command/CreateNamespaceSuiteBase.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ trait CreateNamespaceSuiteBase extends QueryTest with DDLCommandTestUtils {
exception = intercept[SparkIllegalArgumentException] {
sql(sqlText)
},
errorClass = "UNSUPPORTED_EMPTY_LOCATION",
errorClass = "INVALID_EMPTY_LOCATION",
parameters = Map.empty)
val uri = new Path(path).toUri
sql(s"CREATE NAMESPACE $ns LOCATION '$uri'")
Expand Down