yadavay-amzn commented on code in PR #56179:
URL: https://github.com/apache/spark/pull/56179#discussion_r3432224898
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryCatalog.scala:
##########
@@ -27,6 +27,16 @@ import
org.apache.spark.sql.connector.catalog.functions.UnboundFunction
import org.apache.spark.sql.connector.catalog.procedures.UnboundProcedure
class InMemoryCatalog extends InMemoryTableCatalog with FunctionCatalog with
ProcedureCatalog {
+ override def dropNamespace(namespace: Array[String], cascade: Boolean):
Boolean = {
+ if (cascade) {
+ // SPARK-55982: Remove functions in this namespace before dropping.
+ // Only needed for cascade=true because without cascade,
super.dropNamespace
+ // will fail if the namespace still contains objects (tables/functions).
+ listFunctions(namespace).foreach(ident => functions.remove(ident))
+ }
+ super.dropNamespace(namespace, cascade)
Review Comment:
`InMemoryCatalog` also holds a sibling `procedures` map (populated via
`createProcedure`), which has the same orphaning behavior on
`dropNamespace(cascade=true)` — procedures in the dropped namespace stay in the
map. Since `ProcedureCatalog` shares the same "objects under a namespace"
contract, it might be worth cleaning both here (e.g. an analogous
`listProcedures(namespace).foreach(...)`), or adding a short note that
procedures are intentionally out of scope for this change.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryCatalog.scala:
##########
@@ -27,6 +27,16 @@ import
org.apache.spark.sql.connector.catalog.functions.UnboundFunction
import org.apache.spark.sql.connector.catalog.procedures.UnboundProcedure
class InMemoryCatalog extends InMemoryTableCatalog with FunctionCatalog with
ProcedureCatalog {
+ override def dropNamespace(namespace: Array[String], cascade: Boolean):
Boolean = {
+ if (cascade) {
+ // SPARK-55982: Remove functions in this namespace before dropping.
+ // Only needed for cascade=true because without cascade,
super.dropNamespace
Review Comment:
Small accuracy nit on the comment: `super.dropNamespace` (in
`InMemoryTableCatalog`) only checks `listTables(namespace).nonEmpty ||
listNamespaces(namespace).nonEmpty` before throwing
`NonEmptyNamespaceException`, so it doesn't actually consider functions — a
namespace containing only functions would drop successfully even without
cascade. Might read more precisely as "...will fail if the namespace still
contains tables or child namespaces."
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
