From e61f6bb1623e77a87bd7322e171d9154d13616d9 Mon Sep 17 00:00:00 2001 From: Huaxin Gao Date: Sat, 26 Oct 2019 16:44:51 -0700 Subject: [PATCH 1/2] [SPARK-29481][SQL] ALTER TABLE (RECOVER PARTITIONS) should look up catalog/table like v2 commands --- .../org/apache/spark/sql/catalyst/parser/SqlBase.g4 | 2 +- .../spark/sql/catalyst/parser/AstBuilder.scala | 13 +++++++++++++ .../sql/catalyst/plans/logical/statements.scala | 6 ++++++ .../spark/sql/catalyst/parser/DDLParserSuite.scala | 6 ++++++ .../catalyst/analysis/ResolveSessionCatalog.scala | 6 ++++++ .../apache/spark/sql/execution/SparkSqlParser.scala | 13 ------------- .../spark/sql/connector/DataSourceV2SQLSuite.scala | 11 +++++++++++ .../sql/execution/command/DDLParserSuite.scala | 8 -------- 8 files changed, 43 insertions(+), 22 deletions(-) diff --git a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 index c97eb3c935be..fc7e4d1c1792 100644 --- a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 +++ b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 @@ -170,7 +170,7 @@ statement DROP (IF EXISTS)? partitionSpec (',' partitionSpec)* #dropTablePartitions | ALTER TABLE multipartIdentifier SET locationSpec #setTableLocation | ALTER TABLE tableIdentifier partitionSpec SET locationSpec #setPartitionLocation - | ALTER TABLE tableIdentifier RECOVER PARTITIONS #recoverPartitions + | ALTER TABLE multipartIdentifier RECOVER PARTITIONS #recoverPartitions | DROP TABLE (IF EXISTS)? multipartIdentifier PURGE? #dropTable | DROP VIEW (IF EXISTS)? multipartIdentifier #dropView | CREATE (OR REPLACE)? (GLOBAL? TEMPORARY)? diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala index 4fa479f083e1..e95f886a9783 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala @@ -2877,4 +2877,17 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging override def visitRefreshTable(ctx: RefreshTableContext): LogicalPlan = withOrigin(ctx) { RefreshTableStatement(visitMultipartIdentifier(ctx.multipartIdentifier())) } + + /** + * Create an [[AlterTableRecoverPartitionsStatement]] + * + * For example: + * {{{ + * ALTER TABLE table RECOVER PARTITIONS; + * }}} + */ + override def visitRecoverPartitions( + ctx: RecoverPartitionsContext): LogicalPlan = withOrigin(ctx) { + AlterTableRecoverPartitionsStatement(visitMultipartIdentifier(ctx.multipartIdentifier)) + } } diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statements.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statements.scala index 655e87fce4e2..05cea61b6ff7 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statements.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statements.scala @@ -181,6 +181,12 @@ case class AlterTableSetLocationStatement( tableName: Seq[String], location: String) extends ParsedStatement +/** + * ALTER TABLE ... RECOVER PARTITIONS command, as parsed from SQL. + */ +case class AlterTableRecoverPartitionsStatement( + tableName: Seq[String]) extends ParsedStatement + /** * ALTER VIEW ... SET TBLPROPERTIES command, as parsed from SQL. */ diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala index da01c612b350..1f31926bf795 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala @@ -1120,6 +1120,12 @@ class DDLParserSuite extends AnalysisTest { RefreshTableStatement(Seq("a", "b", "c"))) } + test("alter table: recover partitions") { + comparePlans( + parsePlan("ALTER TABLE a.b.c RECOVER PARTITIONS"), + AlterTableRecoverPartitionsStatement(Seq("a", "b", "c"))) + } + private case class TableSpec( name: Seq[String], schema: Option[StructType], diff --git a/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala b/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala index e7e34b1ef312..40fce4524dd8 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala @@ -322,6 +322,12 @@ class ResolveSessionCatalog( ShowPartitionsCommand( v1TableName.asTableIdentifier, partitionSpec) + + case AlterTableRecoverPartitionsStatement(tableName) => + val v1TableName = parseV1Table(tableName, "ALTER TABLE RECOVER PARTITIONS") + AlterTableRecoverPartitionsCommand( + v1TableName.asTableIdentifier, + "ALTER TABLE RECOVER PARTITIONS") } private def parseV1Table(tableName: Seq[String], sql: String): Seq[String] = { diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala index 20894b39ce5d..a0d9539354fd 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala @@ -546,19 +546,6 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder(conf) { retainData = false) } - /** - * Create an [[AlterTableRecoverPartitionsCommand]] command - * - * For example: - * {{{ - * ALTER TABLE table RECOVER PARTITIONS; - * }}} - */ - override def visitRecoverPartitions( - ctx: RecoverPartitionsContext): LogicalPlan = withOrigin(ctx) { - AlterTableRecoverPartitionsCommand(visitTableIdentifier(ctx.tableIdentifier)) - } - /** * Create an [[AlterTableSetLocationCommand]] command for a partition. * diff --git a/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala index b8a8acbba57c..5b6c9031186a 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala @@ -1300,6 +1300,17 @@ class DataSourceV2SQLSuite } } + test("ALTER TABLE RECOVER PARTITIONS") { + val t = "testcat.ns1.ns2.tbl" + withTable(t) { + spark.sql(s"CREATE TABLE $t (id bigint, data string) USING foo") + val e = intercept[AnalysisException] { + val partition = sql(s"ALTER TABLE $t RECOVER PARTITIONS") + } + assert(e.message.contains("ALTER TABLE RECOVER PARTITIONS is only supported with v1 tables")) + } + } + private def testV1Command(sqlCommand: String, sqlParams: String): Unit = { val e = intercept[AnalysisException] { sql(s"$sqlCommand $sqlParams") diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLParserSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLParserSuite.scala index a9b94bea9517..50ac18a774e4 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLParserSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLParserSuite.scala @@ -594,14 +594,6 @@ class DDLParserSuite extends AnalysisTest with SharedSparkSession { comparePlans(parsed2, expected2) } - test("alter table: recover partitions") { - val sql = "ALTER TABLE table_name RECOVER PARTITIONS" - val parsed = parser.parsePlan(sql) - val expected = AlterTableRecoverPartitionsCommand( - TableIdentifier("table_name", None)) - comparePlans(parsed, expected) - } - test("alter view: add partition (not supported)") { assertUnsupported( """ From aaa6bfd03e65e40858e8ec552c2198b0ede898b1 Mon Sep 17 00:00:00 2001 From: Huaxin Gao Date: Mon, 28 Oct 2019 15:20:40 -0700 Subject: [PATCH 2/2] address comments --- .../scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala index e95f886a9783..67a88d2364f4 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala @@ -2883,7 +2883,7 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging * * For example: * {{{ - * ALTER TABLE table RECOVER PARTITIONS; + * ALTER TABLE multi_part_name RECOVER PARTITIONS; * }}} */ override def visitRecoverPartitions(