Skip to content

Conversation

@aokolnychyi
Copy link
Contributor

This PR migrates FlinkInputFormat to use SerializableTable instead of TableLoader to support changes in #2984.

Fixes #2978.

@aokolnychyi
Copy link
Contributor Author

Copy link
Contributor

@stevenzwu stevenzwu left a comment

Choose a reason for hiding this comment

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

LGTM. FlinkSource has two execution modes:

  1. batch. A read-only table should be fine for FlinkInputFormat.
  2. streaming execution. there is a separate StreamingMonitorFunction class that takes in TableLoader. So it is not affected by this change.

Since this is a change in a critical section, I would also wait for @openinx or @JingsongLi to take a look.

@aokolnychyi
Copy link
Contributor Author

Thanks for taking a look, @stevenzwu!

Since this is a change in a critical section, I would also wait for @openinx or @JingsongLi to take a look.

Definitely agree!

@openinx @JingsongLi Like I said before, I can also keep the old behavior if needed and just pass one more argument through all these classes in #2984. Using a table seems cleaner but not a hard requirement. Up to you.

@aokolnychyi
Copy link
Contributor Author

@openinx @JingsongLi, could you check this PR whenever you get a sec? If it makes sense, I'll rebase.

@rdblue
Copy link
Contributor

rdblue commented Sep 19, 2021

@aokolnychyi, can you rebase this? I'll review.

@aokolnychyi
Copy link
Contributor Author

I'll close this for now. We can come back to this once we want to support _partition metadata columns in Flink.

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.

Use SerializableTable instead of TableLoader in Flink

3 participants