Skip to content
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

[FLINK-30249] Fix TableUtils.getRowTypeInfo by using ExternalTypeInfo.of() #188

Merged
merged 1 commit into from
Jan 6, 2023

Conversation

zhipeng93
Copy link
Contributor

@zhipeng93 zhipeng93 commented Nov 30, 2022

What is the purpose of the change

Fix TableUtils.getRowTypeInfo() by avoiding using TypeInformation.of(...).

Currently TypeInformation created TypeInformation.of() may not be correctly identified when converting between tables and datastreams. (e.g., TimeStamp, CHAR, Decimal in the added test case)

Brief change log

  • Fixed TableUtils.getRowTypeInfo() by using ExternalTypeInfo.of() and added corresponding unit test.
  • Updated the corresponding java tests, e.g, BinarierTest, KmeansTest.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@zhipeng93 zhipeng93 changed the title [FLINK-30249] Fix TableUtils.getRowTypeInfo by using ExternalTypeInfo.of [FLINK-30249] Fix TableUtils.getRowTypeInfo by using ExternalTypeInfo.of() Nov 30, 2022
@zhipeng93 zhipeng93 force-pushed the FLINK-30249 branch 4 times, most recently from 4005206 to 81cacf8 Compare November 30, 2022 10:33
@zhipeng93
Copy link
Contributor Author

Can you help to review this PR @gaoyunhaii @lindong28?

Copy link
Contributor

@yunfengzhou-hub yunfengzhou-hub left a 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! Left some comments as below.

@zhipeng93 zhipeng93 force-pushed the FLINK-30249 branch 2 times, most recently from 34fce80 to 6503e73 Compare January 3, 2023 10:54
Copy link
Member

@lindong28 lindong28 left a 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!

@lindong28
Copy link
Member

Thanks for the update! LGTM.

@lindong28 lindong28 merged commit 77aad8f into apache:master Jan 6, 2023
vacaly pushed a commit to vacaly/flink-ml that referenced this pull request Feb 8, 2023
vacaly pushed a commit to vacaly/flink-ml that referenced this pull request Feb 10, 2023
Fanoid pushed a commit to Fanoid/flink-ml that referenced this pull request Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants