-
Notifications
You must be signed in to change notification settings - Fork 593
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
feat(iceberg): support eq delete merge on read for iceberg #18448
Conversation
Cargo.toml
Outdated
iceberg = { git = "https://github.com/risingwavelabs/iceberg-rust.git", rev = "682b38037dafa78ebbc6e0479844e980c5ed68a4" } | ||
iceberg-catalog-rest = { git = "https://github.com/risingwavelabs/iceberg-rust.git", rev = "682b38037dafa78ebbc6e0479844e980c5ed68a4" } | ||
iceberg-catalog-glue = { git = "https://github.com/risingwavelabs/iceberg-rust.git", rev = "682b38037dafa78ebbc6e0479844e980c5ed68a4" } |
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.
Just curious, do we have a plan to contribute back to upstream to expose delete files?
src/expr/impl/Cargo.toml
Outdated
@@ -51,7 +51,7 @@ itertools = { workspace = true } | |||
jsonbb = { workspace = true } | |||
linkme = { version = "0.3", features = ["used_linker"] } | |||
md5 = "0.7" | |||
moka = { version = "0.12", features = ["sync"] } | |||
moka = { version = "0.12.8", features = ["sync"] } |
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.
Any specific reason that we want to pin to a specific version here?
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.
Since we update the version of iceberg_rust in this pr, and it depends on moka ^0.12.8
0fc921e
to
2004482
Compare
2004482
to
e39af11
Compare
b516b85
to
e0df8cc
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, thanks for your effort!
741b069
to
83c30a6
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 for Cargo.lock
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, thanks for the efforts
let visibilitys: Vec<_> = chunk | ||
.columns() | ||
.iter() | ||
.zip_eq(column_names.clone()) |
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.
ditto
} | ||
Ok(splits | ||
.into_iter() | ||
.filter(|split| !split.files.is_empty()) | ||
.collect_vec()) | ||
} | ||
|
||
async fn get_require_field_names( |
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.
Please add some doc / example for this case
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
support merge on read for iceberg source.
Only support eq delete
The delete file's pk must be consistent
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.