wuchong commented on code in PR #21521:
URL: https://github.com/apache/flink/pull/21521#discussion_r1141373104


##########
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/catalog/CatalogTableITCase.scala:
##########
@@ -1060,6 +1060,61 @@ class CatalogTableITCase(isStreamingMode: Boolean) 
extends AbstractTestBase {
     tableEnv.executeSql("SHOW CREATE TABLE `tmp`")
   }
 
+  @Test
+  def testCreateTableWithCommentAndShowCreateTable(): Unit = {
+    val executedDDL =
+      """
+        |create temporary table TBL1 (
+        |  pk1 bigint not null comment 'this is column pk1 which part of 
primary key',
+        |  h string,
+        |  a bigint,
+        |  g as 2*(a+1) comment 'notice: computed column, expression 
''2*(`a`+1)''.',
+        |  pk2 string not null comment 'this is column pk2 which part of 
primary key',
+        |  c bigint metadata virtual comment 'notice: metadata column, named 
''c''.',
+        |  e row<name string, age int, flag boolean>,
+        |  f as myfunc(a),
+        |  ts1 timestamp(3) comment 'notice: watermark, named ''ts1''.',
+        |  ts2 timestamp_ltz(3) metadata from 'timestamp' comment 'notice: 
metadata column, named ''ts2''.',
+        |  `__source__` varchar(255),
+        |  proc as proctime(),
+        |  watermark for ts1 as cast(timestampadd(hour, 8, ts1) as 
timestamp(3)),
+        |  constraint test_constraint primary key (pk1, pk2) not enforced
+        |) comment 'test show create table statement'
+        |partitioned by (h)
+        |with (
+        |  'connector' = 'kafka',
+        |  'kafka.topic' = 'log.test'
+        |)
+        |""".stripMargin
+
+    val expectedDDL =
+      """ |CREATE TEMPORARY TABLE `default_catalog`.`default_database`.`TBL1` (
+        |  `pk1` BIGINT NOT NULL COMMENT 'this is column pk1 which part of 
primary key',
+        |  `h` VARCHAR(2147483647),
+        |  `a` BIGINT,
+        |  `g` AS 2 * (`a` + 1) COMMENT 'notice: computed column, expression 
''2*(`a`+1)''.',
+        |  `pk2` VARCHAR(2147483647) NOT NULL COMMENT 'this is column pk2 
which part of primary key',
+        |  `c` BIGINT METADATA VIRTUAL COMMENT 'notice: metadata column, named 
''c''.',
+        |  `e` ROW<`name` VARCHAR(2147483647), `age` INT, `flag` BOOLEAN>,
+        |  `f` AS `default_catalog`.`default_database`.`myfunc`(`a`),
+        |  `ts1` TIMESTAMP(3) COMMENT 'notice: watermark, named ''ts1''.',
+        |  `ts2` TIMESTAMP(3) WITH LOCAL TIME ZONE METADATA FROM 'timestamp' 
COMMENT 'notice: metadata column, named ''ts2''.',
+        |  `__source__` VARCHAR(255),
+        |  `proc` AS PROCTIME(),
+        |  WATERMARK FOR `ts1` AS CAST(TIMESTAMPADD(HOUR, 8, `ts1`) AS 
TIMESTAMP(3)),
+        |  CONSTRAINT `test_constraint` PRIMARY KEY (`pk1`, `pk2`) NOT ENFORCED
+        |) COMMENT 'test show create table statement'
+        |PARTITIONED BY (`h`)
+        |WITH (
+        |  'connector' = 'kafka',
+        |  'kafka.topic' = 'log.test'
+        |)
+        |""".stripMargin
+    tableEnv.executeSql(executedDDL)
+    val row = tableEnv.executeSql("SHOW CREATE TABLE `TBL1`").collect().next()
+    assertEquals(expectedDDL, row.getField(0))

Review Comment:
   ```suggestion
       assertEquals(expectedDDL, row.getField(0).toString)
   ```
   Add `toString` to avoid mismatch type comparison.



##########
docs/content/docs/dev/table/sql/describe.md:
##########
@@ -112,11 +112,11 @@ table_env = TableEnvironment.create(...)
 # register a table named "Orders"
 table_env.execute_sql( \
         "CREATE TABLE Orders (" 
-        " `user` BIGINT NOT NULl," 
+        " `user` BIGINT NOT NULl comment 'this is primary key'," 
         " product VARCHAR(32),"
         " amount INT,"
-        " ts TIMESTAMP(3),"
-        " ptime AS PROCTIME(),"
+        " ts TIMESTAMP(3) comment 'notice: watermark',"
+        " ptime AS PROCTIME() comment 'this is a computed column',"

Review Comment:
   There is not much meaning to adding comments in the input SQL. This is not 
how to educate users on using COMMENT in CREATE TABLE, but it should show what 
the SHOW CREATE TABLE displays. Could you add examples and outputs for `SHOW 
CREATE TABLE` in [this 
page](https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/show/#show-create-table)?
 



##########
flink-table/flink-sql-client/src/test/resources/sql/table.q:
##########
@@ -926,6 +927,52 @@ desc orders3;
 5 rows in set
 !ok
 
+# test show columns
+show columns from orders3;
++---------+-----------------------------+-------+-----------+---------------+----------------------------+--------------------------+
+|    name |                        type |  null |       key |        extras |  
                watermark |                  comment |
++---------+-----------------------------+-------+-----------+---------------+----------------------------+--------------------------+
+|    user |                      BIGINT | FALSE | PRI(user) |               |  
                          | this is the first column |
+| product |                 VARCHAR(32) |  TRUE |           |               |  
                          |                          |
+|  amount |                         INT |  TRUE |           |               |  
                          |                          |
+|      ts |      TIMESTAMP(3) *ROWTIME* |  TRUE |           |               | 
`ts` - INTERVAL '1' SECOND |        notice: watermark |
+|   ptime | TIMESTAMP_LTZ(3) *PROCTIME* | FALSE |           | AS PROCTIME() |  
                          |  notice: computed column |
++---------+-----------------------------+-------+-----------+---------------+----------------------------+--------------------------+
+5 rows in set
+!ok
+
+show columns in orders3 like 'p%';
++---------+-----------------------------+-------+-----+---------------+-----------+-------------------------+
+|    name |                        type |  null | key |        extras | 
watermark |                 comment |
++---------+-----------------------------+-------+-----+---------------+-----------+-------------------------+
+| product |                 VARCHAR(32) |  TRUE |     |               |        
   |                         |
+|   ptime | TIMESTAMP_LTZ(3) *PROCTIME* | FALSE |     | AS PROCTIME() |        
   | notice: computed column |
++---------+-----------------------------+-------+-----+---------------+-----------+-------------------------+
+2 rows in set
+!ok
+
+# test show create table
+show create table orders3;
++--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+|                                                                              
                                                                                
                                                                                
                                                                                
                                                                                
                                                           result |
++--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+| CREATE TABLE `default_catalog`.`default_database`.`orders3` (
+  `user` BIGINT NOT NULL COMMENT 'this is the first column',
+  `product` VARCHAR(32),
+  `amount` INT,
+  `ts` TIMESTAMP(3) COMMENT 'notice: watermark',
+  `ptime` AS PROCTIME() COMMENT 'notice: computed column',
+  WATERMARK FOR `ts` AS `ts` - INTERVAL '1' SECOND,
+  CONSTRAINT `PK_3599338` PRIMARY KEY (`user`) NOT ENFORCED

Review Comment:
   Please also add a single quote in the comment to verify the escaping. 



##########
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/api/TableEnvironmentTest.scala:
##########
@@ -2528,7 +2528,14 @@ class TableEnvironmentTest {
     tableEnv.executeSql(sourceDDL)
 
     val expectedResult1 = util.Arrays.asList(
-      Row.of("c0", "CHAR(10)", Boolean.box(true), null, null, null, "this is 
the first column"),
+      Row.of(
+        "c0",
+        "CHAR(10)",
+        Boolean.box(true),
+        null,
+        null,
+        null,
+        "this is the first column, named 'c0'."),

Review Comment:
   In my opinion, adding tests for `TableEnvironmentTest` and 
`SqlDdlToOperationConverterTest` doesn't make much sense, because they already 
worked before this PR.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to