Skip to content

Commit

Permalink
[SPARK-38949][SQL] Wrap SQL statements by double quotes in error mess…
Browse files Browse the repository at this point in the history
…ages

In the PR, I propose to wrap any SQL statement in error messages by double quotes "", and apply new implementation of `QueryErrorsBase.toSQLStmt()` to all exceptions in `Query.*Errors` w/ error classes. Also this PR modifies all affected tests, see the list in the section "How was this patch tested?".

To improve user experience with Spark SQL by highlighting SQL statements in error massage and make them more visible to users.

Yes. The changes might influence on error messages that are visible to users.

Before:
```sql
The operation DESC PARTITION is not allowed
```

After:
```sql
The operation "DESC PARTITION" is not allowed
```

By running affected test suites:
```
$ build/sbt "sql/testOnly *QueryExecutionErrorsSuite"
$ build/sbt "sql/testOnly *QueryParsingErrorsSuite"
$ build/sbt "sql/testOnly *QueryCompilationErrorsSuite"
$ build/sbt "test:testOnly *QueryCompilationErrorsDSv2Suite"
$ build/sbt "test:testOnly *ExtractPythonUDFFromJoinConditionSuite"
$ build/sbt "testOnly *PlanParserSuite"
$ build/sbt "sql/testOnly *SQLQueryTestSuite -- -z transform.sql"
$ build/sbt "sql/testOnly *SQLQueryTestSuite -- -z join-lateral.sql"
$ build/sbt "sql/testOnly *SQLQueryTestSuite -- -z describe.sql"
```

Closes apache#36259 from MaxGekk/error-class-apply-toSQLStmt.

Authored-by: Max Gekk <max.gekk@gmail.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit 5aba2b3)
Signed-off-by: Max Gekk <max.gekk@gmail.com>
  • Loading branch information
MaxGekk committed Apr 20, 2022
1 parent 9d0650a commit a8a8cb4
Show file tree
Hide file tree
Showing 16 changed files with 82 additions and 54 deletions.
13 changes: 7 additions & 6 deletions python/pyspark/sql/tests/test_udf.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,15 +258,16 @@ def test_udf_not_supported_in_join_condition(self):
def runWithJoinType(join_type, type_string):
with self.assertRaisesRegex(
AnalysisException,
"Using PythonUDF in join condition of join type %s is not supported" % type_string,
"""Using PythonUDF in join condition of join type "%s" is not supported"""
% type_string,
):
left.join(right, [f("a", "b"), left.a1 == right.b1], join_type).collect()

runWithJoinType("full", "FullOuter")
runWithJoinType("left", "LeftOuter")
runWithJoinType("right", "RightOuter")
runWithJoinType("leftanti", "LeftAnti")
runWithJoinType("leftsemi", "LeftSemi")
runWithJoinType("full", "FULL OUTER")
runWithJoinType("left", "LEFT OUTER")
runWithJoinType("right", "RIGHT OUTER")
runWithJoinType("leftanti", "LEFT ANTI")
runWithJoinType("leftsemi", "LEFT SEMI")

def test_udf_as_join_condition(self):
left = self.spark.createDataFrame([Row(a=1, a1=1, a2=1), Row(a=2, a1=2, a2=2)])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,7 @@ class AstBuilder extends SqlBaseParserBaseVisitor[AnyRef] with SQLConfHelper wit
}
if (join.LATERAL != null) {
if (!Seq(Inner, Cross, LeftOuter).contains(joinType)) {
throw QueryParsingErrors.unsupportedLateralJoinTypeError(ctx, joinType.toString)
throw QueryParsingErrors.unsupportedLateralJoinTypeError(ctx, joinType.sql)
}
LateralJoin(left, LateralSubquery(plan(join.right)), joinType, condition)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ object QueryCompilationErrors extends QueryErrorsBase {
def unsupportedIfNotExistsError(tableName: String): Throwable = {
new AnalysisException(
errorClass = "UNSUPPORTED_FEATURE",
messageParameters = Array(s"IF NOT EXISTS for the table '$tableName' by INSERT INTO."))
messageParameters = Array(
s"${toSQLStmt("IF NOT EXISTS")} for the table '$tableName' " +
s"by ${toSQLStmt("INSERT INTO")}."))
}

def nonPartitionColError(partitionName: String): Throwable = {
Expand Down Expand Up @@ -1576,7 +1578,8 @@ object QueryCompilationErrors extends QueryErrorsBase {
new AnalysisException(
errorClass = "UNSUPPORTED_FEATURE",
messageParameters = Array(
s"Using PythonUDF in join condition of join type $joinType is not supported"))
"Using PythonUDF in join condition of join type " +
s"${toSQLStmt(joinType.sql)} is not supported."))
}

def conflictingAttributesInJoinConditionError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package org.apache.spark.sql.errors

import java.util.Locale

import org.apache.spark.sql.catalyst.expressions.Literal
import org.apache.spark.sql.types.{DataType, DoubleType, FloatType}

Expand Down Expand Up @@ -45,6 +47,11 @@ trait QueryErrorsBase {
litToErrorValue(Literal.create(v, t))
}

// Quote sql statements in error messages.
def toSQLStmt(text: String): String = {
"\"" + text.toUpperCase(Locale.ROOT) + "\""
}

def toSQLType(t: DataType): String = {
t.sql
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1935,13 +1935,13 @@ object QueryExecutionErrors extends QueryErrorsBase {
def repeatedPivotsUnsupportedError(): Throwable = {
new SparkUnsupportedOperationException(
errorClass = "UNSUPPORTED_FEATURE",
messageParameters = Array("Repeated pivots."))
messageParameters = Array(s"Repeated ${toSQLStmt("pivot")}s."))
}

def pivotNotAfterGroupByUnsupportedError(): Throwable = {
new SparkUnsupportedOperationException(
errorClass = "UNSUPPORTED_FEATURE",
messageParameters = Array("Pivot not after a groupBy."))
messageParameters = Array(s"${toSQLStmt("pivot")} not after a ${toSQLStmt("group by")}."))
}

def invalidAesKeyLengthError(actualLength: Int): RuntimeException = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,33 +91,51 @@ object QueryParsingErrors extends QueryErrorsBase {
}

def transformNotSupportQuantifierError(ctx: ParserRuleContext): Throwable = {
new ParseException("UNSUPPORTED_FEATURE",
Array("TRANSFORM does not support DISTINCT/ALL in inputs"), ctx)
new ParseException(
errorClass = "UNSUPPORTED_FEATURE",
messageParameters = Array(s"${toSQLStmt("TRANSFORM")} does not support" +
s" ${toSQLStmt("DISTINCT")}/${toSQLStmt("ALL")} in inputs"),
ctx)
}

def transformWithSerdeUnsupportedError(ctx: ParserRuleContext): Throwable = {
new ParseException("UNSUPPORTED_FEATURE",
Array("TRANSFORM with serde is only supported in hive mode"), ctx)
new ParseException(
errorClass = "UNSUPPORTED_FEATURE",
messageParameters = Array(
s"${toSQLStmt("TRANSFORM")} with serde is only supported in hive mode"),
ctx)
}

def lateralWithPivotInFromClauseNotAllowedError(ctx: FromClauseContext): Throwable = {
new ParseException("LATERAL cannot be used together with PIVOT in FROM clause", ctx)
}

def lateralJoinWithNaturalJoinUnsupportedError(ctx: ParserRuleContext): Throwable = {
new ParseException("UNSUPPORTED_FEATURE", Array("LATERAL join with NATURAL join."), ctx)
new ParseException(
errorClass = "UNSUPPORTED_FEATURE",
messageParameters = Array(s"${toSQLStmt("LATERAL")} join with ${toSQLStmt("NATURAL")} join."),
ctx)
}

def lateralJoinWithUsingJoinUnsupportedError(ctx: ParserRuleContext): Throwable = {
new ParseException("UNSUPPORTED_FEATURE", Array("LATERAL join with USING join."), ctx)
new ParseException(
errorClass = "UNSUPPORTED_FEATURE",
messageParameters = Array(s"${toSQLStmt("LATERAL")} join with ${toSQLStmt("USING")} join."),
ctx)
}

