-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-10170] [table] Support string representation for map and array types in descriptor-based Table API #6578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…escriptor-based Table API
|
@tragicjun +1, this is good, but can you implement the same thing on other types like array? I think they should be in one PR. |
twalthr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @tragicjun. The changes look good to me. One thing that we could improve is making the current implementation more consistent. So far we are using ROW(...) (with parenthesis), but after looking into other implementations such as Hive, we should have used Row<> as well. We should update this in a backwards compatible way. Either in this PR or in a follow-up issue. What do you think?
Can you also update the documentation?
|
@twalthr In the latest commit, I've replaced bracket () with <> for all complex types and updated the documentation. To ensure backward compatibility, bracket () is still supported by the parsers. Please let me know if anything else is missing. |
64ebfcb to
d54672c
Compare
|
@twalthr could you review this again? Anything else I should do before the merge, thanks. |
2d08599 to
e3f4bad
Compare
|
@buptljy In the latest commit, the array types are supported as well, please check it out. |
… types in descriptor-based Table API
e3f4bad to
0165445
Compare
|
lgtm. |
|
Thank you @tragicjun. The changes look good to me. I will take care of merging this... |
…PI types Since 1.6 the recommended way of creating source/sink tables is using connector/format/schema descriptors. This commit adds string-based representation for all types supported by the Table & SQL API. We use a syntax similar to Hive and other SQL projects. This closes #6578.
What is the purpose of the change
Since 1.6 the recommended way of creating source/sink table is using connector/format/schema/ descriptors. However, when declaring map types in the schema descriptor, the following exception would be thrown:
This pull request is to add string representation support for map types.
Brief change log
KeywordandPackratParserfor map types in TypeStringUtilsTypeStringUtils.writeTypeInfo()TypeStringUtilsTestandTableDescriptorTestVerifying this change
This change can be verified covered by updated unit tests, such as TypeStringUtilsTest and TableDescriptorTest.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation