cloud-fan commented on a change in pull request #25747: [SPARK-29039][SQL]
centralize the catalog and table lookup logic
URL: https://github.com/apache/spark/pull/25747#discussion_r326046696
##########
File path:
sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala
##########
@@ -642,51 +668,161 @@ class PlanResolutionSuite extends AnalysisTest {
// ALTER TABLE table_name SET TBLPROPERTIES ('comment' = new_comment);
// ALTER TABLE table_name UNSET TBLPROPERTIES [IF EXISTS] ('comment', 'key');
test("alter table: alter table properties") {
- val sql1_table = "ALTER TABLE table_name SET TBLPROPERTIES ('test' =
'test', " +
- "'comment' = 'new_comment')"
- val sql2_table = "ALTER TABLE table_name UNSET TBLPROPERTIES ('comment',
'test')"
- val sql3_table = "ALTER TABLE table_name UNSET TBLPROPERTIES IF EXISTS
('comment', 'test')"
+ Seq("v1Table" -> true, "v2Table" -> false, "testcat.tab" -> false).foreach
{
+ case (tblName, useV1Command) =>
+ val sql1 = s"ALTER TABLE $tblName SET TBLPROPERTIES ('test' = 'test',
" +
+ "'comment' = 'new_comment')"
+ val sql2 = s"ALTER TABLE $tblName UNSET TBLPROPERTIES ('comment',
'test')"
+ val sql3 = s"ALTER TABLE $tblName UNSET TBLPROPERTIES IF EXISTS
('comment', 'test')"
+
+ val parsed1 = parseAndResolve(sql1)
+ val parsed2 = parseAndResolve(sql2)
+ val parsed3 = parseAndResolve(sql3)
+
+ val tableIdent = TableIdentifier(tblName, None)
+ if (useV1Command) {
+ val expected1 = AlterTableSetPropertiesCommand(
+ tableIdent, Map("test" -> "test", "comment" -> "new_comment"),
isView = false)
+ val expected2 = AlterTableUnsetPropertiesCommand(
+ tableIdent, Seq("comment", "test"), ifExists = false, isView =
false)
+ val expected3 = AlterTableUnsetPropertiesCommand(
+ tableIdent, Seq("comment", "test"), ifExists = true, isView =
false)
+
+ comparePlans(parsed1, expected1)
+ comparePlans(parsed2, expected2)
+ comparePlans(parsed3, expected3)
+ } else {
+ parsed1 match {
+ case a: AlterTable =>
+ assert(a.changes == Seq(
+ TableChange.setProperty("test", "test"),
+ TableChange.setProperty("comment", "new_comment")))
+ case _ => fail("expect AlterTable")
+ }
+
+ parsed2 match {
+ case a: AlterTable =>
+ assert(a.changes == Seq(
+ TableChange.removeProperty("comment"),
+ TableChange.removeProperty("test")))
+ case _ => fail("expect AlterTable")
+ }
+
+ parsed3 match {
+ case a: AlterTable =>
+ assert(a.changes == Seq(
+ TableChange.removeProperty("comment"),
+ TableChange.removeProperty("test")))
+ case _ => fail("expect AlterTable")
+ }
+ }
+ }
- val parsed1_table = parseAndResolve(sql1_table)
- val parsed2_table = parseAndResolve(sql2_table)
- val parsed3_table = parseAndResolve(sql3_table)
+ val sql4 = "ALTER TABLE non_exist SET TBLPROPERTIES ('test' = 'test')"
+ val sql5 = "ALTER TABLE non_exist UNSET TBLPROPERTIES ('test')"
+ val parsed4 = parseAndResolve(sql4)
+ val parsed5 = parseAndResolve(sql5)
- val tableIdent = TableIdentifier("table_name", None)
- val expected1_table = AlterTableSetPropertiesCommand(
- tableIdent, Map("test" -> "test", "comment" -> "new_comment"), isView =
false)
- val expected2_table = AlterTableUnsetPropertiesCommand(
- tableIdent, Seq("comment", "test"), ifExists = false, isView = false)
- val expected3_table = AlterTableUnsetPropertiesCommand(
- tableIdent, Seq("comment", "test"), ifExists = true, isView = false)
-
- comparePlans(parsed1_table, expected1_table)
- comparePlans(parsed2_table, expected2_table)
- comparePlans(parsed3_table, expected3_table)
+ // For non-existing tables, we leave the statement unchanged. Analyzer
will fail the query with
+ // table not found later.
+ assert(parsed4.isInstanceOf[AlterTableSetPropertiesStatement])
+ assert(parsed5.isInstanceOf[AlterTableUnsetPropertiesStatement])
}
test("support for other types in TBLPROPERTIES") {
- val sql =
- """
- |ALTER TABLE table_name
- |SET TBLPROPERTIES ('a' = 1, 'b' = 0.1, 'c' = TRUE)
- """.stripMargin
- val parsed = parseAndResolve(sql)
- val expected = AlterTableSetPropertiesCommand(
- TableIdentifier("table_name"),
- Map("a" -> "1", "b" -> "0.1", "c" -> "true"),
- isView = false)
-
- comparePlans(parsed, expected)
+ Seq("v1Table" -> true, "v2Table" -> false, "testcat.tab" -> false).foreach
{
+ case (tblName, useV1Command) =>
+ val sql =
+ s"""
+ |ALTER TABLE $tblName
+ |SET TBLPROPERTIES ('a' = 1, 'b' = 0.1, 'c' = TRUE)
+ """.stripMargin
+ val parsed = parseAndResolve(sql)
+ if (useV1Command) {
+ val expected = AlterTableSetPropertiesCommand(
+ TableIdentifier(tblName),
+ Map("a" -> "1", "b" -> "0.1", "c" -> "true"),
+ isView = false)
+
+ comparePlans(parsed, expected)
+ } else {
+ parsed match {
+ case a: AlterTable =>
+ assert(a.changes == Seq(
+ TableChange.setProperty("a", "1"),
+ TableChange.setProperty("b", "0.1"),
+ TableChange.setProperty("c", "true")))
+ case _ => fail("expect AlterTable")
+ }
+ }
+ }
}
test("alter table: set location") {
- val sql1 = "ALTER TABLE table_name SET LOCATION 'new location'"
- val parsed1 = parseAndResolve(sql1)
- val tableIdent = TableIdentifier("table_name", None)
- val expected1 = AlterTableSetLocationCommand(
- tableIdent,
- None,
- "new location")
- comparePlans(parsed1, expected1)
+ Seq("v1Table" -> true, "v2Table" -> false, "testcat.tab" -> false).foreach
{
+ case (tblName, useV1Command) =>
+ val sql = s"ALTER TABLE $tblName SET LOCATION 'new location'"
+ val parsed = parseAndResolve(sql)
+ if (useV1Command) {
+ val expected = AlterTableSetLocationCommand(
+ TableIdentifier(tblName, None),
+ None,
+ "new location")
+ comparePlans(parsed, expected)
+ } else {
+ parsed match {
+ case a: AlterTable =>
+ assert(a.changes == Seq(TableChange.setProperty("location", "new
location")))
+ case _ => fail("expect AlterTable")
+ }
+ }
+ }
}
+
+ test("describe table") {
Review comment:
I added one more test here, eventually we should test all the v2 SQL
statements.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
