Skip to content

[SPARK-30420][SQL][FOLLOWUP] Remove statement logical plans for namespace commands #27125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
imback82 wants to merge 2 commits into from
Closed

[SPARK-30420][SQL][FOLLOWUP] Remove statement logical plans for namespace commands #27125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3df0b6
initial commit
imback82 Jan 8, 2020
f65672f
remove unused code
imback82 Jan 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ class Analyzer(
case class ResolveNamespace(catalogManager: CatalogManager)
extends Rule[LogicalPlan] with LookupCatalog {
def apply(plan: LogicalPlan): LogicalPlan = plan resolveOperators {
case s @ ShowTablesStatement(UnresolvedNamespace(Seq()), _) =>
case s @ ShowTables(UnresolvedNamespace(Seq()), _) =>
s.copy(namespace =
ResolvedNamespace(currentCatalog.asNamespaceCatalog, catalogManager.currentNamespace))
case UnresolvedNamespace(Seq()) =>
Expand Down
30 changes: 0 additions & 30 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveCatalogs.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,6 @@ class ResolveCatalogs(val catalogManager: CatalogManager)
s"Can not specify catalog `${catalog.name}` for view ${tbl.quoted} " +
s"because view support in catalog has not been implemented yet")

case AlterNamespaceSetPropertiesStatement(
NonSessionCatalogAndNamespace(catalog, ns), properties) =>
AlterNamespaceSetProperties(catalog, ns, properties)

case AlterNamespaceSetLocationStatement(
NonSessionCatalogAndNamespace(catalog, ns), location) =>
AlterNamespaceSetProperties(catalog, ns,
Map(SupportsNamespaces.PROP_LOCATION -> location))

case RenameTableStatement(NonSessionCatalogAndTable(catalog, oldName), newNameParts, isView) =>
if (isView) {
throw new AnalysisException("Renaming view is not supported in v2 catalogs.")
Expand Down Expand Up @@ -194,18 +185,6 @@ class ResolveCatalogs(val catalogManager: CatalogManager)
if !isSessionCatalog(catalog) =>
CreateNamespace(catalog.asNamespaceCatalog, ns, c.ifNotExists, c.properties)

case DropNamespaceStatement(NonSessionCatalogAndNamespace(catalog, ns), ifExists, cascade) =>
DropNamespace(catalog, ns, ifExists, cascade)

case DescribeNamespaceStatement(NonSessionCatalogAndNamespace(catalog, ns), extended) =>
DescribeNamespace(catalog, ns, extended)

case ShowNamespacesStatement(NonSessionCatalogAndNamespace(catalog, ns), pattern) =>
ShowNamespaces(catalog, ns, pattern)
Copy link
Contributor

@cloud-fan cloud-fan Jan 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We missed the fallback to the v1 ShowDatabasesCommand.

Copy link
Contributor Author

@imback82 imback82 Jan 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me check, thanks.

Copy link
Contributor Author

@imback82 imback82 Jan 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cloud-fan could you clarify on the fallback?

"show databases" and "show namespaces" were merged by #26006 and both are handled by v2 ShowNamespacesExec.

Copy link
Contributor

@cloud-fan cloud-fan Feb 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I misremember that. So we forgot to add a migration guide for the schema change... (databaseName -> namespace) Anyway it's too late now, let's catch schema changes in the following development work.


case ShowTablesStatement(NonSessionCatalogAndNamespace(catalog, ns), pattern) =>
ShowTables(catalog.asTableCatalog, ns, pattern)

case UseStatement(isNamespaceSet, nameParts) =>
if (isNamespaceSet) {
SetCatalogAndNamespace(catalogManager, None, Some(nameParts))
Expand All @@ -231,13 +210,4 @@ class ResolveCatalogs(val catalogManager: CatalogManager)
case _ => None
}
}

object NonSessionCatalogAndNamespace {
def unapply(resolved: ResolvedNamespace): Option[(SupportsNamespaces, Seq[String])] =
if (!isSessionCatalog(resolved.catalog)) {
Some(resolved.catalog -> resolved.namespace)
} else {
None
}
}
}
24 changes: 12 additions & 12 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2563,22 +2563,22 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
}

