You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "SteveStevenpoor (via GitHub)" <gi...@apache.org> on 2026/05/19 10:10:59 UTC
[PR] Flink: Table comment bug [iceberg]
SteveStevenpoor opened a new pull request, #16423:
URL: https://github.com/apache/iceberg/pull/16423
This closes #16422.
## Summary
- _Table comment added to properties map_
- _Added `testCreateTableComment()` to `TestFlinkCatalogTable` to verify changes_
--
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
Re: [PR] Flink: Table comment bug [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16423:
URL: https://github.com/apache/iceberg/pull/16423#discussion_r3273515450
##########
flink/v2.1/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java:
##########
@@ -243,6 +243,29 @@ public void testCreateTableLikeInFlinkCatalog() throws TableNotExistException {
.containsEntry(FlinkCreateTableOptions.SRC_CATALOG_PROPS_KEY, srcCatalogProps);
}
+ @TestTemplate
+ public void testCreateTableWithTableComment() {
+ // create table with comment
+ sql("CREATE TABLE tl(id BIGINT) COMMENT 'table comment'");
+ Map<String, String> properties = Maps.newHashMap();
+ properties.put("comment", "table comment");
+ assertThat(table("tl").properties()).containsAllEntriesOf(properties);
+ }
+
+ @TestTemplate
+ public void testAlterTableModifyTableComment() {
+ // create table with comment
+ sql("CREATE TABLE tl(id BIGINT) COMMENT 'table comment'");
+ Map<String, String> properties = Maps.newHashMap();
+ properties.put("comment", "table comment");
+ assertThat(table("tl").properties()).containsAllEntriesOf(properties);
Review Comment:
```suggestion
assertThat(table("tl").properties()).containsEntry(TableProperties.COMMENT, "table comment");
```
And then we don't need the extra `Map`
--
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
Re: [PR] Flink: Table comment bug [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16423:
URL: https://github.com/apache/iceberg/pull/16423#discussion_r3265887347
##########
flink/v2.1/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java:
##########
@@ -243,6 +243,26 @@ public void testCreateTableLikeInFlinkCatalog() throws TableNotExistException {
.containsEntry(FlinkCreateTableOptions.SRC_CATALOG_PROPS_KEY, srcCatalogProps);
}
+ @TestTemplate
+ public void testCreateTableComment() {
+ // create table with comment
+ sql("CREATE TABLE tl(id BIGINT) COMMENT 'table comment' WITH ('param'='some value')");
+ Map<String, String> properties = Maps.newHashMap();
+ properties.put("comment", "table comment");
+ properties.put("param", "some value");
+ assertThat(table("tl").properties()).containsAllEntriesOf(properties);
+
+ // update parameter
+ sql("ALTER TABLE tl SET('param'='new value')");
Review Comment:
These are different tests. They should have a different test method
--
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
Re: [PR] Flink: Table comment bug [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16423:
URL: https://github.com/apache/iceberg/pull/16423#discussion_r3265887347
##########
flink/v2.1/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java:
##########
@@ -243,6 +243,26 @@ public void testCreateTableLikeInFlinkCatalog() throws TableNotExistException {
.containsEntry(FlinkCreateTableOptions.SRC_CATALOG_PROPS_KEY, srcCatalogProps);
}
+ @TestTemplate
+ public void testCreateTableComment() {
+ // create table with comment
+ sql("CREATE TABLE tl(id BIGINT) COMMENT 'table comment' WITH ('param'='some value')");
+ Map<String, String> properties = Maps.newHashMap();
+ properties.put("comment", "table comment");
+ properties.put("param", "some value");
+ assertThat(table("tl").properties()).containsAllEntriesOf(properties);
+
+ // update parameter
+ sql("ALTER TABLE tl SET('param'='new value')");
Review Comment:
These are different tests. They should have a different test method
--
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
Re: [PR] Flink: Handle table comments in FlinkSQL [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary merged PR #16423:
URL: https://github.com/apache/iceberg/pull/16423
--
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
Re: [PR] Flink: Table comment bug [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16423:
URL: https://github.com/apache/iceberg/pull/16423#discussion_r3273518815
##########
flink/v2.1/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java:
##########
@@ -243,6 +243,29 @@ public void testCreateTableLikeInFlinkCatalog() throws TableNotExistException {
.containsEntry(FlinkCreateTableOptions.SRC_CATALOG_PROPS_KEY, srcCatalogProps);
}
+ @TestTemplate
+ public void testCreateTableWithTableComment() {
+ // create table with comment
+ sql("CREATE TABLE tl(id BIGINT) COMMENT 'table comment'");
+ Map<String, String> properties = Maps.newHashMap();
+ properties.put("comment", "table comment");
+ assertThat(table("tl").properties()).containsAllEntriesOf(properties);
+ }
+
+ @TestTemplate
+ public void testAlterTableModifyTableComment() {
+ // create table with comment
+ sql("CREATE TABLE tl(id BIGINT) COMMENT 'table comment'");
+ Map<String, String> properties = Maps.newHashMap();
+ properties.put("comment", "table comment");
+ assertThat(table("tl").properties()).containsAllEntriesOf(properties);
+
+ // alter table comment
+ sql("ALTER TABLE tl SET('comment' = 'new comment')");
+ properties.put("comment", "new comment");
+ assertThat(table("tl").properties()).containsAllEntriesOf(properties);
Review Comment:
Same as above
--
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
Re: [PR] Flink: Handle table comments in FlinkSQL [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #16423:
URL: https://github.com/apache/iceberg/pull/16423#issuecomment-4505079219
Merged to main.
Thanks @SteveStevenpoor for the fix!
Please don't forget to backport
--
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
Re: [PR] Flink: Table comment bug [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16423:
URL: https://github.com/apache/iceberg/pull/16423#discussion_r3265909144
##########
flink/v2.1/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java:
##########
@@ -243,6 +243,26 @@ public void testCreateTableLikeInFlinkCatalog() throws TableNotExistException {
.containsEntry(FlinkCreateTableOptions.SRC_CATALOG_PROPS_KEY, srcCatalogProps);
}
+ @TestTemplate
+ public void testCreateTableComment() {
Review Comment:
I would create the following tests:
testCreateTableWithTableComment
testAlterTableModifyTableComment
I'm not sure why the alter parameter is here in this test
--
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