Skip to content

[SPARK-29695][SQL] ALTER TABLE (SerDe properties) should look up catalog/table like v2 commands #26374

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
huaxingao wants to merge 2 commits into from
Closed

[SPARK-29695][SQL] ALTER TABLE (SerDe properties) should look up catalog/table like v2 commands #26374

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
d47dd06
[SPARK-29695][SQL] ALTER TABLE (SerDe properties) should look up cata…
huaxingao Nov 3, 2019
7c2078f
address comments
huaxingao Nov 5, 2019
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
4 changes: 2 additions & 2 deletions sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,9 @@ statement
| ALTER TABLE tableIdentifier partitionSpec?
CHANGE COLUMN?
colName=errorCapturingIdentifier colType colPosition? #changeColumn
| ALTER TABLE tableIdentifier (partitionSpec)?
| ALTER TABLE multipartIdentifier (partitionSpec)?
SET SERDE STRING (WITH SERDEPROPERTIES tablePropertyList)? #setTableSerDe
| ALTER TABLE tableIdentifier (partitionSpec)?
| ALTER TABLE multipartIdentifier (partitionSpec)?
SET SERDEPROPERTIES tablePropertyList #setTableSerDe
| ALTER (TABLE | VIEW) multipartIdentifier ADD (IF NOT EXISTS)?
partitionSpecLocation+ #addTablePartition
Expand Down
19 changes: 19 additions & 0 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 @@ -3048,4 +3048,23 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
purge = ctx.PURGE != null,
retainData = false)
}

/**
* Create an [[AlterTableSerDePropertiesStatement]]
*
* For example:
* {{{
* ALTER TABLE multi_part_name [PARTITION spec] SET SERDE serde_name
* [WITH SERDEPROPERTIES props];
* ALTER TABLE multi_part_name [PARTITION spec] SET SERDEPROPERTIES serde_properties;
* }}}
*/
override def visitSetTableSerDe(ctx: SetTableSerDeContext): LogicalPlan = withOrigin(ctx) {
AlterTableSerDePropertiesStatement(
visitMultipartIdentifier(ctx.multipartIdentifier),
Option(ctx.STRING).map(string),
Option(ctx.tablePropertyList).map(visitPropertyKeyValues),
// TODO a partition spec is allowed to have optional values. This is currently violated.
Option(ctx.partitionSpec).map(visitNonOptionalPartitionSpec))
}
}
9 changes: 9 additions & 0 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 @@ -214,6 +214,15 @@ case class AlterTableDropPartitionStatement(
purge: Boolean,
retainData: Boolean) extends ParsedStatement

/**
* ALTER TABLE ... SERDEPROPERTIES command, as parsed from SQL
*/
case class AlterTableSerDePropertiesStatement(
tableName: Seq[String],
serdeClassName: Option[String],
serdeProperties: Option[Map[String, String]],
partitionSpec: Option[TablePartitionSpec]) extends ParsedStatement

/**
* ALTER VIEW ... SET TBLPROPERTIES command, as parsed from SQL.
*/
Expand Down
84 changes: 84 additions & 0 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1320,6 +1320,90 @@ class DDLParserSuite extends AnalysisTest {
ShowCurrentNamespaceStatement())
}

