Skip to content

[cdc-common] add createFieldGetters method to create FieldGetters of given Schema.#2762

Merged
leonardBang merged 2 commits intoapache:masterfrom
lvyanquan0624:UseFieldGetter
Nov 29, 2023
Merged

[cdc-common] add createFieldGetters method to create FieldGetters of given Schema.#2762
leonardBang merged 2 commits intoapache:masterfrom
lvyanquan0624:UseFieldGetter

Conversation

@lvyanquan
Copy link
Copy Markdown
Contributor

@lvyanquan lvyanquan commented Nov 27, 2023

This pull request is a follow-up of #2734.
We now remove GenericRecordData and introduce new BinaryRecordData, so we can't get Field for RecordData directly and need to use FieldGetter.
This pr provide a util method to create FieldGetter and an example to get fields from RecordData.

Copy link
Copy Markdown
Contributor

@leonardBang leonardBang left a comment

Choose a reason for hiding this comment

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

Thanks @lvyanquan for the contribution, left one minor coments

@lvyanquan
Copy link
Copy Markdown
Contributor Author

lvyanquan commented Nov 27, 2023

the failed CI would be rerun after #2737 merged.

@leonardBang
Copy link
Copy Markdown
Contributor

the failed CI would be rerun after #2737 merged.

I've merged #2737

@lvyanquan
Copy link
Copy Markdown
Contributor Author

The left of CI failure is unrelated with this pr.

@lvyanquan lvyanquan closed this Nov 28, 2023
@lvyanquan lvyanquan reopened this Nov 28, 2023
@lvyanquan
Copy link
Copy Markdown
Contributor Author

@leonardBang Reopen it to let ValuesSink not implement StatefulSink and remove class like TableSchemaState.
PTAL again.

}

/** an e2e {@link Sink} implementation that print all {@link DataChangeEvent} out. */
private static class ValuesSink implements StatefulSink<Event, TableSchemaState> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TableSchemaState could remove now ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we don't need to add Schema message to State as CreateTableEvent will be sent firstly.

} else if (materializedInMemory && event instanceof DataChangeEvent) {
ValuesDatabase.applyDataChangeEvent((DataChangeEvent) event);
}
// print the detail message to console for verification.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we add a config to control print or not ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

address it.

"True if the DataChangeEvent need to be materialized in memory.");

public static final ConfigOption<Boolean> PRINT =
ConfigOptions.key("print")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

print.enable ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Copy Markdown
Contributor

@leonardBang leonardBang left a comment

Choose a reason for hiding this comment

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

Thanks @lvyanquan for the update, LGTM

@leonardBang leonardBang merged commit a81d4a1 into apache:master Nov 29, 2023
e-mhui pushed a commit to e-mhui/flink-cdc-connectors that referenced this pull request Dec 2, 2023
ChaomingZhangCN pushed a commit to ChaomingZhangCN/flink-cdc that referenced this pull request Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants