feat: Add equality delete writer #372
Conversation
crates/iceberg/src/writer/base_writer/equality_delete_writer.rs
Outdated
Show resolved
Hide resolved
| impl<B: FileWriterBuilder> IcebergWriter for EqualityDeleteFileWriter<B> { | ||
| async fn write(&mut self, batch: RecordBatch) -> Result<()> { | ||
| let batch = self.project_record_batch_columns(batch)?; | ||
| self.inner_writer.as_mut().unwrap().write(&batch).await |
There was a problem hiding this comment.
We should use ? rather than unwrap() here.
There was a problem hiding this comment.
? is for Result but self.inner_writer is an Option. I think it is reasonable to leave it just Option. Maybe self.inner_writer.as_mut().ok_or_else(...).write(&batch).await is better?
crates/iceberg/src/writer/base_writer/equality_delete_writer.rs
Outdated
Show resolved
Hide resolved
crates/iceberg/src/writer/base_writer/equality_delete_writer.rs
Outdated
Show resolved
Hide resolved
crates/iceberg/src/writer/base_writer/equality_delete_writer.rs
Outdated
Show resolved
Hide resolved
Dysprosium0626
force-pushed
the
add_equality_delete_writer
branch
2 times, most recently
from
2aa2e5f to
f4c6cfa
Compare
| } else { | ||
| Err(Error::new( | ||
| ErrorKind::Unexpected, | ||
| "Equality delete inner writer does not exist", |
There was a problem hiding this comment.
I think returning an unexpected error here is reasonable. I will fix
later.
Dysprosium0626
force-pushed
the
add_equality_delete_writer
branch
from
0cc7049 to
d140194
Compare
There was a problem hiding this comment.
Looks good to me. Thank you @Dysprosium0626 for working on this, and thank @sdd & @ZENOTME for the review.
This reverts commit ad89eac.
|
Hi, I'm sorry, but I need to revert this PR. @Dysprosium0626, could you reopen and rebase your original PR and test it again? |
Okay, I will reopen this PR after test it again. |
|
Hi, I find that this PR has some fail. I try to fix it in #703. To simplify the review, I separate this PR and fix into two commits. Feel free to tell me if something needs to be refined or fixed. cc @Dysprosium0626 @Xuanwo |
* copy from #372 * fix test and refine ArrowFieldProjector * add comment and rename to make code more clear * refine code * refine RecordBatchProjector * refine error * refine function name --------- Co-authored-by: ZENOTME <[email protected]>
* feat: add EqualityDeleteWriter * WIP: add test cases * fix: move delete schema out of writer * test: add test case for equality delete writer * fix: refactor projector * fix: fix projector * fix: add result * test: add float and double column test for equality delete writer * fmt * fix: compatibility with apache#364 * fix: remove unwrap * fix: minor
* copy from apache#372 * fix test and refine ArrowFieldProjector * add comment and rename to make code more clear * refine code * refine RecordBatchProjector * refine error * refine function name --------- Co-authored-by: ZENOTME <[email protected]>
Close #341
EqualityDeleteWriterrefers tohttps://iceberg.apache.org/spec/#equality-delete-filesThis pr complete the implementation of
EqualityDeleteWriterand its test case