test("alter table: SerDe properties") {
val sql1 = "ALTER TABLE table_name SET SERDE 'org.apache.class'"
val parsed1 = parsePlan(sql1)
val expected1 = AlterTableSerDePropertiesStatement(
Seq("table_name"), Some("org.apache.class"), None, None)
comparePlans(parsed1, expected1)

val sql2 =
"""
|ALTER TABLE table_name SET SERDE 'org.apache.class'
|WITH SERDEPROPERTIES ('columns'='foo,bar', 'field.delim' = ',')
""".stripMargin
val parsed2 = parsePlan(sql2)
val expected2 = AlterTableSerDePropertiesStatement(
Seq("table_name"),
Some("org.apache.class"),
Some(Map("columns" -> "foo,bar", "field.delim" -> ",")),
None)
comparePlans(parsed2, expected2)

val sql3 =
"""
|ALTER TABLE table_name
|SET SERDEPROPERTIES ('columns'='foo,bar', 'field.delim' = ',')
""".stripMargin
val parsed3 = parsePlan(sql3)
val expected3 = AlterTableSerDePropertiesStatement(
Seq("table_name"), None, Some(Map("columns" -> "foo,bar", "field.delim" -> ",")), None)
comparePlans(parsed3, expected3)

val sql4 =
"""
|ALTER TABLE table_name PARTITION (test=1, dt='2008-08-08', country='us')
|SET SERDE 'org.apache.class'
|WITH SERDEPROPERTIES ('columns'='foo,bar', 'field.delim' = ',')
""".stripMargin
val parsed4 = parsePlan(sql4)
val expected4 = AlterTableSerDePropertiesStatement(
Seq("table_name"),
Some("org.apache.class"),
Some(Map("columns" -> "foo,bar", "field.delim" -> ",")),
Some(Map("test" -> "1", "dt" -> "2008-08-08", "country" -> "us")))
comparePlans(parsed4, expected4)

val sql5 =
"""
|ALTER TABLE table_name PARTITION (test=1, dt='2008-08-08', country='us')
|SET SERDEPROPERTIES ('columns'='foo,bar', 'field.delim' = ',')
""".stripMargin
val parsed5 = parsePlan(sql5)
val expected5 = AlterTableSerDePropertiesStatement(
Seq("table_name"),
None,
Some(Map("columns" -> "foo,bar", "field.delim" -> ",")),
Some(Map("test" -> "1", "dt" -> "2008-08-08", "country" -> "us")))
comparePlans(parsed5, expected5)

val sql6 =
"""
|ALTER TABLE a.b.c SET SERDE 'org.apache.class'
|WITH SERDEPROPERTIES ('columns'='foo,bar', 'field.delim' = ',')
""".stripMargin
val parsed6 = parsePlan(sql6)
val expected6 = AlterTableSerDePropertiesStatement(
Seq("a", "b", "c"),
Some("org.apache.class"),
Some(Map("columns" -> "foo,bar", "field.delim" -> ",")),
None)
comparePlans(parsed6, expected6)

val sql7 =
"""
|ALTER TABLE a.b.c PARTITION (test=1, dt='2008-08-08', country='us')
|SET SERDEPROPERTIES ('columns'='foo,bar', 'field.delim' = ',')
""".stripMargin
val parsed7 = parsePlan(sql7)
val expected7 = AlterTableSerDePropertiesStatement(
Seq("a", "b", "c"),
None,
Some(Map("columns" -> "foo,bar", "field.delim" -> ",")),
Some(Map("test" -> "1", "dt" -> "2008-08-08", "country" -> "us")))
comparePlans(parsed7, expected7)
Comment thread
dongjoon-hyun marked this conversation as resolved.
}

private case class TableSpec(
name: Seq[String],
schema: Option[StructType],
Expand Down
9 changes: 9 additions & 0 deletions sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,15 @@ class ResolveSessionCatalog(
ifExists,
purge,
retainData)

case AlterTableSerDePropertiesStatement(
tableName, serdeClassName, serdeProperties, partitionSpec) =>
val v1TableName = parseV1Table(tableName, "ALTER TABLE SerDe Properties")
AlterTableSerDePropertiesCommand(
v1TableName.asTableIdentifier,
serdeClassName,
serdeProperties,
partitionSpec)
}

private def parseV1Table(tableName: Seq[String], sql: String): Seq[String] = {
Expand Down
18 changes: 0 additions & 18 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,24 +408,6 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder(conf) {
ctx.VIEW != null)
}

/**
* Create an [[AlterTableSerDePropertiesCommand]] command.
*
* For example:
* {{{
* ALTER TABLE table [PARTITION spec] SET SERDE serde_name [WITH SERDEPROPERTIES props];
* ALTER TABLE table [PARTITION spec] SET SERDEPROPERTIES serde_properties;
* }}}
*/
override def visitSetTableSerDe(ctx: SetTableSerDeContext): LogicalPlan = withOrigin(ctx) {
AlterTableSerDePropertiesCommand(
visitTableIdentifier(ctx.tableIdentifier),
Option(ctx.STRING).map(string),
Option(ctx.tablePropertyList).map(visitPropertyKeyValues),
// TODO a partition spec is allowed to have optional values. This is currently violated.
Option(ctx.partitionSpec).map(visitNonOptionalPartitionSpec))
}

