You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by ge...@apache.org on 2024/07/30 00:35:41 UTC
(spark) branch master updated: [SPARK-48740][SQL] Catch missing window specification error early
This is an automated email from the ASF dual-hosted git repository.
gengliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 0b48d3f61b72 [SPARK-48740][SQL] Catch missing window specification error early
0b48d3f61b72 is described below
commit 0b48d3f61b726209e96b0b967530534b5ad9101d
Author: Amanda Liu <am...@databricks.com>
AuthorDate: Mon Jul 29 17:35:28 2024 -0700
[SPARK-48740][SQL] Catch missing window specification error early
### What changes were proposed in this pull request?
Before, aggregate queries containing a window function without a window specification (e.g. `PARTITION BY`) would return a non-descriptive internal error message:
`org.apache.spark.sql.catalyst.analysis.UnresolvedException: [INTERNAL_ERROR] Invalid call to exprId on unresolved object SQLSTATE: XX000`
This PR catches the user error early and returns a more accurate description of the issue:
`Window specification is not defined in the WINDOW clause for <windowName>. For more information about WINDOW clauses, please refer to '<docroot>/sql-ref-syntax-qry-select-window.html'.`
### Why are the changes needed?
This change produces a more descriptive error message for window functions, improving the Spark user experience.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Added unit test in `sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala`
### Was this patch authored or co-authored using generative AI tooling?
No
Closes #47129 from asl3/improve-window-partition-error.
Lead-authored-by: Amanda Liu <am...@databricks.com>
Co-authored-by: Gengliang Wang <ge...@apache.org>
Signed-off-by: Gengliang Wang <ge...@apache.org>
---
.../src/main/resources/error/error-conditions.json | 11 ++++++-----
.../spark/sql/catalyst/analysis/CheckAnalysis.scala | 19 ++++++++++++++-----
.../spark/sql/errors/QueryCompilationErrors.scala | 7 +++++--
.../sql/catalyst/analysis/AnalysisErrorSuite.scala | 21 ++++++++++++++++++++-
.../sql-tests/analyzer-results/window.sql.out | 8 ++++++--
.../test/resources/sql-tests/results/window.sql.out | 8 ++++++--
6 files changed, 57 insertions(+), 17 deletions(-)
diff --git a/common/utils/src/main/resources/error/error-conditions.json b/common/utils/src/main/resources/error/error-conditions.json
index 4f1172d6968e..0187f985770b 100644
--- a/common/utils/src/main/resources/error/error-conditions.json
+++ b/common/utils/src/main/resources/error/error-conditions.json
@@ -3195,6 +3195,12 @@
],
"sqlState" : "42803"
},
+ "MISSING_WINDOW_SPECIFICATION" : {
+ "message" : [
+ "Window specification is not defined in the WINDOW clause for <windowName>. For more information about WINDOW clauses, please refer to '<docroot>/sql-ref-syntax-qry-select-window.html'."
+ ],
+ "sqlState" : "42P20"
+ },
"MULTIPLE_TIME_TRAVEL_SPEC" : {
"message" : [
"Cannot specify time travel in both the time travel clause and options."
@@ -5326,11 +5332,6 @@
"Try moving this class out of its parent class."
]
},
- "_LEGACY_ERROR_TEMP_1004" : {
- "message" : [
- "Window specification <windowName> is not defined in the WINDOW clause."
- ]
- },
"_LEGACY_ERROR_TEMP_1005" : {
"message" : [
"<expr> doesn't show up in the GROUP BY list <groupByAliases>."
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
index 3a18b0c4d2ea..b4753d0c6e4f 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
@@ -154,6 +154,15 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog with QueryErrorsB
}
}
+ private def checkForUnspecifiedWindow(expressions: Seq[Expression]): Unit = {
+ expressions.foreach(_.transformDownWithPruning(
+ _.containsPattern(UNRESOLVED_WINDOW_EXPRESSION)) {
+ case UnresolvedWindowExpression(_, windowSpec) =>
+ throw QueryCompilationErrors.windowSpecificationNotDefinedError(windowSpec.name)
+ }
+ )
+ }
+
def checkAnalysis(plan: LogicalPlan): Unit = {
// We should inline all CTE relations to restore the original plan shape, as the analysis check
// may need to match certain plan shapes. For dangling CTE relations, they will still be kept
@@ -668,11 +677,11 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog with QueryErrorsB
}
case p @ Project(projectList, _) =>
- projectList.foreach(_.transformDownWithPruning(
- _.containsPattern(UNRESOLVED_WINDOW_EXPRESSION)) {
- case UnresolvedWindowExpression(_, windowSpec) =>
- throw QueryCompilationErrors.windowSpecificationNotDefinedError(windowSpec.name)
- })
+ checkForUnspecifiedWindow(projectList)
+
+ case agg@Aggregate(_, aggregateExpressions, _) if
+ PlanHelper.specialExpressionsInUnsupportedOperator(agg).isEmpty =>
+ checkForUnspecifiedWindow(aggregateExpressions)
case j: Join if !j.duplicateResolved =>
val conflictingAttributes =
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
index d1acc0dd7075..dd02597801d6 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
@@ -448,8 +448,11 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase with Compilat
def windowSpecificationNotDefinedError(windowName: String): Throwable = {
new AnalysisException(
- errorClass = "_LEGACY_ERROR_TEMP_1004",
- messageParameters = Map("windowName" -> windowName))
+ errorClass = "MISSING_WINDOW_SPECIFICATION",
+ messageParameters = Map(
+ "windowName" -> windowName,
+ "docroot" -> SPARK_DOC_ROOT)
+ )
}
def selectExprNotInGroupByError(expr: Expression, groupByAliases: Seq[Alias]): Throwable = {
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala
index 6c6414dd190d..cfe08e189536 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala
@@ -17,7 +17,7 @@
package org.apache.spark.sql.catalyst.analysis
-import org.apache.spark.SparkException
+import org.apache.spark.{SPARK_DOC_ROOT, SparkException}
import org.apache.spark.sql.catalyst.TableIdentifier
import org.apache.spark.sql.catalyst.dsl.expressions._
import org.apache.spark.sql.catalyst.dsl.plans._
@@ -261,6 +261,25 @@ class AnalysisErrorSuite extends AnalysisTest with DataTypeErrorsBase {
Array(ExpectedContext("percent_rank(a) FILTER (WHERE c > 1) OVER ()", 7, 50)))
}
+ test("window specification error") {
+ assertAnalysisErrorClass(
+ inputPlan = CatalystSqlParser.parsePlan(
+ """
+ |WITH sample_data AS (
+ | SELECT 1 AS a, 10 AS b UNION ALL
+ | SELECT 2 AS a, 20 AS b
+ |)
+ |SELECT
+ | AVG(a) OVER (b) AS avg_a
+ |FROM sample_data
+ |GROUP BY a, b;
+ |""".stripMargin),
+ expectedErrorClass = "MISSING_WINDOW_SPECIFICATION",
+ expectedMessageParameters = Map(
+ "windowName" -> "b",
+ "docroot" -> SPARK_DOC_ROOT))
+ }
+
test("higher order function with filter predicate") {
assertAnalysisErrorClass(
CatalystSqlParser.parsePlan("SELECT aggregate(array(1, 2, 3), 0, (acc, x) -> acc + x) " +
diff --git a/sql/core/src/test/resources/sql-tests/analyzer-results/window.sql.out b/sql/core/src/test/resources/sql-tests/analyzer-results/window.sql.out
index 8c129534e7d0..12a90ce32cc6 100644
--- a/sql/core/src/test/resources/sql-tests/analyzer-results/window.sql.out
+++ b/sql/core/src/test/resources/sql-tests/analyzer-results/window.sql.out
@@ -1259,8 +1259,10 @@ FROM
-- !query analysis
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
- "errorClass" : "_LEGACY_ERROR_TEMP_1004",
+ "errorClass" : "MISSING_WINDOW_SPECIFICATION",
+ "sqlState" : "42P20",
"messageParameters" : {
+ "docroot" : "https://spark.apache.org/docs/latest",
"windowName" : "w"
}
}
@@ -1274,8 +1276,10 @@ FROM
-- !query analysis
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
- "errorClass" : "_LEGACY_ERROR_TEMP_1004",
+ "errorClass" : "MISSING_WINDOW_SPECIFICATION",
+ "sqlState" : "42P20",
"messageParameters" : {
+ "docroot" : "https://spark.apache.org/docs/latest",
"windowName" : "w"
}
}
diff --git a/sql/core/src/test/resources/sql-tests/results/window.sql.out b/sql/core/src/test/resources/sql-tests/results/window.sql.out
index d70f7c974c2a..96cf9e53787f 100644
--- a/sql/core/src/test/resources/sql-tests/results/window.sql.out
+++ b/sql/core/src/test/resources/sql-tests/results/window.sql.out
@@ -1320,8 +1320,10 @@ struct<>
-- !query output
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
- "errorClass" : "_LEGACY_ERROR_TEMP_1004",
+ "errorClass" : "MISSING_WINDOW_SPECIFICATION",
+ "sqlState" : "42P20",
"messageParameters" : {
+ "docroot" : "https://spark.apache.org/docs/latest",
"windowName" : "w"
}
}
@@ -1337,8 +1339,10 @@ struct<>
-- !query output
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
- "errorClass" : "_LEGACY_ERROR_TEMP_1004",
+ "errorClass" : "MISSING_WINDOW_SPECIFICATION",
+ "sqlState" : "42P20",
"messageParameters" : {
+ "docroot" : "https://spark.apache.org/docs/latest",
"windowName" : "w"
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org