MaxGekk commented on code in PR #41504:
URL: https://github.com/apache/spark/pull/41504#discussion_r1223887503
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -191,6 +196,11 @@
],
"sqlState" : "0AKD0"
},
+ "CANNOT_RESOLVE_STAR_EXPAND" : {
+ "message" : [
+ "Cannot resolve '<targetString>.*' given input columns '<columns>'.
Please check that the specified table or struct exists and is accessible in the
input columns."
Review Comment:
Incorrect quoting of <columns>. Please, remove '' and use `toSQLId` (if they
are ids).
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -157,6 +157,11 @@
],
"sqlState" : "22018"
},
+ "CANNOT_PARSE_INTERVAL" : {
+ "message" : [
+ "Unable to parse '<intervalString>'. Please ensure that the value
provided is in a valid format for defining an interval. You can reference the
documentation for the correct format. If the issue persists, please double
check that the input value is not null or empty and try again."
Review Comment:
Please, quote `intervalString` using `toSQLValue`.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveInlineTables.scala:
##########
@@ -116,7 +116,7 @@ object ResolveInlineTables extends Rule[LogicalPlan] with
CastSupport with Alias
} catch {
case NonFatal(ex) =>
table.failAnalysis(
- errorClass = "_LEGACY_ERROR_TEMP_2331",
+ errorClass = "FAILED_SQL_EXPRESSION_EVALUATION",
messageParameters = Map("sqlExpr" -> e.sql, "msg" ->
ex.getMessage),
Review Comment:
Quote `e` by `toSQLExpr`
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -1754,6 +1779,11 @@
],
"sqlState" : "42826"
},
+ "OPERATION_NOT_ALLOWED" : {
+ "message" : [
+ "Operation not allowed: <message>. This error occurs when attempting an
operation that is not currently supported. Please check the documentation for
the list of allowable operations."
Review Comment:
Instead of the generic `message`. Please, add sub-classes.
I told you about this in your PR https://github.com/apache/spark/pull/39965
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveInlineTables.scala:
##########
@@ -116,7 +116,7 @@ object ResolveInlineTables extends Rule[LogicalPlan] with
CastSupport with Alias
} catch {
case NonFatal(ex) =>
table.failAnalysis(
- errorClass = "_LEGACY_ERROR_TEMP_2331",
+ errorClass = "FAILED_SQL_EXPRESSION_EVALUATION",
messageParameters = Map("sqlExpr" -> e.sql, "msg" ->
ex.getMessage),
Review Comment:
Regarding to `msg`, it seems from errors it comes from another
`SparkThrowable`. Please, just bypass such kind of exceptions.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -2069,7 +2069,7 @@ class Analyzer(override val catalogManager:
CatalogManager) extends RuleExecutor
} catch {
case _: NoSuchFunctionException =>
u.failAnalysis(
- errorClass = "_LEGACY_ERROR_TEMP_2308",
+ errorClass = "UNRESOLVABLE_TABLE_VALUED_FUNCTION",
messageParameters = Map("name" -> u.name.quoted))
Review Comment:
Use `toSQLId` for quoting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
