-
Notifications
You must be signed in to change notification settings - Fork 2k
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-36784][common] Support to add metadata columns for data in the meta fields of DataChangeEvent at transform #3758
Conversation
17b61d5
to
35d174e
Compare
35d174e
to
ac4c708
Compare
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.
LGTM!
flink-cdc-common/src/main/java/org/apache/flink/cdc/common/source/SupportedMetadataColumn.java
Outdated
Show resolved
Hide resolved
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 @ruanhang1993's nice work, looks nice overall. Just left some optional comments that should not block this PR.
tableInfo, | ||
transformFilter, | ||
timezone, | ||
udfDescriptors, | ||
udfFunctionInstances, | ||
supportedMetadataColumns); |
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.
Feels like we need something like TransformContext
to gather all these things together instead of extending arguments lists forever, but it's not a must in this PR.
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 this suggestion. Let open a new issue for this optimization.
...src/main/java/org/apache/flink/cdc/runtime/operators/transform/TransformFilterProcessor.java
Outdated
Show resolved
Hide resolved
ac4c708
to
f6bd9d6
Compare
f6bd9d6
to
b19b585
Compare
b19b585
to
eaa6152
Compare
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.
LGTM.
Updating docs for the newly added option is also welcomed.
9778882
to
a1c8227
Compare
flink-cdc-composer/src/main/java/org/apache/flink/cdc/composer/flink/FlinkPipelineComposer.java
Show resolved
Hide resolved
...ysql/src/main/java/org/apache/flink/cdc/connectors/mysql/factory/MySqlDataSourceFactory.java
Outdated
Show resolved
Hide resolved
...mysql/src/main/java/org/apache/flink/cdc/connectors/mysql/source/MySqlDataSourceOptions.java
Outdated
Show resolved
Hide resolved
...-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/parser/metadata/MetadataColumns.java
Outdated
Show resolved
Hide resolved
… meta fields of DataChangeEvent at transform
…in MySqlDataSource to pass metadata info like op_ts to downstream.
11fea9d
to
a503ee9
Compare
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.
LGTM
… meta fields of DataChangeEvent at transform (apache#3758) Co-authored-by: Kunni <[email protected]>
This PR supports to add metadata in the meta field of DataChangeEvent at transform.