You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/01/19 06:49:37 UTC

[GitHub] [spark] dchvn opened a new pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

dchvn opened a new pull request #35246:
URL: https://github.com/apache/spark/pull/35246


   ### What changes were proposed in this pull request?
   This PR adds a new API `dropNamespace(String[] ns, boolean cascade)` to replace the existing one: Add a boolean parameter `cascade` that supports deleting all the Namespaces and Tables under the namespace.
   
   Also include changing the implementations and tests that are relevant to this API.
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   
   
   ### Why are the changes needed?
   According to [#cmt](https://github.com/apache/spark/pull/35202#discussion_r784463563), the current `dropNamespace` API doesn't support cascade mode. So this PR replaces that to support cascading.
   If cascade is set True, delete all namespaces and tables under the namespace.
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   ### How was this patch tested?
   Existing test.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792595479



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review 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.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r787533283



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -146,5 +147,5 @@ void alterNamespace(
    * @throws NoSuchNamespaceException If the namespace does not exist (optional)
    * @throws UnsupportedOperationException If drop is not a supported operation

Review comment:
       let's add parameter doc for `cascade`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan closed pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #35246:
URL: https://github.com/apache/spark/pull/35246


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dchvn commented on pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
dchvn commented on pull request #35246:
URL: https://github.com/apache/spark/pull/35246#issuecomment-1016136934


   cc @cloud-fan @imback82 @huaxingao, please take a look if you have time. Thanks!!!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792675110



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review comment:
       Can we at least keep the old signature, deprecate it and throw an exception? Also this PR changed the behaviour.
   
   I know that DSv2 has been less strict on this but probably we should start taking care about this given that the API has been almost 2 years since Spark 3.0.0.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dchvn commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
dchvn commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792396103



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review comment:
       Do you mean we should keep the old API `dropNamespace(String[] namespace)`, and add a new API `dropNamespace( String[] namespace, boolean cascade)`? @HyukjinKwon 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r787536954



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -146,5 +147,5 @@ void alterNamespace(
    * @throws NoSuchNamespaceException If the namespace does not exist (optional)
    * @throws UnsupportedOperationException If drop is not a supported operation

Review comment:
       We also need to create this new exception class `NonEmptyNamespace`, and caller side of `dropNamespace` should try-catch it and re-throw `QueryCompilationErrors.cannotDropNonemptyNamespaceError`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dchvn commented on pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
dchvn commented on pull request #35246:
URL: https://github.com/apache/spark/pull/35246#issuecomment-1018249472


   Thank you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792289685



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review comment:
       Wait .. this API was added in 3.0.0 at SPARK-27661. Shouldn't we have overwritten version to keep the binary compatibility?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] beliefer commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r789265731



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -278,7 +278,9 @@ class JDBCTableCatalog extends TableCatalog with SupportsNamespaces with Logging
     }
   }
 
-  override def dropNamespace(namespace: Array[String]): Boolean = namespace match {
+  override def dropNamespace(
+      namespace: Array[String],
+      cascade: Boolean): Boolean = namespace match {

Review comment:
       `DROP DATABASE` fails if exists table in database.
   ` DROP DATABASE CASCADE` drops all the table in database and database itself.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] beliefer commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r789265731



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -278,7 +278,9 @@ class JDBCTableCatalog extends TableCatalog with SupportsNamespaces with Logging
     }
   }
 
-  override def dropNamespace(namespace: Array[String]): Boolean = namespace match {
+  override def dropNamespace(
+      namespace: Array[String],
+      cascade: Boolean): Boolean = namespace match {

Review comment:
       `DROP DATABASE` fails if exists table in database.
   ` DROP DATABASE CASCADE` drop all the table in database and database itself.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792620857



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review comment:
       Can we implement default method of `dropNamespace(String[] namespace)` that calls `dropNamespace(namespace, cascade = false)` or `dropNamespace(namespace, cascade = true)` with deprecating and pointing the alternative? It already used to work with cascading before, and that's what we documented. This API was added from Spark 3.0.0, and I don't think we should break this binary compatibility unless we must.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792403002



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review 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.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dchvn commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
dchvn commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792445346



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review comment:
       Should I create a PR for reversing this API `dropNamespace` and adding a new one? WDYT? @cloud-fan @HyukjinKwon 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] beliefer commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r789496803



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -278,7 +278,9 @@ class JDBCTableCatalog extends TableCatalog with SupportsNamespaces with Logging
     }
   }
 
-  override def dropNamespace(namespace: Array[String]): Boolean = namespace match {
+  override def dropNamespace(
+      namespace: Array[String],
+      cascade: Boolean): Boolean = namespace match {

Review comment:
       @cloud-fan Thank you for the ping. I will try to fix it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r789412358



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -278,7 +278,9 @@ class JDBCTableCatalog extends TableCatalog with SupportsNamespaces with Logging
     }
   }
 
