[GitHub] [iceberg] bitsondatadev commented on a diff in pull request #7382: Docs: Fix conflicting description on primary key support in Flink

["bitsondatadev (via GitHub)" <gi...@apache.org>]

bitsondatadev commented on code in PR #7382:
URL: https://github.com/apache/iceberg/pull/7382#discussion_r1247731522


##########
docs/flink-ddl.md:
##########
@@ -26,11 +26,11 @@ menu:
  - limitations under the License.
  -->
 
-## DDL commands

Review Comment:
   This seems unrelated, If this needs to be fixed I'd rather do it on an update focused on fixing these issues holistically.



##########
docs/flink-ddl.md:
##########
@@ -145,13 +145,14 @@ CREATE DATABASE iceberg_db;
 USE iceberg_db;
 ```
 
-### `CREATE TABLE`

Review Comment:
   Same comment for everything from here up to line 29



##########
docs/flink-ddl.md:
##########
@@ -169,13 +184,15 @@ To create a partition table, use `PARTITIONED BY`:
 ```sql
 CREATE TABLE `hive_catalog`.`default`.`sample` (
     id BIGINT COMMENT 'unique id',
-    data STRING
-) PARTITIONED BY (data);
+    `data` STRING NOT NULL
+) 
+PARTITIONED BY (data) 
+WITH ('format-version'='2');
 ```
 
-Iceberg support hidden partition but Flink don't support partitioning by a function on columns, so there is no way to support hidden partition in Flink DDL.
+Iceberg supports hidden partition but Flink doesn't support partitioning by a function on columns, so there is no way to support hidden partition in Flink DDL.
 
-### `CREATE TABLE LIKE`

Review Comment:
   Same comment here for below. Let's keep this focused on fixing these descriptions and do this in a larger PR.



##########
docs/flink-ddl.md:
##########
@@ -169,13 +184,15 @@ To create a partition table, use `PARTITIONED BY`:
 ```sql
 CREATE TABLE `hive_catalog`.`default`.`sample` (
     id BIGINT COMMENT 'unique id',
-    data STRING
-) PARTITIONED BY (data);
+    `data` STRING NOT NULL
+) 
+PARTITIONED BY (data) 
+WITH ('format-version'='2');
 ```
 
-Iceberg support hidden partition but Flink don't support partitioning by a function on columns, so there is no way to support hidden partition in Flink DDL.
+Iceberg supports hidden partition but Flink doesn't support partitioning by a function on columns, so there is no way to support hidden partition in Flink DDL.

Review Comment:
   ```suggestion
   Iceberg supports hidden partitioning but Flink doesn't support partitioning by a function on columns. There is no way to support hidden partitions in the Flink DDL.
   ```



-- 
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@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org