/**
* Create a [[DropNamespaceStatement]] command.
* Create a [[DropNamespace]] command.
*
* For example:
* {{{
* DROP (DATABASE|SCHEMA|NAMESPACE) [IF EXISTS] ns1.ns2 [RESTRICT|CASCADE];
* }}}
*/
override def visitDropNamespace(ctx: DropNamespaceContext): LogicalPlan = withOrigin(ctx) {
DropNamespaceStatement(
DropNamespace(
UnresolvedNamespace(visitMultipartIdentifier(ctx.multipartIdentifier)),
ctx.EXISTS != null,
ctx.CASCADE != null)
}

/**
* Create an [[AlterNamespaceSetPropertiesStatement]] logical plan.
* Create an [[AlterNamespaceSetProperties]] logical plan.
*
* For example:
* {{{
Expand All @@ -2588,14 +2588,14 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
*/
override def visitSetNamespaceProperties(ctx: SetNamespacePropertiesContext): LogicalPlan = {
withOrigin(ctx) {
AlterNamespaceSetPropertiesStatement(
AlterNamespaceSetProperties(
UnresolvedNamespace(visitMultipartIdentifier(ctx.multipartIdentifier)),
visitPropertyKeyValues(ctx.tablePropertyList))
}
}

/**
* Create an [[AlterNamespaceSetLocationStatement]] logical plan.
* Create an [[AlterNamespaceSetLocation]] logical plan.
*
* For example:
* {{{
Expand All @@ -2604,28 +2604,28 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
*/
override def visitSetNamespaceLocation(ctx: SetNamespaceLocationContext): LogicalPlan = {
withOrigin(ctx) {
AlterNamespaceSetLocationStatement(
AlterNamespaceSetLocation(
UnresolvedNamespace(visitMultipartIdentifier(ctx.multipartIdentifier)),
visitLocationSpec(ctx.locationSpec))
}
}

/**
* Create a [[ShowNamespacesStatement]] command.
* Create a [[ShowNamespaces]] command.
*/
override def visitShowNamespaces(ctx: ShowNamespacesContext): LogicalPlan = withOrigin(ctx) {
if (ctx.DATABASES != null && ctx.multipartIdentifier != null) {
throw new ParseException(s"FROM/IN operator is not allowed in SHOW DATABASES", ctx)
}

val multiPart = Option(ctx.multipartIdentifier).map(visitMultipartIdentifier)
ShowNamespacesStatement(
ShowNamespaces(
UnresolvedNamespace(multiPart.getOrElse(Seq.empty[String])),
Option(ctx.pattern).map(string))
}

/**
* Create a [[DescribeNamespaceStatement]].
* Create a [[DescribeNamespace]].
*
* For example:
* {{{
Expand All @@ -2634,7 +2634,7 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
*/
override def visitDescribeNamespace(ctx: DescribeNamespaceContext): LogicalPlan =
withOrigin(ctx) {
DescribeNamespaceStatement(
DescribeNamespace(
UnresolvedNamespace(visitMultipartIdentifier(ctx.multipartIdentifier())),
ctx.EXTENDED != null)
}
Expand Down Expand Up @@ -2802,11 +2802,11 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
}

/**
* Create a [[ShowTablesStatement]] command.
* Create a [[ShowTables]] command.
*/
override def visitShowTables(ctx: ShowTablesContext): LogicalPlan = withOrigin(ctx) {
val multiPart = Option(ctx.multipartIdentifier).map(visitMultipartIdentifier)
ShowTablesStatement(
ShowTables(
UnresolvedNamespace(multiPart.getOrElse(Seq.empty[String])),
Option(ctx.pattern).map(string))
}
Expand Down
55 changes: 0 additions & 55 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statements.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,15 +298,6 @@ case class DescribeTableStatement(
partitionSpec: TablePartitionSpec,
isExtended: Boolean) extends ParsedStatement

/**
* A DESCRIBE NAMESPACE statement, as parsed from SQL.
*/
case class DescribeNamespaceStatement(
namespace: LogicalPlan,
extended: Boolean) extends ParsedStatement {
override def children: Seq[LogicalPlan] = Seq(namespace)
}

/**
* A DESCRIBE TABLE tbl_name col_name statement, as parsed from SQL.
*/
Expand Down Expand Up @@ -345,15 +336,6 @@ case class InsertIntoStatement(
override def children: Seq[LogicalPlan] = query :: Nil
}

/**
* A SHOW TABLES statement, as parsed from SQL.
*/
case class ShowTablesStatement(
namespace: LogicalPlan,
pattern: Option[String]) extends ParsedStatement {
override def children: Seq[LogicalPlan] = Seq(namespace)
}

/**
* A SHOW TABLE EXTENDED statement, as parsed from SQL.
*/
Expand All @@ -371,43 +353,6 @@ case class CreateNamespaceStatement(
ifNotExists: Boolean,
properties: Map[String, String]) extends ParsedStatement

/**
* A DROP NAMESPACE statement, as parsed from SQL.
*/
case class DropNamespaceStatement(
namespace: LogicalPlan,
ifExists: Boolean,
cascade: Boolean) extends ParsedStatement {
override def children: Seq[LogicalPlan] = Seq(namespace)
}

/**
* ALTER (DATABASE|SCHEMA|NAMESPACE) ... SET (DBPROPERTIES|PROPERTIES) command, as parsed from SQL.
*/
case class AlterNamespaceSetPropertiesStatement(
namespace: LogicalPlan,
properties: Map[String, String]) extends ParsedStatement {
override def children: Seq[LogicalPlan] = Seq(namespace)
}

/**
* ALTER (DATABASE|SCHEMA|NAMESPACE) ... SET LOCATION command, as parsed from SQL.
*/
case class AlterNamespaceSetLocationStatement(
namespace: LogicalPlan,
location: String) extends ParsedStatement {
override def children: Seq[LogicalPlan] = Seq(namespace)
}

/**
* A SHOW NAMESPACES statement, as parsed from SQL.
*/
case class ShowNamespacesStatement(
namespace: LogicalPlan,
pattern: Option[String]) extends ParsedStatement {
override def children: Seq[LogicalPlan] = Seq(namespace)
}

/**
* A USE statement, as parsed from SQL.
*/
Expand Down
38 changes: 26 additions & 12 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,18 +250,19 @@ case class CreateNamespace(
* The logical plan of the DROP NAMESPACE command that works for v2 catalogs.
*/
case class DropNamespace(
catalog: CatalogPlugin,
namespace: Seq[String],
namespace: LogicalPlan,
ifExists: Boolean,
cascade: Boolean) extends Command
cascade: Boolean) extends Command {
override def children: Seq[LogicalPlan] = Seq(namespace)
}

/**
* The logical plan of the DESCRIBE NAMESPACE command that works for v2 catalogs.
*/
case class DescribeNamespace(
catalog: SupportsNamespaces,
namespace: Seq[String],
namespace: LogicalPlan,
extended: Boolean) extends Command {
override def children: Seq[LogicalPlan] = Seq(namespace)

override def output: Seq[Attribute] = Seq(
AttributeReference("name", StringType, nullable = false,
Expand All @@ -275,17 +276,29 @@ case class DescribeNamespace(
* command that works for v2 catalogs.
*/
case class AlterNamespaceSetProperties(
catalog: SupportsNamespaces,
namespace: Seq[String],
properties: Map[String, String]) extends Command
namespace: LogicalPlan,
properties: Map[String, String]) extends Command {
override def children: Seq[LogicalPlan] = Seq(namespace)
}

/**
* The logical plan of the ALTER (DATABASE|SCHEMA|NAMESPACE) ... SET LOCATION
* command that works for v2 catalogs.
*/
case class AlterNamespaceSetLocation(
namespace: LogicalPlan,
location: String) extends Command {
override def children: Seq[LogicalPlan] = Seq(namespace)
}

/**
* The logical plan of the SHOW NAMESPACES command that works for v2 catalogs.
*/
case class ShowNamespaces(
catalog: SupportsNamespaces,
namespace: Seq[String],
namespace: LogicalPlan,
pattern: Option[String]) extends Command {
override def children: Seq[LogicalPlan] = Seq(namespace)

override val output: Seq[Attribute] = Seq(
AttributeReference("namespace", StringType, nullable = false)())
}
Expand Down Expand Up @@ -412,9 +425,10 @@ case class RenameTable(
* The logical plan of the SHOW TABLE command that works for v2 catalogs.
*/
case class ShowTables(
catalog: TableCatalog,
namespace: Seq[String],
namespace: LogicalPlan,
pattern: Option[String]) extends Command {
override def children: Seq[LogicalPlan] = Seq(namespace)

override val output: Seq[Attribute] = Seq(
AttributeReference("namespace", StringType, nullable = false)(),
AttributeReference("tableName", StringType, nullable = false)())
Expand Down
Loading