diff --git a/common/utils/src/main/resources/error/error-conditions.json b/common/utils/src/main/resources/error/error-conditions.json index 2b7c146fcf997..afe983854839b 100644 --- a/common/utils/src/main/resources/error/error-conditions.json +++ b/common/utils/src/main/resources/error/error-conditions.json @@ -9545,6 +9545,12 @@ ], "sqlState" : "0A000" }, + "UNSUPPORTED_VIEW_CHANGE" : { + "message" : [ + "Unsupported view change: " + ], + "sqlState" : "0A000" + }, "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/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewCatalog.java b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewCatalog.java index 12782ad7f1314..923da349ae297 100644 --- a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewCatalog.java +++ b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewCatalog.java @@ -16,6 +16,9 @@ */ package org.apache.spark.sql.connector.catalog; +import java.util.HashMap; +import java.util.Map; + import org.apache.spark.annotation.Evolving; import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException; import org.apache.spark.sql.catalyst.analysis.NoSuchViewException; @@ -110,6 +113,52 @@ View createView(Identifier ident, View info) */ View replaceView(Identifier ident, View info) throws NoSuchViewException; + /** + * Apply a set of {@link ViewChange changes} to a view in the catalog. + *

+ * Implementations may reject the requested changes. If any change is rejected, none of the + * changes should be applied to the view. The requested changes must be applied in order. + *

