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/21 06:50:32 UTC

[GitHub] [flink] lsyldliu opened a new pull request, #19544: [FLINK-27240][table] Support ADD PARTITION statement for partitioned table

lsyldliu opened a new pull request, #19544:
URL: https://github.com/apache/flink/pull/19544

   ## What is the purpose of the change
   
   Support ADD PARTITION statement for partitioned table
   
   
   ## Brief change log
   
     - *Support ADD PARTITION statement for partitioned table*
   
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
   
     - *Added unit tests in SqlToOperationConverterTest*
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): ( no)
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes)
     - The serializers: (no)
     - The runtime per-record code paths (performance sensitive): (now)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: ( no)
     - The S3 file system connector: (no)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (yes)
     - If yes, how is the feature documented? (docs)
   


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


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

Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19544:
URL: https://github.com/apache/flink/pull/19544#issuecomment-1104790514

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "e0f40248e0cc76ab7fd2095b7f79a856e7481abe",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "e0f40248e0cc76ab7fd2095b7f79a856e7481abe",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * e0f40248e0cc76ab7fd2095b7f79a856e7481abe UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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


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

Posted by GitBox <gi...@apache.org>.
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


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

Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on PR #19544:
URL: https://github.com/apache/flink/pull/19544#issuecomment-1576440459

   @lsyldliu Thanks for contribution. I'll continue your work in #22711


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


[GitHub] [flink] lsyldliu closed pull request #19544: [FLINK-27240][table] Support ADD PARTITION statement for partitioned table

Posted by "lsyldliu (via GitHub)" <gi...@apache.org>.
lsyldliu closed pull request #19544: [FLINK-27240][table] Support ADD PARTITION statement for partitioned table
URL: https://github.com/apache/flink/pull/19544


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