Skip to content

[SPARK-34475][SQL] Rename logical nodes of v2 ALTER commands #31596

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
MaxGekk wants to merge 17 commits into from
Closed

[SPARK-34475][SQL] Rename logical nodes of v2 ALTER commands #31596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
4388358
AlterTableAddPartition -> AddPartition
MaxGekk Feb 19, 2021
b4b6c15
AlterTableDropPartition -> DropPartition
MaxGekk Feb 19, 2021
91b738f
AlterTableRenamePartition -> RenamePartition
MaxGekk Feb 19, 2021
2a50be1
AlterNamespaceSetProperties -> SetNamespaceProperties
MaxGekk Feb 19, 2021
f26ac6f
AlterNamespaceSetLocation -> SetNamespaceLocation
MaxGekk Feb 19, 2021
c3938a6
AlterViewSetProperties -> SetViewProperties
MaxGekk Feb 19, 2021
19dbddd
AlterViewUnsetProperties -> UnsetViewProperties
MaxGekk Feb 19, 2021
1280b32
AlterTableSerDeProperties -> SetTableSerDeProperties
MaxGekk Feb 19, 2021
905f873
AlterTableSetLocation -> SetTableLocation
MaxGekk Feb 19, 2021
f7b6ce2
AlterTableSetProperties -> SetTableProperties
MaxGekk Feb 19, 2021
d314b60
AlterTableUnsetProperties -> UnsetTableProperties
MaxGekk Feb 19, 2021
5109748
Fix imports
MaxGekk Feb 19, 2021
2a721ad
Address review comments
MaxGekk Feb 20, 2021
8270b6e
AddPartition -> AddPartitions
MaxGekk Feb 20, 2021
6da1d08
DropPartition -> DropPartitions
MaxGekk Feb 20, 2021
09e7268
RenamePartition -> RenamePartitions
MaxGekk Feb 22, 2021
f6ab9ab
Trigger build
MaxGekk Feb 22, 2021
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
6 changes: 3 additions & 3 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,13 +565,13 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog {
// no validation needed for set and remove property
}

case AlterTableAddPartition(r: ResolvedTable, parts, _) =>
case AddPartitions(r: ResolvedTable, parts, _) =>
checkAlterTablePartition(r.table, parts)

case AlterTableDropPartition(r: ResolvedTable, parts, _, _) =>
case DropPartitions(r: ResolvedTable, parts, _, _) =>
checkAlterTablePartition(r.table, parts)

case AlterTableRenamePartition(r: ResolvedTable, from, _) =>
case RenamePartitions(r: ResolvedTable, from, _) =>
checkAlterTablePartition(r.table, Seq(from))