+ * The default implementation loads the latest view metadata, applies the changes, and calls + * {@link #replaceView}. Catalogs should override this method if they can apply the changes in a + * single atomic operation. + * + * @param ident a view identifier + * @param changes changes to apply to the view + * @return updated metadata for the view + * @throws NoSuchViewException if the view does not exist + * @throws IllegalArgumentException if any change is rejected by the implementation + * + * @since 5.0.0 + */ + default View alterView(Identifier ident, ViewChange... changes) throws NoSuchViewException { + invalidateView(ident); + View current = loadView(ident); + Map properties = new HashMap<>(current.properties()); + for (ViewChange change : changes) { + if (change instanceof ViewChange.SetProperty set) { + properties.put(set.property(), set.value()); + } else if (change instanceof ViewChange.RemoveProperty remove) { + properties.remove(remove.property()); + } else { + throw new IllegalArgumentException("Unsupported view change: " + change); + } + } + + View updated = new View.Builder() + .withColumns(current.columns()) + .withProperties(properties) + .withQueryText(current.queryText()) + .withCurrentCatalog(current.currentCatalog()) + .withCurrentNamespace(current.currentNamespace()) + .withSqlConfigs(current.sqlConfigs()) + .withSchemaMode(current.schemaMode()) + .withQueryColumnNames(current.queryColumnNames()) + .withViewDependencies(current.viewDependencies()) + .build(); + return replaceView(ident, updated); + } + /** * Create a view if one does not exist at {@code ident}, or atomically replace it if one does. *

diff --git a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewChange.java b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewChange.java new file mode 100644 index 0000000000000..847e84a4696c1 --- /dev/null +++ b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewChange.java @@ -0,0 +1,112 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.spark.sql.connector.catalog; + +import java.util.Objects; + +import org.apache.spark.annotation.Evolving; + +/** + * ViewChange subclasses represent requested changes to a view. These are passed to + * {@link ViewCatalog#alterView}. + * + * @since 5.0.0 + */ +@Evolving +public interface ViewChange { + + /** + * Create a ViewChange for setting a view property. + * + * @param property the property name + * @param value the new property value + * @return a ViewChange for setting the property + */ + static ViewChange setProperty(String property, String value) { + return new SetProperty(property, value); + } + + /** + * Create a ViewChange for removing a view property. + *

+ * If the property does not exist, the change will succeed. + * + * @param property the property name + * @return a ViewChange for removing the property + */ + static ViewChange removeProperty(String property) { + return new RemoveProperty(property); + } + + /** A ViewChange to set a view property. */ + final class SetProperty implements ViewChange { + private final String property; + private final String value; + + private SetProperty(String property, String value) { + this.property = property; + this.value = value; + } + + public String property() { return property; } + + public String value() { return value; } + + @Override + public String toString() { return "SET PROPERTY " + property + " = " + value; } + + @Override + public boolean equals(Object other) { + if (this == other) return true; + if (other == null || getClass() != other.getClass()) return false; + SetProperty that = (SetProperty) other; + return property.equals(that.property) && value.equals(that.value); + } + + @Override + public int hashCode() { return Objects.hash(property, value); } + } + + /** + * A ViewChange to remove a view property. + *

+ * If the property does not exist, the change should succeed. + */ + final class RemoveProperty implements ViewChange { + private final String property; + + private RemoveProperty(String property) { + this.property = property; + } + + public String property() { return property; } + + @Override + public String toString() { return "REMOVE PROPERTY " + property; } + + @Override + public boolean equals(Object other) { + if (this == other) return true; + if (other == null || getClass() != other.getClass()) return false; + RemoveProperty that = (RemoveProperty) other; + return property.equals(that.property); + } + + @Override + public int hashCode() { return Objects.hash(property); } + } +} diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala index 8a9e776938249..0029e9ad30ce9 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala @@ -577,11 +577,10 @@ private[sql] object CatalogV2Util { } /** - * Construct a [[View.Builder]] seeded from an existing view's metadata. Used by ALTER - * VIEW execs (SET / UNSET TBLPROPERTIES, ALTER VIEW ... WITH SCHEMA BINDING) -- override - * the one field that changes, then `build` to produce the replacement payload for - * [[ViewCatalog#replaceView]]. Every other field flows through unchanged so a metadata-only - * mutation does not perturb the view body. + * Construct a [[View.Builder]] seeded from an existing view's metadata. Used by the ALTER + * VIEW ... WITH SCHEMA BINDING exec -- override the one field that changes, then `build` to + * produce the replacement payload for [[ViewCatalog#replaceView]]. Every other field flows + * through unchanged so a metadata-only mutation does not perturb the view body. */ def viewInfoBuilderFrom(existing: View): View.Builder = { val builder = new View.Builder() diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala index e08e078548f60..b6d6bc9ea85a6 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala @@ -739,6 +739,13 @@ private[sql] object QueryExecutionErrors extends QueryErrorsBase with ExecutionE cause = e) } + def unsupportedViewChangeError(e: IllegalArgumentException): Throwable = { + new SparkException( + errorClass = "UNSUPPORTED_VIEW_CHANGE", + messageParameters = Map("message" -> e.getMessage), + cause = e) + } + def notADatasourceRDDPartitionError(split: Partition): Throwable = { new SparkException( errorClass = "_LEGACY_ERROR_TEMP_2046", diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryRelationCatalog.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryRelationCatalog.scala index d6f526b30ce09..7911f0499bf96 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryRelationCatalog.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryRelationCatalog.scala @@ -41,6 +41,8 @@ class InMemoryRelationCatalog extends RelationCatalog with SupportsNamespaces { new ConcurrentHashMap[(Seq[String], String), Relation]() private val namespaces = new ConcurrentHashMap[Seq[String], util.Map[String, String]]() + @volatile private var lastViewChanges: Seq[ViewChange] = Seq.empty + @volatile private var alterViewFailure: IllegalArgumentException = _ override def loadRelation(ident: Identifier): Relation = { val key = (ident.namespace().toSeq, ident.name()) @@ -124,6 +126,14 @@ class InMemoryRelationCatalog extends RelationCatalog with SupportsNamespaces { info } + override def alterView(ident: Identifier, changes: ViewChange*): View = { + lastViewChanges = changes + if (alterViewFailure != null) { + throw alterViewFailure + } + super.alterView(ident, changes: _*) + } + override def dropView(ident: Identifier): Boolean = { val key = (ident.namespace().toSeq, ident.name()) val existing = store.get(key) @@ -234,6 +244,19 @@ class InMemoryRelationCatalog extends RelationCatalog with SupportsNamespaces { } } + /** Returns the changes from the most recent alterView call. */ + def getLastViewChanges: Seq[ViewChange] = lastViewChanges + + /** Configures a failure for alterView. */ + def failAlterViewWith(failure: IllegalArgumentException): Unit = { + alterViewFailure = failure + } + + /** Clears a configured alterView failure. */ + def clearAlterViewFailure(): Unit = { + alterViewFailure = null + } + // CatalogPlugin -------------------------------------------------------------------- private var catalogName: String = "" diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/ViewCatalogSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/ViewCatalogSuite.scala new file mode 100644 index 0000000000000..6fde02f489645 --- /dev/null +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/ViewCatalogSuite.scala @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.connector.catalog + +import scala.jdk.CollectionConverters._ + +import org.apache.spark.SparkFunSuite +import org.apache.spark.sql.types.IntegerType +import org.apache.spark.sql.util.CaseInsensitiveStringMap + +class ViewCatalogSuite extends SparkFunSuite { + + test("default alterView applies changes in order and preserves view metadata") { + val catalog = new InMemoryRelationCatalog + catalog.initialize("test", CaseInsensitiveStringMap.empty()) + val ident = Identifier.of(Array("ns"), "view") + val dependencies = DependencyList.of( + Array(Dependency.table(Array("source_catalog", "source_ns", "source")))) + val original = new View.Builder() + .withColumns(Array(Column.create("id", IntegerType))) + .withProperties(Map("first" -> "old", "second" -> "remove").asJava) + .withQueryText("SELECT id FROM source_catalog.source_ns.source") + .withCurrentCatalog("source_catalog") + .withCurrentNamespace(Array("source_ns")) + .withSqlConfigs(Map("spark.sql.ansi.enabled" -> "true").asJava) + .withSchemaMode("BINDING") + .withQueryColumnNames(Array("id")) + .withViewDependencies(dependencies) + .build() + catalog.createView(ident, original) + + val updated = catalog.alterView( + ident, + ViewChange.setProperty("first", "intermediate"), + ViewChange.removeProperty("first"), + ViewChange.setProperty("first", "new"), + ViewChange.removeProperty("second")) + + assert(updated.properties.get("first") === "new") + assert(!updated.properties.containsKey("second")) + assert(updated.columns.sameElements(original.columns)) + assert(updated.queryText === original.queryText) + assert(updated.currentCatalog === original.currentCatalog) + assert(updated.currentNamespace.sameElements(original.currentNamespace)) + assert(updated.sqlConfigs === original.sqlConfigs) + assert(updated.schemaMode === original.schemaMode) + assert(updated.queryColumnNames.sameElements(original.queryColumnNames)) + assert(updated.viewDependencies === original.viewDependencies) + } + + test("default alterView invalidates cached metadata before loading the current view") { + var invalidations = 0 + val catalog = new InMemoryRelationCatalog { + private var cachedView: View = _ + + override def loadView(ident: Identifier): View = { + if (cachedView == null) { + cachedView = super.loadView(ident) + } + cachedView + } + + override def invalidateView(ident: Identifier): Unit = { + invalidations += 1 + cachedView = null + } + } + catalog.initialize("test", CaseInsensitiveStringMap.empty()) + val ident = Identifier.of(Array("ns"), "view") + val original = new View.Builder() + .withColumns(Array(Column.create("id", IntegerType))) + .withProperties(Map("original" -> "value").asJava) + .withQueryText("SELECT 1 AS id") + .build() + catalog.createView(ident, original) + catalog.loadView(ident) + + val concurrent = CatalogV2Util.viewInfoBuilderFrom(original) + .withProperties(Map("concurrent" -> "value").asJava) + .withQueryText("SELECT 2 AS id") + .build() + catalog.replaceView(ident, concurrent) + + val updated = catalog.alterView(ident, ViewChange.setProperty("new", "value")) + + assert(invalidations === 1) + assert(updated.queryText === concurrent.queryText) + assert(updated.properties.asScala === Map("concurrent" -> "value", "new" -> "value")) + } +} diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/AlterV2ViewExec.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/AlterV2ViewExec.scala index 3afed35d894bc..c1a5412ccf9e5 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/AlterV2ViewExec.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/AlterV2ViewExec.scala @@ -19,12 +19,14 @@ package org.apache.spark.sql.execution.datasources.v2 import scala.jdk.CollectionConverters._ +import org.apache.spark.SparkThrowable import org.apache.spark.sql.catalyst.InternalRow import org.apache.spark.sql.catalyst.analysis.{ResolvedIdentifier, SchemaEvolution, ViewSchemaMode} import org.apache.spark.sql.catalyst.catalog.CatalogTable import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan -import org.apache.spark.sql.connector.catalog.{CatalogV2Util, Identifier, TableCatalog, View, ViewCatalog} +import org.apache.spark.sql.connector.catalog.{CatalogV2Util, Identifier, TableCatalog, View, ViewCatalog, ViewChange} import org.apache.spark.sql.connector.catalog.CatalogV2Implicits.{IdentifierHelper, MultipartIdentifierHelper} +import org.apache.spark.sql.errors.QueryExecutionErrors import org.apache.spark.sql.execution.command.CommandUtils /** @@ -89,53 +91,54 @@ case class AlterV2ViewExec( } /** - * Physical plan node for ALTER VIEW ... SET TBLPROPERTIES on a v2 [[ViewCatalog]]. Merges the - * user-supplied properties on top of the analysis-time view properties and dispatches to - * [[ViewCatalog#replaceView]] -- views carry no data, so a single atomic-swap call is sufficient. + * Physical plan node for ALTER VIEW ... SET TBLPROPERTIES on a v2 [[ViewCatalog]]. Dispatches the + * user-supplied properties as one batch to [[ViewCatalog#alterView]]. */ case class AlterV2ViewSetPropertiesExec( catalog: ViewCatalog, identifier: Identifier, - existingView: View, properties: Map[String, String]) extends LeafV2CommandExec { override def output: Seq[org.apache.spark.sql.catalyst.expressions.Attribute] = Seq.empty override protected def run(): Seq[InternalRow] = { - val merged = existingView.properties.asScala.toMap ++ properties - val info = CatalogV2Util.viewInfoBuilderFrom(existingView) - .withProperties(merged.asJava) - .build() // Match v1 `AlterTableSetPropertiesCommand`'s `invalidateCachedTable` so cached query // plans referencing the view drop their stale entries. CommandUtils.uncacheTableOrView(session, ResolvedIdentifier(catalog, identifier)) - catalog.replaceView(identifier, info) + val changes = properties.map { case (key, value) => ViewChange.setProperty(key, value) } + try { + catalog.alterView(identifier, changes.toSeq: _*) + } catch { + case e: IllegalArgumentException if !e.isInstanceOf[SparkThrowable] => + throw QueryExecutionErrors.unsupportedViewChangeError(e) + } Seq.empty } } /** - * Physical plan node for ALTER VIEW ... UNSET TBLPROPERTIES on a v2 [[ViewCatalog]]. Drops the - * listed property keys from the analysis-time view properties and dispatches to - * [[ViewCatalog#replaceView]]. Missing keys are silently dropped, matching v1 + * Physical plan node for ALTER VIEW ... UNSET TBLPROPERTIES on a v2 [[ViewCatalog]]. Dispatches + * the property keys as one batch to [[ViewCatalog#alterView]]. Missing keys are silently dropped, + * matching v1 * `AlterTableUnsetPropertiesCommand` for views (`ifExists` is unused on the view path -- the * v1 view command never errors on missing keys). */ case class AlterV2ViewUnsetPropertiesExec( catalog: ViewCatalog, identifier: Identifier, - existingView: View, propertyKeys: Seq[String]) extends LeafV2CommandExec { override def output: Seq[org.apache.spark.sql.catalyst.expressions.Attribute] = Seq.empty override protected def run(): Seq[InternalRow] = { - val remaining = existingView.properties.asScala.toMap -- propertyKeys - val info = CatalogV2Util.viewInfoBuilderFrom(existingView) - .withProperties(remaining.asJava) - .build() CommandUtils.uncacheTableOrView(session, ResolvedIdentifier(catalog, identifier)) - catalog.replaceView(identifier, info) + val changes = propertyKeys.map(ViewChange.removeProperty) + try { + catalog.alterView(identifier, changes: _*) + } catch { + case e: IllegalArgumentException if !e.isInstanceOf[SparkThrowable] => + throw QueryExecutionErrors.unsupportedViewChangeError(e) + } Seq.empty } } diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala index 8805bfe75298f..61d7c264636de 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala @@ -381,16 +381,16 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat // View DDL / inspection on a non-session v2 catalog that the v1 rewrite in // `ResolveSessionCatalog` can't handle (its `ResolvedViewIdentifier` matcher is gated on - // `isSessionCatalog`). Routed to dedicated v2 execs that read the typed `View` - // resolved at analysis time directly from `ResolvedPersistentView.info` -- no re-loading - // at exec time. - case SetViewProperties(rpv @ ResolvedPersistentView(catalog, ident, _), props) => + // `isSessionCatalog`). Routed to dedicated v2 execs. Metadata replacements read the typed + // `View` resolved at analysis time; property updates are delegated to the catalog as partial + // changes. + case SetViewProperties(ResolvedPersistentView(catalog, ident, _), props) => AlterV2ViewSetPropertiesExec( - catalog.asInstanceOf[ViewCatalog], ident, rpv.info, props) :: Nil + catalog.asInstanceOf[ViewCatalog], ident, props) :: Nil - case UnsetViewProperties(rpv @ ResolvedPersistentView(catalog, ident, _), keys, _) => + case UnsetViewProperties(ResolvedPersistentView(catalog, ident, _), keys, _) => AlterV2ViewUnsetPropertiesExec( - catalog.asInstanceOf[ViewCatalog], ident, rpv.info, keys) :: Nil + catalog.asInstanceOf[ViewCatalog], ident, keys) :: Nil case AlterViewSchemaBinding(rpv @ ResolvedPersistentView(catalog, ident, _), schemaMode) => AlterV2ViewSchemaBindingExec( diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterViewSetTblPropertiesSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterViewSetTblPropertiesSuite.scala index 46499b6b49693..2606caebd2840 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterViewSetTblPropertiesSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterViewSetTblPropertiesSuite.scala @@ -17,6 +17,8 @@ package org.apache.spark.sql.execution.command.v2 +import org.apache.spark.{SparkException, SparkIllegalArgumentException} +import org.apache.spark.sql.connector.catalog.ViewChange import org.apache.spark.sql.execution.command /** @@ -32,5 +34,40 @@ class AlterViewSetTblPropertiesSuite sql(s"ALTER VIEW $view SET TBLPROPERTIES ('k' = 'v')") val stored = viewCatalog.getStoredView(Array(namespace), "v2_set_view_info") assert(stored.properties.get("k") == "v") + assert(viewCatalog.getLastViewChanges === Seq(ViewChange.setProperty("k", "v"))) + } + + test("V2: catalog IllegalArgumentException is converted to a structured error") { + val view = s"$catalog.$namespace.v2_set_view_rejected" + createView(view) + viewCatalog.failAlterViewWith(new IllegalArgumentException("set rejected")) + try { + checkError( + exception = intercept[SparkException] { + sql(s"ALTER VIEW $view SET TBLPROPERTIES ('k' = 'v')") + }, + condition = "UNSUPPORTED_VIEW_CHANGE", + parameters = Map("message" -> "set rejected")) + } finally { + viewCatalog.clearAlterViewFailure() + } + } + + test("V2: catalog SparkThrowable is preserved") { + val view = s"$catalog.$namespace.v2_set_view_spark_error" + createView(view) + viewCatalog.failAlterViewWith(new SparkIllegalArgumentException( + errorClass = "UNSUPPORTED_SAVE_MODE.EXISTENT_PATH", + messageParameters = Map("saveMode" -> "TEST"))) + try { + checkError( + exception = intercept[SparkIllegalArgumentException] { + sql(s"ALTER VIEW $view SET TBLPROPERTIES ('k' = 'v')") + }, + condition = "UNSUPPORTED_SAVE_MODE.EXISTENT_PATH", + parameters = Map("saveMode" -> "TEST")) + } finally { + viewCatalog.clearAlterViewFailure() + } } } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterViewUnsetTblPropertiesSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterViewUnsetTblPropertiesSuite.scala index 52871b1a04128..1cf7083326eb8 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterViewUnsetTblPropertiesSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterViewUnsetTblPropertiesSuite.scala @@ -17,6 +17,8 @@ package org.apache.spark.sql.execution.command.v2 +import org.apache.spark.SparkException +import org.apache.spark.sql.connector.catalog.ViewChange import org.apache.spark.sql.execution.command class AlterViewUnsetTblPropertiesSuite @@ -28,5 +30,22 @@ class AlterViewUnsetTblPropertiesSuite sql(s"ALTER VIEW $view UNSET TBLPROPERTIES ('k')") val stored = viewCatalog.getStoredView(Array(namespace), "v2_unset_view_info") assert(!stored.properties.containsKey("k")) + assert(viewCatalog.getLastViewChanges === Seq(ViewChange.removeProperty("k"))) + } + + test("V2: catalog IllegalArgumentException is converted to a structured error") { + val view = s"$catalog.$namespace.v2_unset_view_rejected" + createViewWithProps(view, "k" -> "v") + viewCatalog.failAlterViewWith(new IllegalArgumentException("unset rejected")) + try { + checkError( + exception = intercept[SparkException] { + sql(s"ALTER VIEW $view UNSET TBLPROPERTIES ('k')") + }, + condition = "UNSUPPORTED_VIEW_CHANGE", + parameters = Map("message" -> "unset rejected")) + } finally { + viewCatalog.clearAlterViewFailure() + } } }