feat(batch): use iceberg rust in iceberg scan #17545
Conversation
There was a problem hiding this comment.
Is the change on sink related? 👀
There was a problem hiding this comment.
Iceberg sink can't be replaced in the short term. As far as I know, we can do it until Iceberg-rust releases 0.4.0 version. (currently 0.2.0)
There was a problem hiding this comment.
I mean, why do we change sink's code for batch?
There was a problem hiding this comment.
Ohh, it seems the config is defined in sink, but shared by batch and sink... Perhaps we should refactor it later
There was a problem hiding this comment.
BTW, perhaps we can create a tracking issue for deprecating icelake, especially note down blockers.
| @@ -307,19 +308,32 @@ impl IcebergConfig { | |||
| iceberg_configs.insert(CATALOG_NAME.to_string(), self.catalog_name()); | |||
|
|
|||
| if let Some(region) = &self.region { | |||
| // icelake | |||
| iceberg_configs.insert( | |||
| "iceberg.table.io.region".to_string(), | |||
There was a problem hiding this comment.
Why need to keep the config for the deprecated icelake implementation?
There was a problem hiding this comment.
Icelake is still used in other crates like frontend (metadata) and connector (e.g. sink). The frontend part would be done in a later PR, but we still need icelake for sink.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
IcebergScanExecutorwill only depend on iceberg-rust after this PR.Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.