[SPARK-45163][SQL] Merge UNSUPPORTED_VIEW_OPERATION & UNSUPPORTED_TAB…

…LE_OPERATION & fix some issue ### What changes were proposed in this pull request? A.The pr aims to - Merge `UNSUPPORTED_VIEW_OPERATION.WITH_SUGGESTION` into `EXPECT_TABLE_NOT_VIEW.USE_ALTER_VIEW` (new added) - Merge `UNSUPPORTED_VIEW_OPERATION.WITHOUT_SUGGESTION ` into `EXPECT_TABLE_NOT_VIEW.NO_ALTERNATIVE` - Merge `UNSUPPORTED_TABLE_OPERATION.WITH_SUGGESTION` into `EXPECT_VIEW_NOT_TABLE.USE_ALTER_TABLE` - Merge `UNSUPPORTED_TABLE_OPERATION.WITHOUT_SUGGESTION` into `EXPECT_VIEW_NOT_TABLE.NO_ALTERNATIVE` - Merge `UNSUPPORTED_TABLE_OPERATION.WITH_SUGGESTION` into `EXPECT_VIEW_NOT_TABLE.USE_ALTER_VIEW` - Add `EXPECT_PERMANENT_VIEW_NOT_TEMP` - Fix some naming issues based on the suggestions of [PR](apache#42824) reviewers. B.The pr is also follow up apache#42824. ### Why are the changes needed? - Better code readability. - Fix some error message prompt. - Fix a type: "ALTER COLUMN ... FIRST | ALTER" -> "ALTER COLUMN ... FIRST | AFTER" ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#42917 from panbingkun/SPARK-45085_FOLLOWUP. Authored-by: panbingkun <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
beliefer · Sep 22, 2023 · 39d6fda · 39d6fda
1 parent f4f3b28
commit 39d6fda
Show file tree

Hide file tree

Showing 26 changed files with 196 additions and 180 deletions.
diff --git a/R/pkg/tests/fulltests/test_sparkSQL.R b/R/pkg/tests/fulltests/test_sparkSQL.R
@@ -4199,7 +4199,7 @@ test_that("catalog APIs, listTables, getTable, listColumns, listFunctions, funct
 
   # recoverPartitions does not work with temporary view
   expect_error(recoverPartitions("cars"),
-               "[UNSUPPORTED_VIEW_OPERATION.WITH_SUGGESTION]*`cars`*")
+               "[EXPECT_TABLE_NOT_VIEW.NO_ALTERNATIVE]*`cars`*")
   expect_error(refreshTable("cars"), NA)
   expect_error(refreshByPath("/"), NA)
 

diff --git a/common/utils/src/main/resources/error/error-classes.json b/common/utils/src/main/resources/error/error-classes.json
@@ -877,6 +877,45 @@
       "Exceeds char/varchar type length limitation: <limit>."
     ]
   },
+  "EXPECT_PERMANENT_VIEW_NOT_TEMP" : {
+    "message" : [
+      "'<operation>' expects a permanent view but <viewName> is a temp view."
+    ]
+  },
+  "EXPECT_TABLE_NOT_VIEW" : {
+    "message" : [
+      "'<operation>' expects a table but <viewName> is a view."
+    ],
+    "subClass" : {
+      "NO_ALTERNATIVE" : {
+        "message" : [
+          ""
+        ]
+      },
+      "USE_ALTER_VIEW" : {
+        "message" : [
+          "Please use ALTER VIEW instead."
+        ]
+      }
+    }
+  },
+  "EXPECT_VIEW_NOT_TABLE" : {
+    "message" : [
+      "The table <tableName> does not support <operation>."
+    ],
+    "subClass" : {
+      "NO_ALTERNATIVE" : {
+        "message" : [
+          ""
+        ]
+      },
+      "USE_ALTER_TABLE" : {
+        "message" : [
+          "Please use ALTER TABLE instead."
+        ]
+      }
+    }
+  },
   "EXPRESSION_TYPE_IS_NOT_ORDERABLE" : {
     "message" : [
       "Column expression <expr> cannot be sorted because its type <exprType> is not orderable."
@@ -3444,46 +3483,12 @@
     },
     "sqlState" : "0A000"
   },
