You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/04/25 10:02:15 UTC

[GitHub] [flink] luoyuxia commented on a diff in pull request #19544: [FLINK-27240][table] Support ADD PARTITION statement for partitioned table

luoyuxia commented on code in PR #19544:
URL: https://github.com/apache/flink/pull/19544#discussion_r857444294


##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkSqlParserImplTest.java:
##########
@@ -338,6 +338,25 @@ void testAlterTableCompact() {
                 .fails("(?s).*Encountered \"\\)\" at line 1, column 26.\n.*");
     }
 
+    @Test
+    public void testAddPartition() {
+        sql("alter table tbl add partition (p1=1,p2='a')")

Review Comment:
   nit: test a table like `c1.d1.t1`



##########
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/api/TableEnvironmentTest.scala:
##########
@@ -1932,6 +1942,56 @@ class TableEnvironmentTest {
           "Please instantiate a new TableEnvironment if necessary.")
   }
 
+  @Test
+  def testAddPartitions(): Unit = {
+    tableEnv.useCatalog("part_test_cat")
+    val createTableStmt =
+      """
+        |CREATE TABLE tbl (
+        |  a INT,
+        |  b BIGINT,
+        |  c DATE
+        |) PARTITIONED BY (b, c)
+        |WITH (
+        |  'connector' = 'COLLECTION'
+        |)
+      """.stripMargin
+    tableEnv.executeSql(createTableStmt)
+
+    // test add partition
+    var tableResult = tableEnv.executeSql(
+      "alter table tbl add partition " +
+        "(b=1000,c='2020-05-01') partition (b=2000,c='2020-01-01') with ('k'='v')")
+    assertEquals(ResultKind.SUCCESS, tableResult.getResultKind)
+
+    val spec1 = new CatalogPartitionSpec(Map("b" -> "1000", "c" -> "2020-05-01").asJava)
+    val spec2 = new CatalogPartitionSpec(Map("b" -> "2000", "c" -> "2020-01-01").asJava)
+
+    val tablePath = new ObjectPath("default", "tbl")
+    val actual = catalog.listPartitions(tablePath)
+    // assert partition spec
+    assertEquals(List(spec1, spec2).asJava, actual)
+
+    val part1 = catalog.getPartition(tablePath, spec1)
+    val part2 = catalog.getPartition(tablePath, spec2)
+    // assert partition properties
+    assertEquals(Collections.emptyMap(), part1.getProperties)
+    assertEquals(Collections.singletonMap("k", "v"), part2.getProperties)
+
+    // add existed partition with if not exists
+    tableResult =
+      tableEnv.executeSql("alter table tbl add if not exists partition (b=1000,c='2020-05-01')")
+    assertEquals(ResultKind.SUCCESS, tableResult.getResultKind)

Review Comment:
   maybe we need check it dose have no impact to this tblae.



##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -567,12 +568,20 @@ SqlAlterTable SqlAlterTable() :
                         propertyList);
         }
     |
-        <ADD> constraint = TableConstraint() {
-            return new SqlAlterTableAddConstraint(
-                        tableIdentifier,
-                        constraint,
-                        startPos.plus(getPos()));
-        }
+        <ADD>
+        (
+            constraint = TableConstraint() {
+                return new SqlAlterTableAddConstraint(
+                            tableIdentifier,
+                            constraint,
+                            startPos.plus(getPos()));
+            }
+        |
+            [ <IF> <NOT> <EXISTS> { ifNotExists = true; } ]
+            {

Review Comment:
   Will it be more readable like
   `
   partSpecs = PartSpecs() {
   return SqlAddPartitions( tableIdentifier, partSpecs, ifExists);
   }
   `?
   



##########
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/api/TableEnvironmentTest.scala:
##########
@@ -1932,6 +1942,56 @@ class TableEnvironmentTest {
           "Please instantiate a new TableEnvironment if necessary.")
   }
 
+  @Test
+  def testAddPartitions(): Unit = {

Review Comment:
   Shoud rename/drop/show partition also do the same test?
   https://github.com/apache/flink/pull/19553
   https://github.com/apache/flink/pull/19544
   https://github.com/apache/flink/pull/19545



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org