[SPARK-37929][SQL] Support cascade mode for dropNamespace API
#35246
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
| import org.apache.spark.annotation.Evolving; | ||
| import org.apache.spark.sql.catalyst.analysis.NamespaceAlreadyExistsException; | ||
| import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException; | ||
| import org.apache.spark.sql.catalyst.analysis.NonEmptyNamespaceException; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
|
|
@@ -136,15 +137,20 @@ void alterNamespace( | |
| NamespaceChange... changes) throws NoSuchNamespaceException; | ||
|
|
||
| /** | ||
| * Drop a namespace from the catalog with cascade mode, recursively dropping all objects | ||
| * within the namespace if cascade is true. | ||
| * <p> | ||
| * If the catalog implementation does not support this operation, it may throw | ||
| * {@link UnsupportedOperationException}. | ||
| * | ||
| * @param namespace a multi-part namespace | ||
| * @param cascade When true, deletes all objects under the namespace | ||
| * @return true if the namespace was dropped | ||
| * @throws NoSuchNamespaceException If the namespace does not exist (optional) | ||
| * @throws NonEmptyNamespaceException If the namespace is non-empty and cascade is false | ||
| * @throws UnsupportedOperationException If drop is not a supported operation | ||
| */ | ||
| boolean dropNamespace( | ||
| String[] namespace, | ||
| boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException; | ||
|
Member
There was a problem hiding this comment. Wait .. this API was added in 3.0.0 at SPARK-27661. Shouldn't we have overwritten version to keep the binary compatibility?
Contributor
Author
There was a problem hiding this comment. Do you mean we should keep the old API
Member
There was a problem hiding this comment. Yes, otherwise the downstream datasources complied against Spark 3.2 would not work with Spark 3.3 with an exception like method does not exist. Mima plugin we use usually detects such binary compatibility but seems it didn't for some reasons.
Contributor
Author
There was a problem hiding this comment. Should I create a PR for reversing this API
Contributor
There was a problem hiding this comment. We changed the implementation of
Member
There was a problem hiding this comment. Can we keep the binary compatibility but change the old behavior?
Contributor
There was a problem hiding this comment. I don't think it's possible, unless we add versioning to the DS v2 APIs, so that we know which version the catalog implementation is.
Member
There was a problem hiding this comment. Can we implement default method of
Member
There was a problem hiding this comment. If we think that we shouldn't make this implemented, we should throw an exception with keeping the signature so we can show nicer error message instead of method not found.
Contributor
There was a problem hiding this comment.
This doesn't help, because the new A common way to keep backward compatibility is to add default implementation for the new API. e.g. However, |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| /* | ||
| * 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.catalyst.analysis | ||
|
|
||
| import org.apache.spark.sql.AnalysisException | ||
| import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ | ||
|
|
||
|
|
||
| /** | ||
| * Thrown by a catalog when an item already exists. The analyzer will rethrow the exception | ||
| * as an [[org.apache.spark.sql.AnalysisException]] with the correct position information. | ||
| */ | ||
| case class NonEmptyNamespaceException( | ||
| override val message: String, | ||
| override val cause: Option[Throwable] = None) | ||
| extends AnalysisException(message, cause = cause) { | ||
|
|
||
| def this(namespace: Array[String]) = { | ||
| this(s"Namespace '${namespace.quoted}' is non empty.") | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -278,7 +278,9 @@ class JDBCTableCatalog extends TableCatalog with SupportsNamespaces with Logging | |
| } | ||
| } | ||
|
|
||
| override def dropNamespace( | ||
| namespace: Array[String], | ||
| cascade: Boolean): Boolean = namespace match { | ||
|
Contributor
There was a problem hiding this comment. @beliefer do you know how JDBC support DROP DATABASE CASCADE?
Contributor
There was a problem hiding this comment.
Contributor
There was a problem hiding this comment. Can you create a followup PR to fix JDBC source? We need to respect the cascade parameter. thanks!
Contributor
There was a problem hiding this comment. @cloud-fan Thank you for the ping. I will try to fix it. |
||
| case Array(db) if namespaceExists(namespace) => | ||
| if (listTables(Array(db)).nonEmpty) { | ||
| throw QueryExecutionErrors.namespaceNotEmptyError(namespace) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.