Skip to content

Flink: Handle table comments in FlinkSQL#16423

Merged
pvary merged 3 commits into
apache:mainfrom
SteveStevenpoor:table-comment-bug
May 21, 2026
Merged

Flink: Handle table comments in FlinkSQL#16423
pvary merged 3 commits into
apache:mainfrom
SteveStevenpoor:table-comment-bug

Conversation

@SteveStevenpoor
Copy link
Copy Markdown
Contributor

This closes #16422.

Summary

  • Table comment added to properties map
  • Added testCreateTableComment() to TestFlinkCatalogTable to verify changes

@github-actions github-actions Bot added the flink label May 19, 2026
}

@TestTemplate
public void testCreateTableComment() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would create the following tests:
testCreateTableWithTableComment
testAlterTableModifyTableComment

I'm not sure why the alter parameter is here in this test

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);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assertThat(table("tl").properties()).containsAllEntriesOf(properties);
assertThat(table("tl").properties()).containsEntry(TableProperties.COMMENT, "table comment");

And then we don't need the extra Map

// alter table comment
sql("ALTER TABLE tl SET('comment' = 'new comment')");
properties.put("comment", "new comment");
assertThat(table("tl").properties()).containsAllEntriesOf(properties);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@pvary pvary changed the title Flink: Table comment bug Flink: Handle table comments in FlinkSQL May 21, 2026
@pvary pvary merged commit 693e8a7 into apache:main May 21, 2026
30 checks passed
@pvary
Copy link
Copy Markdown
Contributor

pvary commented May 21, 2026

Merged to main.
Thanks @SteveStevenpoor for the fix!
Please don't forget to backport

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flink: Table comment bug

2 participants