Skip to content

Commit

Permalink
[SPARK-30605][SQL] move defaultNamespace from SupportsNamespace to Ca…
Browse files Browse the repository at this point in the history
…talogPlugin

### What changes were proposed in this pull request?

Move the `defaultNamespace` method from the interface `SupportsNamespace` to `CatalogPlugin`.

### Why are the changes needed?

While I'm implementing JDBC V2, I realize that the default namespace is very an important information. Even if you don't want to implement namespace manipulation functionalities like CREATE/DROP/ALTER namespace, you still need to report the default namespace.

The default namespace is not about functionality but a matter of correctness. If you don't know the default namespace of a catalog, it's wrong to assume it's `[]`. You may get table not found exception if you do so.

I think it's more reasonable to put the `defaultNamespace` method in the base class `CatalogPlugin`. It returns `[]` by default so won't bother implementation if they really don't have namespace concept.

### Does this PR introduce any user-facing change?

yes, but for an unreleased API.

### How was this patch tested?

existing tests

Closes #27319 from cloud-fan/ns.

Authored-by: Wenchen Fan <wenchen@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
  • Loading branch information
cloud-fan committed Jan 23, 2020
1 parent ffd435b commit dbed4c7
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 48 deletions.
Expand Up @@ -58,4 +58,18 @@ public interface CatalogPlugin {
* called to pass the catalog's name.
*/
String name();

/**
* Return a default namespace for the catalog.
* <p>
* When this catalog is set as the current catalog, the namespace returned by this method will be
* set as the current namespace.
* <p>
* The namespace returned by this method is not required to exist.
*
* @return a multi-part namespace
*/
default String[] defaultNamespace() {
return new String[0];
}
}
Expand Up @@ -51,6 +51,11 @@ public String name() {
@Override
public final void initialize(String name, CaseInsensitiveStringMap options) {}

@Override
public String[] defaultNamespace() {
return delegate.defaultNamespace();
}

@Override
public Identifier[] listTables(String[] namespace) throws NoSuchNamespaceException {
return asTableCatalog().listTables(namespace);
Expand Down Expand Up @@ -99,11 +104,6 @@ public void renameTable(
asTableCatalog().renameTable(oldIdent, newIdent);
}

@Override
public String[] defaultNamespace() {
return asNamespaceCatalog().defaultNamespace();
}

@Override
public String[][] listNamespaces() throws NoSuchNamespaceException {
return asNamespaceCatalog().listNamespaces();
Expand Down
Expand Up @@ -69,20 +69,6 @@ public interface SupportsNamespaces extends CatalogPlugin {
*/
List<String> RESERVED_PROPERTIES = Arrays.asList(PROP_COMMENT, PROP_LOCATION, PROP_OWNER);

/**
* Return a default namespace for the catalog.
* <p>
* When this catalog is set as the current catalog, the namespace returned by this method will be
* set as the current namespace.
* <p>
* The namespace returned by this method is not required to exist.
*
* @return a multi-part namespace
*/
default String[] defaultNamespace() {
return new String[0];
}

/**
* List top-level namespaces from the catalog.
* <p>
Expand Down
Expand Up @@ -93,19 +93,14 @@ class CatalogManager(
}.getOrElse(defaultSessionCatalog)
}

private def getDefaultNamespace(c: CatalogPlugin) = c match {
case c: SupportsNamespaces => c.defaultNamespace()
case _ => Array.empty[String]
}

private var _currentNamespace: Option[Array[String]] = None

def currentNamespace: Array[String] = synchronized {
_currentNamespace.getOrElse {
if (currentCatalog.name() == SESSION_CATALOG_NAME) {
Array(v1SessionCatalog.getCurrentDatabase)
} else {
getDefaultNamespace(currentCatalog)
currentCatalog.defaultNamespace()
}
}
}
Expand Down
Expand Up @@ -18,7 +18,6 @@
package org.apache.spark.sql.connector.catalog

import java.net.URI
import java.util

import org.apache.spark.SparkFunSuite
import org.apache.spark.sql.catalyst.analysis.{EmptyFunctionRegistry, FakeV2SessionCatalog, NoSuchNamespaceException}
Expand Down Expand Up @@ -112,31 +111,11 @@ class CatalogManagerSuite extends SparkFunSuite {
}
}

class DummyCatalog extends SupportsNamespaces {
override def defaultNamespace(): Array[String] = Array("a", "b")

override def listNamespaces(): Array[Array[String]] = {
throw new UnsupportedOperationException
}
override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
throw new UnsupportedOperationException
}
override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
throw new UnsupportedOperationException
}
override def createNamespace(
namespace: Array[String], metadata: util.Map[String, String]): Unit = {
throw new UnsupportedOperationException
}
override def alterNamespace(namespace: Array[String], changes: NamespaceChange*): Unit = {
throw new UnsupportedOperationException
}
override def dropNamespace(namespace: Array[String]): Boolean = {
throw new UnsupportedOperationException
}
class DummyCatalog extends CatalogPlugin {
override def initialize(name: String, options: CaseInsensitiveStringMap): Unit = {
_name = name
}
private var _name: String = null
override def name(): String = _name
override def defaultNamespace(): Array[String] = Array("a", "b")
}

0 comments on commit dbed4c7

Please sign in to comment.