ulysses-you 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_r354680010
########## File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala ########## @@ -642,51 +678,166 @@ 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 AlterTable(_, _, _: DataSourceV2Relation, changes) => + assert(changes == Seq( + TableChange.setProperty("test", "test"), + TableChange.setProperty("comment", "new_comment"))) + case _ => fail("expect AlterTable") + } + + parsed2 match { + case AlterTable(_, _, _: DataSourceV2Relation, changes) => + assert(changes == Seq( + TableChange.removeProperty("comment"), + TableChange.removeProperty("test"))) + case _ => fail("expect AlterTable") + } + + parsed3 match { + case AlterTable(_, _, _: DataSourceV2Relation, changes) => + assert(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 convert it to v2 command with `UnresolvedV2Table` Review comment: That's OK. Thanks. ---------------------------------------------------------------- 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