[FLINK-18958] Lose column comment when create table

[oikomi]

What is the purpose of the change
add colum comment when users use describe instruction .

Verifying this change
Tested manually on a local cluster.

UT case see BatchTableEnvironmentTest.testExecuteSqlWithDescribeForComment and BatchTableEnvironmentTest.testExecuteSqlWithDescribe

also I have test in my production env as follow.

[flinkbot]

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit b238cb2 (Mon Aug 17 13:06:15 UTC 2020)

Warnings:

• No documentation files were touched! Remember to keep the Flink docs up to date!

_{Mention the bot in a comment to re-run the automated checks.}

Review Progress

• ❓ 1. The [description] looks good.
• ❓ 2. There is [consensus] that the contribution should go into to Flink.
• ❓ 3. Needs [attention] from.
• ❓ 4. The change fits into the overall [architecture].
• ❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

Details

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
• @flinkbot approve all to approve all aspects
• @flinkbot approve-until architecture to approve everything until architecture
• @flinkbot attention @username1 [@username2 ..] to require somebody's attention
• @flinkbot disapprove architecture to remove an approval you gave earlier

[flinkbot]

CI report:

• 1db995a Azure: FAILURE

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot run azure re-run the last Azure build

[fsk119]

@oikomi Thanks for your contribution. But could you offer some test cases to verify your work? I think you should offer tests that when users use describe instruction, they can also have information about column comments. You can take a look at org.apache.flink.table.api.batch.BatchTableEnviromentTest#testExecuteSqlWithDescribe as an example.

[oikomi]

hi @fsk119 I have commited a UT for colum comment and I have found a bug when not use blink planner, thank you very much!

[oikomi]

@flinkbot run azure

[lsyldliu]

@wuchong can you help review this pr when you are free？

[oikomi]

@flinkbot run travis
@flinkbot run azure

[oikomi]

@flinkbot run azure

[wuchong]

Thanks for the contribution @oikomi , after going through it roughly, I have some concern that we are making the TableSchema fatter and fatter. The community had some discussion to introduce a CatalogTableSchema to preserve the metadata information of catalog table and keep TableSchema clean to describe a schema (see FLINK-17793). Obviously, the column comment should belong to CatalogTableSchema.

So I'm hesitated whether we should wait for FLINK-17793 or update TableSchema further.

[oikomi]

@wuchong OK , let's wait for FLINK-17793 .