-
Notifications
You must be signed in to change notification settings - Fork 596
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
fix(iceberg): fix commit_checkpoint_interval for iceberg table with connector #19788
fix(iceberg): fix commit_checkpoint_interval for iceberg table with connector #19788
Conversation
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.
Why not make commit_checkpoint_interval
to be a known field? I guess just need to add it to the config struct.
Because I want to make it available in every source connector when creating a table with an iceberg engine, making it a known field will be invasive to every connector property, while it also doesn't make sense to make it a known field when the table is hummock table. |
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.
Rest lgtm
// remove commit_checkpoint_interval from source options, otherwise it will be considered as an unknown field. | ||
source | ||
.as_mut() | ||
.map(|x| x.with_properties.remove(COMMIT_CHECKPOINT_INTERVAL)); |
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.
I guess it will be persisted in the sink plan, so even remove it from the PbSource
, it can still recovery correctly?
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.
Yes.
…terval_with_connector
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
commit_checkpoint_interval
from the connector property which will result in a unknown field.Checklist
Documentation
Release note