-  override def dropNamespace(namespace: Array[String]): Boolean = namespace match {
+  override def dropNamespace(
+      namespace: Array[String],
+      cascade: Boolean): Boolean = namespace match {

Review comment:
       Can you create a followup PR to fix JDBC source? We need to respect the cascade parameter. thanks! 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792478532



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review comment:
       Can we keep the binary compatibility but change the old behavior? 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792623987



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review 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.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792450987



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review comment:
       We changed the implementation of `DropNamespaceExec` to not call `listTables` at the beginning, and the catalog implementation must deal with the `cascade` parameter properly. It's better to break binary compatibility here, instead of silently calling the old `dropNamespace` implementation which may drop all the tables in the namespace.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r788565999



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,21 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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 a boolean flag that deletes all namespaces and tables under the namespace

Review comment:
       ```suggestion
      * @param cascade When true, deletes all objects under the namespace.
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r788568402



##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryTableCatalog.scala
##########
@@ -213,10 +213,16 @@ class InMemoryTableCatalog extends BasicInMemoryTableCatalog with SupportsNamesp
     namespaces.put(namespace.toList, CatalogV2Util.applyNamespaceChanges(metadata, changes))
   }
 
-  override def dropNamespace(namespace: Array[String]): Boolean = {
-    listNamespaces(namespace).foreach(dropNamespace)
+  override def dropNamespace(namespace: Array[String], cascade: Boolean): Boolean = {
+    listNamespaces(namespace).foreach(namespace => dropNamespace(namespace, cascade))

Review comment:
       I think we should only do this if `cascade` is true




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r788574434



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -278,7 +278,9 @@ class JDBCTableCatalog extends TableCatalog with SupportsNamespaces with Logging
     }
   }
 
-  override def dropNamespace(namespace: Array[String]): Boolean = namespace match {
+  override def dropNamespace(
+      namespace: Array[String],
+      cascade: Boolean): Boolean = namespace match {

Review comment:
       @beliefer do you know how JDBC support DROP DATABASE CASCADE?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792639178



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review comment:
       > Can we implement default method of dropNamespace(String[] namespace)
   
   This doesn't help,  because the new `dropNamespace(String[] namespace, boolean cascade)` is not implemented and will break all the existing implementations.
   
   A common way to keep backward compatibility is to add default implementation for the new API. e.g.
   ```
   boolean dropNamespace(String[] namespace, boolean cascade) {
     if (cascade) dropNamespace(namespace) else throw ...
   }
   ```
   However, `DROP DATABASE` is much more commonly used than `DROP DATABASE ... CASCADE`. So this doesn't help either.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r787535822



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -146,5 +147,5 @@ void alterNamespace(
    * @throws NoSuchNamespaceException If the namespace does not exist (optional)
    * @throws UnsupportedOperationException If drop is not a supported operation

Review comment:
       let's mention that implementation should throw `NonEmptyNamespace` exception if the namespace is not empty and `cascade = false`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #35246:
URL: https://github.com/apache/spark/pull/35246#issuecomment-1018248437


   thanks, merging to master!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r788567043



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,21 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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 a boolean flag that deletes all namespaces and tables under the namespace
+   * if it is set true
    * @return true if the namespace was dropped
    * @throws NoSuchNamespaceException If the namespace does not exist (optional)
+   * @throws NonEmptyNamespaceException If the namespace is non-empty

Review comment:
       ```suggestion
      * @throws NonEmptyNamespaceException If the namespace is non-empty and cascade is false.
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r788565999



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,21 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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 a boolean flag that deletes all namespaces and tables under the namespace

Review comment:
       ```suggestion
      * @param cascade a boolean flag that deletes all objects under the namespace
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792675110



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review comment:
       Can we at least keep the old signature, deprecate it and throw an exception? Also this PR changed the behaviour.
   
   I know that DSv2 has been less strict on this but probably we should start taking care about this given that the API has been almost 2 years since Spark 3.0.0.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r792289685



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java
##########
@@ -136,15 +137,20 @@ void alterNamespace(
       NamespaceChange... changes) throws NoSuchNamespaceException;
 
   /**
-   * Drop a namespace from the catalog, recursively dropping all objects within the namespace.
+   * 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) throws NoSuchNamespaceException;
+  boolean dropNamespace(
+      String[] namespace,
+      boolean cascade) throws NoSuchNamespaceException, NonEmptyNamespaceException;

Review comment:
       Wait .. this API was added in 3.0.0 at SPARK-27661. Shouldn't we have overwritten version to keep the backward compatibility?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] beliefer commented on a change in pull request #35246: [SPARK-37929][SQL] Support cascade mode for `dropNamespace` API

Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #35246:
URL: https://github.com/apache/spark/pull/35246#discussion_r789496803



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -278,7 +278,9 @@ class JDBCTableCatalog extends TableCatalog with SupportsNamespaces with Logging
     }
   }
 
-  override def dropNamespace(namespace: Array[String]): Boolean = namespace match {
+  override def dropNamespace(
+      namespace: Array[String],
+      cascade: Boolean): Boolean = namespace match {

Review comment:
       @cloud-fan Thank you for the ping.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org