feat(iceberg): support alter iceberg table connector with #22040
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chenzl25
reviewed
May 30, 2025
Comment on lines
+354
to
+357
| oneof object_type { | ||
| AlterConnectorPropsObject alter_connector_props_object = 6; | ||
| AlterIcebergTablePropsObject alter_iceberg_table_props_object = 7; | ||
| } |
There was a problem hiding this comment.
We'd better use a new RPC instead of reusing the altering sink RPC to reduce the coupling. Current implementation is a bit invasive. Use one RPC is ok, however, for the caller side and the RPC implementation, we'd better handle sink and iceberg table separately.
chenzl25
reviewed
May 30, 2025
| @@ -1311,13 +1311,14 @@ impl MetaClient { | |||
| changed_props: BTreeMap<String, String>, | |||
| changed_secret_refs: BTreeMap<String, PbSecretRef>, | |||
| connector_conn_ref: Option<u32>, | |||
| object_type: ObjectType, | |||
There was a problem hiding this comment.
We should use a new method like alter_iceberg_table instead of alter_sink
Comment on lines
34
to
38
| pub async fn handle_alter_sink_props( | ||
| handler_args: HandlerArgs, | ||
| table_name: ObjectName, | ||
| alter_sink_object: AlterSinkObject, | ||
| changed_props: Vec<SqlOption>, | ||
| ) -> Result<RwPgResponse> { |
There was a problem hiding this comment.
we should have a new method handle_alter_iceberg_table
| @@ -1612,6 +1613,7 @@ impl CatalogController { | |||
| &self, | |||
| sink_id: SinkId, | |||
| props: BTreeMap<String, String>, | |||
| object_type: risingwave_pb::meta::alter_connector_props_request::ObjectType, | |||
There was a problem hiding this comment.
This method is update_sink_props_by_sink_id, if we need to change iceberg table properies, we'd better use a new method.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
Previously, we implemented the
ALTER SINK CONNECTORsyntax, but it lacked support for directly modifying Iceberg tables. Since Iceberg table definitions are not stored via Sink statements, attempts to alter them using Sink syntax would inevitably fail. This PR introducesALTER TABLEsupport for Iceberg tables, enabling direct modifications to their associated Sink configurations.Previously, we synchronized configurations by pulling the full config from the Meta service and dispatching it to the Sink. However, we observed that the Meta-stored configurations were incomplete. This PR refactors the mechanism to incremental updates: only modified configurations are pushed to Compute Nodes and dynamically merged with the Sink's existing configurations.
Checklist
Documentation
Release note