case showPartitions: ShowPartitions => checkShowPartitions(showPartitions)
Expand Down
8 changes: 4 additions & 4 deletions ...catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolvePartitionSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ package org.apache.spark.sql.catalyst.analysis
import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec
import org.apache.spark.sql.catalyst.expressions.{Cast, Literal}
import org.apache.spark.sql.catalyst.plans.logical.{AlterTableAddPartition, AlterTableDropPartition, AlterTableRenamePartition, LogicalPlan, ShowPartitions}
import org.apache.spark.sql.catalyst.plans.logical.{AddPartitions, DropPartitions, LogicalPlan, RenamePartitions, ShowPartitions}
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.catalyst.util.CharVarcharUtils
import org.apache.spark.sql.connector.catalog.SupportsPartitionManagement
Expand All @@ -33,7 +33,7 @@ import org.apache.spark.sql.util.PartitioningUtils.{normalizePartitionSpec, requ
object ResolvePartitionSpec extends Rule[LogicalPlan] {

def apply(plan: LogicalPlan): LogicalPlan = plan resolveOperators {
case r @ AlterTableAddPartition(
case r @ AddPartitions(
ResolvedTable(_, _, table: SupportsPartitionManagement, _), partSpecs, _) =>
val partitionSchema = table.partitionSchema()
r.copy(parts = resolvePartitionSpecs(
Expand All @@ -42,7 +42,7 @@ object ResolvePartitionSpec extends Rule[LogicalPlan] {
partitionSchema,
requireExactMatchedPartitionSpec(table.name, _, partitionSchema.fieldNames)))

case r @ AlterTableDropPartition(
case r @ DropPartitions(
ResolvedTable(_, _, table: SupportsPartitionManagement, _), partSpecs, _, _) =>
val partitionSchema = table.partitionSchema()
r.copy(parts = resolvePartitionSpecs(
Expand All @@ -51,7 +51,7 @@ object ResolvePartitionSpec extends Rule[LogicalPlan] {
partitionSchema,
requireExactMatchedPartitionSpec(table.name, _, partitionSchema.fieldNames)))

case r @ AlterTableRenamePartition(
case r @ RenamePartitions(
ResolvedTable(_, _, table: SupportsPartitionManagement, _), from, to) =>
val partitionSchema = table.partitionSchema()
val Seq(resolvedFrom, resolvedTo) = resolvePartitionSpecs(
Expand Down
6 changes: 3 additions & 3 deletions ...talyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveTableProperties.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@ package org.apache.spark.sql.catalyst.analysis
import scala.collection.JavaConverters._

import org.apache.spark.sql.AnalysisException
import org.apache.spark.sql.catalyst.plans.logical.{AlterTableUnsetProperties, LogicalPlan}
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, UnsetTableProperties}
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.connector.catalog.CatalogV2Implicits.IdentifierHelper
import org.apache.spark.sql.connector.catalog.TableCatalog

/**
* A rule for resolving AlterTableUnsetProperties to handle non-existent properties.
* A rule for resolving [[UnsetTableProperties]] to handle non-existent properties.
*/
object ResolveTableProperties extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = plan.resolveOperatorsUp {
case a @ AlterTableUnsetProperties(r: ResolvedTable, props, ifExists) if !ifExists =>
case a @ UnsetTableProperties(r: ResolvedTable, props, ifExists) if !ifExists =>
val tblProperties = r.table.properties.asScala
props.foreach { p =>
if (!tblProperties.contains(p) && p != TableCatalog.PROP_COMMENT) {
Expand Down
44 changes: 22 additions & 22 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 @@ -2767,7 +2767,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
}

/**
* Create an [[AlterNamespaceSetProperties]] logical plan.
* Create an [[SetNamespaceProperties]] logical plan.
*
* For example:
* {{{
Expand All @@ -2778,14 +2778,14 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
override def visitSetNamespaceProperties(ctx: SetNamespacePropertiesContext): LogicalPlan = {
withOrigin(ctx) {
val properties = cleanNamespaceProperties(visitPropertyKeyValues(ctx.tablePropertyList), ctx)
AlterNamespaceSetProperties(
SetNamespaceProperties(
UnresolvedNamespace(visitMultipartIdentifier(ctx.multipartIdentifier)),
properties)
}
}

/**
* Create an [[AlterNamespaceSetLocation]] logical plan.
* Create an [[SetNamespaceLocation]] logical plan.
*
* For example:
* {{{
Expand All @@ -2794,7 +2794,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
*/
override def visitSetNamespaceLocation(ctx: SetNamespaceLocationContext): LogicalPlan = {
withOrigin(ctx) {
AlterNamespaceSetLocation(
SetNamespaceLocation(
UnresolvedNamespace(visitMultipartIdentifier(ctx.multipartIdentifier)),
visitLocationSpec(ctx.locationSpec))
}
Expand Down Expand Up @@ -3477,7 +3477,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
}

/**
* Parse [[AlterViewSetProperties]] or [[AlterTableSetProperties]] commands.
* Parse [[SetViewProperties]] or [[SetTableProperties]] commands.
*
* For example:
* {{{
Expand All @@ -3490,15 +3490,15 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
val properties = visitPropertyKeyValues(ctx.tablePropertyList)
val cleanedTableProperties = cleanTableProperties(ctx, properties)
if (ctx.VIEW != null) {
AlterViewSetProperties(
SetViewProperties(
createUnresolvedView(
ctx.multipartIdentifier,
commandName = "ALTER VIEW ... SET TBLPROPERTIES",
allowTemp = false,
relationTypeMismatchHint = alterViewTypeMismatchHint),
cleanedTableProperties)
} else {
AlterTableSetProperties(
SetTableProperties(
createUnresolvedTable(
ctx.multipartIdentifier,
"ALTER TABLE ... SET TBLPROPERTIES",
Expand All @@ -3508,7 +3508,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
}

/**
* Parse [[AlterViewUnsetProperties]] or [[AlterTableUnsetProperties]] commands.
* Parse [[UnsetViewProperties]] or [[UnsetTableProperties]] commands.
*
* For example:
* {{{
Expand All @@ -3523,7 +3523,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg

val ifExists = ctx.EXISTS != null
if (ctx.VIEW != null) {
AlterViewUnsetProperties(
UnsetViewProperties(
createUnresolvedView(
ctx.multipartIdentifier,
commandName = "ALTER VIEW ... UNSET TBLPROPERTIES",
Expand All @@ -3532,7 +3532,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
cleanedProperties,
ifExists)
} else {
AlterTableUnsetProperties(
UnsetTableProperties(
createUnresolvedTable(
ctx.multipartIdentifier,
"ALTER TABLE ... UNSET TBLPROPERTIES",
Expand All @@ -3543,15 +3543,15 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
}

/**
* Create an [[AlterTableSetLocation]] command.
* Create an [[SetTableLocation]] command.
*
* For example:
* {{{
* ALTER TABLE table_name [PARTITION partition_spec] SET LOCATION "loc";
* }}}
*/
override def visitSetTableLocation(ctx: SetTableLocationContext): LogicalPlan = withOrigin(ctx) {
AlterTableSetLocation(
SetTableLocation(
createUnresolvedTable(
ctx.multipartIdentifier,
"ALTER TABLE ... SET LOCATION ...",
Expand Down Expand Up @@ -3810,7 +3810,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
}

/**
* Create an [[AlterTableRecoverPartitions]]
* Create an [[RecoverPartitions]]
*
* For example:
* {{{
Expand All @@ -3819,15 +3819,15 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
*/
override def visitRecoverPartitions(
ctx: RecoverPartitionsContext): LogicalPlan = withOrigin(ctx) {
AlterTableRecoverPartitions(
RecoverPartitions(
createUnresolvedTable(
ctx.multipartIdentifier,
"ALTER TABLE ... RECOVER PARTITIONS",
alterTableTypeMismatchHint))
}

/**
* Create an [[AlterTableAddPartition]].
* Create an [[AddPartitions]].
*
* For example:
* {{{
Expand All @@ -3849,7 +3849,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
val location = Option(splCtx.locationSpec).map(visitLocationSpec)
UnresolvedPartitionSpec(spec, location)
}
AlterTableAddPartition(
AddPartitions(
createUnresolvedTable(
ctx.multipartIdentifier,
"ALTER TABLE ... ADD PARTITION ...",
Expand All @@ -3859,7 +3859,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
}

/**
* Create an [[AlterTableRenamePartition]]
* Create an [[RenamePartitions]]
*
* For example:
* {{{
Expand All @@ -3868,7 +3868,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
*/
override def visitRenameTablePartition(
ctx: RenameTablePartitionContext): LogicalPlan = withOrigin(ctx) {
AlterTableRenamePartition(
RenamePartitions(
createUnresolvedTable(
ctx.multipartIdentifier,
"ALTER TABLE ... RENAME TO PARTITION",
Expand All @@ -3878,7 +3878,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
}

/**
* Create an [[AlterTableDropPartition]]
* Create an [[DropPartitions]]
*
* For example:
* {{{
Expand All @@ -3897,7 +3897,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
}
val partSpecs = ctx.partitionSpec.asScala.map(visitNonOptionalPartitionSpec)
.map(spec => UnresolvedPartitionSpec(spec))
AlterTableDropPartition(
DropPartitions(
createUnresolvedTable(
ctx.multipartIdentifier,
"ALTER TABLE ... DROP PARTITION ...",
Expand All @@ -3908,7 +3908,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
}

/**
* Create an [[AlterTableSerDeProperties]]
* Create an [[SetTableSerDeProperties]]
*
* For example:
* {{{
Expand All @@ -3918,7 +3918,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
* }}}
*/
override def visitSetTableSerDe(ctx: SetTableSerDeContext): LogicalPlan = withOrigin(ctx) {
AlterTableSerDeProperties(
SetTableSerDeProperties(
createUnresolvedTable(
ctx.multipartIdentifier,
"ALTER TABLE ... SET [SERDE|SERDEPROPERTIES]",
Expand Down
24 changes: 12 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 @@ -305,7 +305,7 @@ case class DescribeNamespace(
* The logical plan of the ALTER (DATABASE|SCHEMA|NAMESPACE) ... SET (DBPROPERTIES|PROPERTIES)
* command.
*/
case class AlterNamespaceSetProperties(
case class SetNamespaceProperties(
namespace: LogicalPlan,
properties: Map[String, String]) extends Command {
override def children: Seq[LogicalPlan] = Seq(namespace)
Expand All @@ -314,7 +314,7 @@ case class AlterNamespaceSetProperties(
/**
* The logical plan of the ALTER (DATABASE|SCHEMA|NAMESPACE) ... SET LOCATION command.
*/
case class AlterNamespaceSetLocation(
case class SetNamespaceLocation(
namespace: LogicalPlan,
location: String) extends Command {
override def children: Seq[LogicalPlan] = Seq(namespace)
Expand Down Expand Up @@ -676,7 +676,7 @@ case class AnalyzeColumn(
* PARTITION spec1 [LOCATION 'loc1'][, PARTITION spec2 [LOCATION 'loc2'], ...];
* }}}
*/
case class AlterTableAddPartition(
case class AddPartitions(
child: LogicalPlan,
parts: Seq[PartitionSpec],
ifNotExists: Boolean) extends Command {
Expand All @@ -698,7 +698,7 @@ case class AlterTableAddPartition(
* ALTER TABLE table DROP [IF EXISTS] PARTITION spec1[, PARTITION spec2, ...] [PURGE];
* }}}
*/
case class AlterTableDropPartition(
case class DropPartitions(
child: LogicalPlan,
parts: Seq[PartitionSpec],
ifExists: Boolean,
Expand All @@ -712,7 +712,7 @@ case class AlterTableDropPartition(
/**
* The logical plan of the ALTER TABLE ... RENAME TO PARTITION command.
*/
case class AlterTableRenamePartition(
case class RenamePartitions(
child: LogicalPlan,
from: PartitionSpec,
to: PartitionSpec) extends Command {
Expand All @@ -727,7 +727,7 @@ case class AlterTableRenamePartition(
/**
* The logical plan of the ALTER TABLE ... RECOVER PARTITIONS command.
*/
case class AlterTableRecoverPartitions(child: LogicalPlan) extends Command {
case class RecoverPartitions(child: LogicalPlan) extends Command {
override def children: Seq[LogicalPlan] = child :: Nil
}

Expand Down Expand Up @@ -819,7 +819,7 @@ case class AlterViewAs(
/**
* The logical plan of the ALTER VIEW ... SET TBLPROPERTIES command.
*/
case class AlterViewSetProperties(
case class SetViewProperties(
child: LogicalPlan,
properties: Map[String, String]) extends Command {
override def children: Seq[LogicalPlan] = child :: Nil
Expand All @@ -828,7 +828,7 @@ case class AlterViewSetProperties(
/**
* The logical plan of the ALTER VIEW ... UNSET TBLPROPERTIES command.
*/
case class AlterViewUnsetProperties(
case class UnsetViewProperties(
child: LogicalPlan,
propertyKeys: Seq[String],
ifExists: Boolean) extends Command {
Expand All @@ -838,7 +838,7 @@ case class AlterViewUnsetProperties(
/**
* The logical plan of the ALTER TABLE ... SET [SERDE|SERDEPROPERTIES] command.
*/
case class AlterTableSerDeProperties(
case class SetTableSerDeProperties(
child: LogicalPlan,
serdeClassName: Option[String],
serdeProperties: Option[Map[String, String]],
Expand Down Expand Up @@ -876,7 +876,7 @@ case class UncacheTable(
/**
* The logical plan of the ALTER TABLE ... SET LOCATION command.
*/
case class AlterTableSetLocation(
case class SetTableLocation(
Copy link
Member Author

@MaxGekk MaxGekk Feb 19, 2021

Choose a reason for hiding this comment

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

The command incapsulates 2 commands actually: set table location and set partition location. Should we split it to SetTableLocation and SetPartitionLocation (separately in another PR).

Copy link
Contributor

@cloud-fan cloud-fan Feb 22, 2021

Choose a reason for hiding this comment

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

SGTM

table: LogicalPlan,
partitionSpec: Option[TablePartitionSpec],
location: String) extends Command {
Expand All @@ -886,7 +886,7 @@ case class AlterTableSetLocation(
/**
* The logical plan of the ALTER TABLE ... SET TBLPROPERTIES command.
*/
case class AlterTableSetProperties(
case class SetTableProperties(
table: LogicalPlan,
properties: Map[String, String]) extends Command {
override def children: Seq[LogicalPlan] = table :: Nil
Expand All @@ -895,7 +895,7 @@ case class AlterTableSetProperties(
/**
* The logical plan of the ALTER TABLE ... UNSET TBLPROPERTIES command.
*/
case class AlterTableUnsetProperties(
case class UnsetTableProperties(
table: LogicalPlan,
propertyKeys: Seq[String],
ifExists: Boolean) extends Command {
Expand Down
Loading