def unsupportedLateralJoinTypeError(ctx: ParserRuleContext, joinType: String): Throwable = {
new ParseException("UNSUPPORTED_FEATURE", Array(s"LATERAL join type '$joinType'."), ctx)
new ParseException(
errorClass = "UNSUPPORTED_FEATURE",
messageParameters = Array(s"${toSQLStmt("LATERAL")} join type ${toSQLStmt(joinType)}."),
ctx)
}

def invalidLateralJoinRelationError(ctx: RelationPrimaryContext): Throwable = {
new ParseException("INVALID_SQL_SYNTAX", Array("LATERAL can only be used with subquery."), ctx)
new ParseException(
errorClass = "INVALID_SQL_SYNTAX",
messageParameters = Array(s"${toSQLStmt("LATERAL")} can only be used with subquery."),
ctx)
}

def repetitiveWindowDefinitionError(name: String, ctx: WindowClauseContext): Throwable = {
Expand All @@ -136,7 +154,7 @@ object QueryParsingErrors extends QueryErrorsBase {
}

def naturalCrossJoinUnsupportedError(ctx: RelationContext): Throwable = {
new ParseException("UNSUPPORTED_FEATURE", Array("NATURAL CROSS JOIN."), ctx)
new ParseException("UNSUPPORTED_FEATURE", Array(toSQLStmt("NATURAL CROSS JOIN") + "."), ctx)
}

def emptyInputForTableSampleError(ctx: ParserRuleContext): Throwable = {
Expand Down Expand Up @@ -298,14 +316,18 @@ object QueryParsingErrors extends QueryErrorsBase {
}

def showFunctionsUnsupportedError(identifier: String, ctx: IdentifierContext): Throwable = {
new ParseException(s"SHOW $identifier FUNCTIONS not supported", ctx)
new ParseException(
errorClass = "INVALID_SQL_SYNTAX",
messageParameters = Array(
s"${toSQLStmt("SHOW")} $identifier ${toSQLStmt("FUNCTIONS")} not supported"),
ctx)
}

def showFunctionsInvalidPatternError(pattern: String, ctx: ParserRuleContext): Throwable = {
new ParseException(
errorClass = "INVALID_SQL_SYNTAX",
messageParameters = Array(
s"Invalid pattern in SHOW FUNCTIONS: $pattern. " +
s"Invalid pattern in ${toSQLStmt("SHOW FUNCTIONS")}: $pattern. " +
s"It must be a ${toSQLType(StringType)} literal."),
ctx)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,9 @@ class ExtractPythonUDFFromJoinConditionSuite extends PlanTest {
condition = Some(unevaluableJoinCond))
Optimize.execute(query.analyze)
}
assert(e.message.contentEquals(
assert(e.message ==
"The feature is not supported: " +
s"Using PythonUDF in join condition of join type $joinType is not supported"))
s"""Using PythonUDF in join condition of join type "${joinType.sql}" is not supported.""")

val query2 = testRelationLeft.join(
testRelationRight,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1254,7 +1254,7 @@ class PlanParserSuite extends AnalysisTest {
| "escapeChar" = "\\")
|FROM testData
""".stripMargin,
"TRANSFORM with serde is only supported in hive mode")
"\"TRANSFORM\" with serde is only supported in hive mode")
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ struct<>
-- !query output
org.apache.spark.sql.catalyst.parser.ParseException

The feature is not supported: LATERAL join with NATURAL join.(line 1, pos 14)
The feature is not supported: "LATERAL" join with "NATURAL" join.(line 1, pos 14)

== SQL ==
SELECT * FROM t1 NATURAL JOIN LATERAL (SELECT c1 + c2 AS c2)
Expand All @@ -167,7 +167,7 @@ struct<>
-- !query output
org.apache.spark.sql.catalyst.parser.ParseException

The feature is not supported: LATERAL join with USING join.(line 1, pos 14)
The feature is not supported: "LATERAL" join with "USING" join.(line 1, pos 14)

== SQL ==
SELECT * FROM t1 JOIN LATERAL (SELECT c1 + c2 AS c2) USING (c2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,7 @@ struct<>
-- !query output
org.apache.spark.sql.catalyst.parser.ParseException

The feature is not supported: TRANSFORM does not support DISTINCT/ALL in inputs(line 1, pos 17)
The feature is not supported: "TRANSFORM" does not support "DISTINCT"/"ALL" in inputs(line 1, pos 17)

== SQL ==
SELECT TRANSFORM(DISTINCT b, a, c)
Expand All @@ -739,7 +739,7 @@ struct<>
-- !query output
org.apache.spark.sql.catalyst.parser.ParseException

The feature is not supported: TRANSFORM does not support DISTINCT/ALL in inputs(line 1, pos 17)
The feature is not supported: "TRANSFORM" does not support "DISTINCT"/"ALL" in inputs(line 1, pos 17)

== SQL ==
SELECT TRANSFORM(ALL b, a, c)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class QueryCompilationErrorsDSv2Suite

checkAnswer(spark.table(tbl), spark.emptyDataFrame)
assert(e.getMessage === "The feature is not supported: " +
s"IF NOT EXISTS for the table '$tbl' by INSERT INTO.")
s""""IF NOT EXISTS" for the table '$tbl' by "INSERT INTO".""")
assert(e.getErrorClass === "UNSUPPORTED_FEATURE")
assert(e.getSqlState === "0A000")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
assert(e.getSqlState === "0A000")
assert(e.message ===
"The feature is not supported: " +
"Using PythonUDF in join condition of join type LeftOuter is not supported")
"Using PythonUDF in join condition of join type \"LEFT OUTER\" is not supported.")
}

test("UNSUPPORTED_FEATURE: Using pandas UDF aggregate expression with pivot") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ class QueryExecutionErrorsSuite extends QueryTest
}
assert(e1.getErrorClass === "UNSUPPORTED_FEATURE")
assert(e1.getSqlState === "0A000")
assert(e1.getMessage === "The feature is not supported: Repeated pivots.")
assert(e1.getMessage === """The feature is not supported: Repeated "PIVOT"s.""")

val e2 = intercept[SparkUnsupportedOperationException] {
trainingSales
Expand All @@ -167,7 +167,7 @@ class QueryExecutionErrorsSuite extends QueryTest
}
assert(e2.getErrorClass === "UNSUPPORTED_FEATURE")
assert(e2.getSqlState === "0A000")
assert(e2.getMessage === "The feature is not supported: Pivot not after a groupBy.")
assert(e2.getMessage === """The feature is not supported: "PIVOT" not after a "GROUP BY".""")
}

test("INCONSISTENT_BEHAVIOR_CROSS_VERSION: " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import org.apache.spark.sql.QueryTest
import org.apache.spark.sql.catalyst.parser.ParseException
import org.apache.spark.sql.test.SharedSparkSession

// Turn of the length check because most of the tests check entire error messages
// scalastyle:off line.size.limit
class QueryParsingErrorsSuite extends QueryTest with SharedSparkSession {
def validateParsingError(
sqlText: String,
Expand All @@ -42,7 +44,7 @@ class QueryParsingErrorsSuite extends QueryTest with SharedSparkSession {
sqlState = "0A000",
message =
"""
|The feature is not supported: LATERAL join with NATURAL join.(line 1, pos 14)
|The feature is not supported: "LATERAL" join with "NATURAL" join.(line 1, pos 14)
|
|== SQL ==
|SELECT * FROM t1 NATURAL JOIN LATERAL (SELECT c1 + c2 AS c2)
Expand All @@ -57,7 +59,7 @@ class QueryParsingErrorsSuite extends QueryTest with SharedSparkSession {
sqlState = "0A000",
message =
"""
|The feature is not supported: LATERAL join with USING join.(line 1, pos 14)
|The feature is not supported: "LATERAL" join with "USING" join.(line 1, pos 14)
|
|== SQL ==
|SELECT * FROM t1 JOIN LATERAL (SELECT c1 + c2 AS c2) USING (c2)
Expand All @@ -66,21 +68,17 @@ class QueryParsingErrorsSuite extends QueryTest with SharedSparkSession {
}

test("UNSUPPORTED_FEATURE: Unsupported LATERAL join type") {
Seq(
("RIGHT OUTER", "RightOuter"),
("FULL OUTER", "FullOuter"),
("LEFT SEMI", "LeftSemi"),
("LEFT ANTI", "LeftAnti")).foreach { pair =>
Seq("RIGHT OUTER", "FULL OUTER", "LEFT SEMI", "LEFT ANTI").foreach { joinType =>
validateParsingError(
sqlText = s"SELECT * FROM t1 ${pair._1} JOIN LATERAL (SELECT c1 + c2 AS c3) ON c2 = c3",
sqlText = s"SELECT * FROM t1 $joinType JOIN LATERAL (SELECT c1 + c2 AS c3) ON c2 = c3",
errorClass = "UNSUPPORTED_FEATURE",
sqlState = "0A000",
message =
s"""
|The feature is not supported: LATERAL join type '${pair._2}'.(line 1, pos 14)
|The feature is not supported: "LATERAL" join type "$joinType".(line 1, pos 14)
|
|== SQL ==
|SELECT * FROM t1 ${pair._1} JOIN LATERAL (SELECT c1 + c2 AS c3) ON c2 = c3
|SELECT * FROM t1 $joinType JOIN LATERAL (SELECT c1 + c2 AS c3) ON c2 = c3
|--------------^^^
|""".stripMargin)
}
Expand All @@ -101,7 +99,7 @@ class QueryParsingErrorsSuite extends QueryTest with SharedSparkSession {
sqlState = "42000",
message =
s"""
|Invalid SQL syntax: LATERAL can only be used with subquery.(line 1, pos $pos)
|Invalid SQL syntax: "LATERAL" can only be used with subquery.(line 1, pos $pos)
|
|== SQL ==
|$sqlText
Expand All @@ -117,7 +115,7 @@ class QueryParsingErrorsSuite extends QueryTest with SharedSparkSession {
sqlState = "0A000",
message =
"""
|The feature is not supported: NATURAL CROSS JOIN.(line 1, pos 14)
|The feature is not supported: "NATURAL CROSS JOIN".(line 1, pos 14)
|
|== SQL ==
|SELECT * FROM a NATURAL CROSS JOIN b
Expand Down Expand Up @@ -177,8 +175,7 @@ class QueryParsingErrorsSuite extends QueryTest with SharedSparkSession {
sqlState = "0A000",
message =
"""
|The feature is not supported: """.stripMargin +
"""TRANSFORM does not support DISTINCT/ALL in inputs(line 1, pos 17)
|The feature is not supported: "TRANSFORM" does not support "DISTINCT"/"ALL" in inputs(line 1, pos 17)
|
|== SQL ==
|SELECT TRANSFORM(DISTINCT a) USING 'a' FROM t
Expand All @@ -194,12 +191,10 @@ class QueryParsingErrorsSuite extends QueryTest with SharedSparkSession {
sqlState = "0A000",
message =
"""
|The feature is not supported: """.stripMargin +
"""TRANSFORM with serde is only supported in hive mode(line 1, pos 0)
|The feature is not supported: "TRANSFORM" with serde is only supported in hive mode(line 1, pos 0)
|
|== SQL ==
|SELECT TRANSFORM(a) ROW FORMAT SERDE """.stripMargin +
"""'org.apache.hadoop.hive.serde2.OpenCSVSerde' USING 'a' FROM t
|SELECT TRANSFORM(a) ROW FORMAT SERDE 'org.apache.hadoop.hive.serde2.OpenCSVSerde' USING 'a' FROM t
|^^^
|""".stripMargin)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class SparkScriptTransformationSuite extends BaseScriptTransformationSuite with
|FROM v
""".stripMargin)
}.getMessage
assert(e.contains("TRANSFORM with serde is only supported in hive mode"))
assert(e.contains("\"TRANSFORM\" with serde is only supported in hive mode"))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -484,9 +484,9 @@ class DDLParserSuite extends AnalysisTest with SharedSparkSession {
DropFunctionCommand(None, "a", true, true))

intercept("DROP TEMPORARY FUNCTION a.b",
"DROP TEMPORARY FUNCTION requires a single part name")
"\"DROP TEMPORARY FUNCTION\" requires a single part name")
intercept("DROP TEMPORARY FUNCTION IF EXISTS a.b",
"DROP TEMPORARY FUNCTION requires a single part name")
"\"DROP TEMPORARY FUNCTION\" requires a single part name")
}

test("SPARK-32374: create temporary view with properties not allowed") {
Expand Down

0 comments on commit a8a8cb4

Please sign in to comment.