/**
* Create a [[AlterTableChangeColumnCommand]] command.
*
Expand Down
11 changes: 11 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1441,6 +1441,17 @@ class DataSourceV2SQLSuite
}
}

test("ALTER TABLE SerDe properties") {
val t = "testcat.ns1.ns2.tbl"
withTable(t) {
spark.sql(s"CREATE TABLE $t (id bigint, data string) USING foo PARTITIONED BY (id)")
val e = intercept[AnalysisException] {
sql(s"ALTER TABLE $t SET SERDEPROPERTIES ('columns'='foo,bar', 'field.delim' = ',')")
}
assert(e.message.contains("ALTER TABLE SerDe Properties is only supported with v1 tables"))
}
}

private def testV1Command(sqlCommand: String, sqlParams: String): Unit = {
val e = intercept[AnalysisException] {
sql(s"$sqlCommand $sqlParams")
Expand Down
55 changes: 0 additions & 55 deletions sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLParserSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,61 +458,6 @@ class DDLParserSuite extends AnalysisTest with SharedSparkSession {
containsThesePhrases = Seq("key_with_value"))
}

test("alter table: SerDe properties") {
val sql1 = "ALTER TABLE table_name SET SERDE 'org.apache.class'"
val sql2 =
"""
|ALTER TABLE table_name SET SERDE 'org.apache.class'
|WITH SERDEPROPERTIES ('columns'='foo,bar', 'field.delim' = ',')
""".stripMargin
val sql3 =
"""
|ALTER TABLE table_name SET SERDEPROPERTIES ('columns'='foo,bar',
|'field.delim' = ',')
""".stripMargin
val sql4 =
"""
|ALTER TABLE table_name PARTITION (test=1, dt='2008-08-08',
|country='us') SET SERDE 'org.apache.class' WITH SERDEPROPERTIES ('columns'='foo,bar',
|'field.delim' = ',')
""".stripMargin
val sql5 =
"""
|ALTER TABLE table_name PARTITION (test=1, dt='2008-08-08',
|country='us') SET SERDEPROPERTIES ('columns'='foo,bar', 'field.delim' = ',')
""".stripMargin
val parsed1 = parser.parsePlan(sql1)
val parsed2 = parser.parsePlan(sql2)
val parsed3 = parser.parsePlan(sql3)
val parsed4 = parser.parsePlan(sql4)
val parsed5 = parser.parsePlan(sql5)
val tableIdent = TableIdentifier("table_name", None)
val expected1 = AlterTableSerDePropertiesCommand(
tableIdent, Some("org.apache.class"), None, None)
val expected2 = AlterTableSerDePropertiesCommand(
tableIdent,
Some("org.apache.class"),
Some(Map("columns" -> "foo,bar", "field.delim" -> ",")),
None)
val expected3 = AlterTableSerDePropertiesCommand(
tableIdent, None, Some(Map("columns" -> "foo,bar", "field.delim" -> ",")), None)
val expected4 = AlterTableSerDePropertiesCommand(
tableIdent,
Some("org.apache.class"),
Some(Map("columns" -> "foo,bar", "field.delim" -> ",")),
Some(Map("test" -> "1", "dt" -> "2008-08-08", "country" -> "us")))
val expected5 = AlterTableSerDePropertiesCommand(
tableIdent,
None,
Some(Map("columns" -> "foo,bar", "field.delim" -> ",")),
Some(Map("test" -> "1", "dt" -> "2008-08-08", "country" -> "us")))
comparePlans(parsed1, expected1)
comparePlans(parsed2, expected2)
comparePlans(parsed3, expected3)
comparePlans(parsed4, expected4)
comparePlans(parsed5, expected5)
}

test("alter table - SerDe property values must be set") {
assertUnsupported(
sql = "ALTER TABLE my_tab SET SERDE 'serde' " +
Expand Down