-  "UNSUPPORTED_TABLE_OPERATION" : {
-    "message" : [
-      "The table <tableName> does not support <operation>."
-    ],
-    "subClass" : {
-      "WITHOUT_SUGGESTION" : {
-        "message" : [
-          ""
-        ]
-      },
-      "WITH_SUGGESTION" : {
-        "message" : [
-          "Please use ALTER TABLE instead."
-        ]
-      }
-    }
-  },
   "UNSUPPORTED_TYPED_LITERAL" : {
     "message" : [
       "Literals of the type <unsupportedType> are not supported. Supported types are <supportedTypes>."
     ],
     "sqlState" : "0A000"
   },
-  "UNSUPPORTED_VIEW_OPERATION" : {
-    "message" : [
-      "The view <viewName> does not support <operation>."
-    ],
-    "subClass" : {
-      "WITHOUT_SUGGESTION" : {
-        "message" : [
-          ""
-        ]
-      },
-      "WITH_SUGGESTION" : {
-        "message" : [
-          "Please use ALTER VIEW instead."
-        ]
-      }
-    }
-  },
   "UNTYPED_SCALA_UDF" : {
     "message" : [
       "You're using untyped Scala UDF, which does not have the input type information. Spark may blindly pass null to the Scala closure with primitive-type argument, and the closure will see the default value of the Java type for the null argument, e.g. `udf((x: Int) => x, IntegerType)`, the result is 0 for null input. To get rid of this error, you could:",

diff --git a/...unsupported-view-operation-error-class.md → ...ions-expect-table-not-view-error-class.md b/...unsupported-view-operation-error-class.md → ...ions-expect-table-not-view-error-class.md
@@ -1,7 +1,7 @@
 ---
 layout: global
-title: UNSUPPORTED_VIEW_OPERATION error class
-displayTitle: UNSUPPORTED_VIEW_OPERATION error class
+title: EXPECT_TABLE_NOT_VIEW error class
+displayTitle: EXPECT_TABLE_NOT_VIEW error class
 license: |
   Licensed to the Apache Software Foundation (ASF) under one or more
   contributor license agreements.  See the NOTICE file distributed with
@@ -21,15 +21,15 @@ license: |
 
 SQLSTATE: none assigned
 
-The view `<viewName>` does not support `<operation>`.
+'`<operation>`' expects a table but `<viewName>` is a view.
 
 This error class has the following derived error classes:
 
-## WITHOUT_SUGGESTION
+## NO_ALTERNATIVE
 
 
 
-## WITH_SUGGESTION
+## USE_ALTER_VIEW
 
 Please use ALTER VIEW instead.
 

diff --git a/...nsupported-table-operation-error-class.md → ...ions-expect-view-not-table-error-class.md b/...nsupported-table-operation-error-class.md → ...ions-expect-view-not-table-error-class.md
@@ -1,7 +1,7 @@
 ---
 layout: global
-title: UNSUPPORTED_TABLE_OPERATION error class
-displayTitle: UNSUPPORTED_TABLE_OPERATION error class
+title: EXPECT_VIEW_NOT_TABLE error class
+displayTitle: EXPECT_VIEW_NOT_TABLE error class
 license: |
   Licensed to the Apache Software Foundation (ASF) under one or more
   contributor license agreements.  See the NOTICE file distributed with
@@ -25,11 +25,11 @@ The table `<tableName>` does not support `<operation>`.
 
 This error class has the following derived error classes:
 
-## WITHOUT_SUGGESTION
+## NO_ALTERNATIVE
 
 
 
-## WITH_SUGGESTION
+## USE_ALTER_TABLE
 
 Please use ALTER TABLE instead.
 

diff --git a/docs/sql-error-conditions.md b/docs/sql-error-conditions.md
@@ -523,6 +523,28 @@ SQLSTATE: none assigned
 
 Exceeds char/varchar type length limitation: `<limit>`.
 
+### EXPECT_PERMANENT_VIEW_NOT_TEMP
+
+SQLSTATE: none assigned
+
+'`<operation>`' expects a permanent view but `<viewName>` is a temp view.
+
+### [EXPECT_TABLE_NOT_VIEW](sql-error-conditions-expect-table-not-view-error-class.html)
+
+SQLSTATE: none assigned
+
+'`<operation>`' expects a table but `<viewName>` is a view.
+
+For more details see [EXPECT_TABLE_NOT_VIEW](sql-error-conditions-expect-table-not-view-error-class.html)
+
+### [EXPECT_VIEW_NOT_TABLE](sql-error-conditions-expect-view-not-table-error-class.html)
+
+SQLSTATE: none assigned
+
+The table `<tableName>` does not support `<operation>`.
+
+For more details see [EXPECT_VIEW_NOT_TABLE](sql-error-conditions-expect-view-not-table-error-class.html)
+
 ### EXPRESSION_TYPE_IS_NOT_ORDERABLE
 
 SQLSTATE: none assigned
@@ -2106,28 +2128,12 @@ Unsupported subquery expression:
 
 For more details see [UNSUPPORTED_SUBQUERY_EXPRESSION_CATEGORY](sql-error-conditions-unsupported-subquery-expression-category-error-class.html)
 
-### [UNSUPPORTED_TABLE_OPERATION](sql-error-conditions-unsupported-table-operation-error-class.html)
-
-SQLSTATE: none assigned
-
-The table `<tableName>` does not support `<operation>`.
-
-For more details see [UNSUPPORTED_TABLE_OPERATION](sql-error-conditions-unsupported-table-operation-error-class.html)
-
 ### UNSUPPORTED_TYPED_LITERAL
 
 [SQLSTATE: 0A000](sql-error-conditions-sqlstates.html#class-0A-feature-not-supported)
 
 Literals of the type `<unsupportedType>` are not supported. Supported types are `<supportedTypes>`.
 
-### [UNSUPPORTED_VIEW_OPERATION](sql-error-conditions-unsupported-view-operation-error-class.html)
-
-SQLSTATE: none assigned
-
-The view `<viewName>` does not support `<operation>`.
-
-For more details see [UNSUPPORTED_VIEW_OPERATION](sql-error-conditions-unsupported-view-operation-error-class.html)
-
 ### UNTYPED_SCALA_UDF
 
 SQLSTATE: none assigned

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
@@ -1128,18 +1128,19 @@ class Analyzer(override val catalogManager: CatalogManager) extends RuleExecutor
         lookupTableOrView(identifier).map {
           case v: ResolvedPersistentView =>
             val nameParts = v.catalog.name() +: v.identifier.asMultipartIdentifier
-            throw QueryCompilationErrors.unsupportedViewOperationError(
+            throw QueryCompilationErrors.expectTableNotViewError(
               nameParts, cmd, suggestAlternative, u)
           case _: ResolvedTempView =>
-            throw QueryCompilationErrors.unsupportedViewOperationError(
+            throw QueryCompilationErrors.expectTableNotViewError(
               identifier, cmd, suggestAlternative, u)
           case table => table
         }.getOrElse(u)
 
       case u @ UnresolvedView(identifier, cmd, allowTemp, suggestAlternative) =>
         lookupTableOrView(identifier, viewOnly = true).map {
           case _: ResolvedTempView if !allowTemp =>
-            throw QueryCompilationErrors.unsupportedViewOperationError(identifier, cmd, false, u)
+            throw QueryCompilationErrors.expectPermanentViewNotTempViewError(
+              identifier, cmd, u)
           case t: ResolvedTable =>
             val nameParts = t.catalog.name() +: t.identifier.asMultipartIdentifier
             throw QueryCompilationErrors.expectViewNotTableError(
@@ -1150,7 +1151,8 @@ class Analyzer(override val catalogManager: CatalogManager) extends RuleExecutor
       case u @ UnresolvedTableOrView(identifier, cmd, allowTempView) =>
         lookupTableOrView(identifier).map {
           case _: ResolvedTempView if !allowTempView =>
-            throw QueryCompilationErrors.unsupportedViewOperationError(identifier, cmd, false, u)
+            throw QueryCompilationErrors.expectPermanentViewNotTempViewError(
+              identifier, cmd, u)
           case other => other
         }.getOrElse(u)
     }

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/v2ResolutionPlans.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/v2ResolutionPlans.scala
@@ -54,7 +54,7 @@ case class UnresolvedView(
     multipartIdentifier: Seq[String],
     commandName: String,
     allowTemp: Boolean,
-    suggestAlternative: Boolean) extends UnresolvedLeafNode
+    suggestAlternative: Boolean = false) extends UnresolvedLeafNode
 
 /**
  * Holds the name of a table or view that has yet to be looked up in a catalog. It will

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
@@ -2812,8 +2812,8 @@ class AstBuilder extends DataTypeAstBuilder with SQLConfHelper with Logging {
   private def createUnresolvedTable(
       ctx: IdentifierReferenceContext,
       commandName: String,
-      hint: Boolean = false): LogicalPlan = withOrigin(ctx) {
-    withIdentClause(ctx, UnresolvedTable(_, commandName, hint))
+      suggestAlternative: Boolean = false): LogicalPlan = withOrigin(ctx) {
+    withIdentClause(ctx, UnresolvedTable(_, commandName, suggestAlternative))
   }
 
   /**
@@ -2823,8 +2823,8 @@ class AstBuilder extends DataTypeAstBuilder with SQLConfHelper with Logging {
       ctx: IdentifierReferenceContext,
       commandName: String,
       allowTemp: Boolean = true,
-      hint: Boolean = false): LogicalPlan = withOrigin(ctx) {
-    withIdentClause(ctx, UnresolvedView(_, commandName, allowTemp, hint))
+      suggestAlternative: Boolean = false): LogicalPlan = withOrigin(ctx) {
+    withIdentClause(ctx, UnresolvedView(_, commandName, allowTemp, suggestAlternative))
   }
 
   /**
@@ -4359,7 +4359,7 @@ class AstBuilder extends DataTypeAstBuilder with SQLConfHelper with Logging {
           ctx.identifierReference,
           commandName = "ALTER VIEW ... SET TBLPROPERTIES",
           allowTemp = false,
-          hint = true),
+          suggestAlternative = true),
         cleanedTableProperties)
     } else {
       SetTableProperties(
@@ -4392,7 +4392,7 @@ class AstBuilder extends DataTypeAstBuilder with SQLConfHelper with Logging {
           ctx.identifierReference,
           commandName = "ALTER VIEW ... UNSET TBLPROPERTIES",
           allowTemp = false,
-          hint = true),
+          suggestAlternative = true),
         cleanedProperties,
         ifExists)
     } else {
@@ -4418,8 +4418,7 @@ class AstBuilder extends DataTypeAstBuilder with SQLConfHelper with Logging {
     SetTableLocation(
       createUnresolvedTable(
         ctx.identifierReference,
-        "ALTER TABLE ... SET LOCATION ...",
-        true),
+        "ALTER TABLE ... SET LOCATION ..."),
       Option(ctx.partitionSpec).map(visitNonOptionalPartitionSpec),
       visitLocationSpec(ctx.locationSpec))
   }
@@ -4715,8 +4714,7 @@ class AstBuilder extends DataTypeAstBuilder with SQLConfHelper with Logging {
     RecoverPartitions(
       createUnresolvedTable(
         ctx.identifierReference,
-        "ALTER TABLE ... RECOVER PARTITIONS",
-        true))
+        "ALTER TABLE ... RECOVER PARTITIONS"))
   }
 
   /**
@@ -4745,8 +4743,7 @@ class AstBuilder extends DataTypeAstBuilder with SQLConfHelper with Logging {
     AddPartitions(
       createUnresolvedTable(
         ctx.identifierReference,
-        "ALTER TABLE ... ADD PARTITION ...",
-        true),
+        "ALTER TABLE ... ADD PARTITION ..."),
       specsAndLocs.toSeq,
       ctx.EXISTS != null)
   }
@@ -4764,8 +4761,7 @@ class AstBuilder extends DataTypeAstBuilder with SQLConfHelper with Logging {
     RenamePartitions(
       createUnresolvedTable(
         ctx.identifierReference,
-        "ALTER TABLE ... RENAME TO PARTITION",
-        true),
+        "ALTER TABLE ... RENAME TO PARTITION"),
       UnresolvedPartitionSpec(visitNonOptionalPartitionSpec(ctx.from)),
       UnresolvedPartitionSpec(visitNonOptionalPartitionSpec(ctx.to)))
   }
@@ -4793,8 +4789,7 @@ class AstBuilder extends DataTypeAstBuilder with SQLConfHelper with Logging {
     DropPartitions(
       createUnresolvedTable(
         ctx.identifierReference,
-        "ALTER TABLE ... DROP PARTITION ...",
-        true),
+        "ALTER TABLE ... DROP PARTITION ..."),
       partSpecs.toSeq,
       ifExists = ctx.EXISTS != null,
       purge = ctx.PURGE != null)