ZENOTME commented on code in PR #741:
URL: https://github.com/apache/iceberg-rust/pull/741#discussion_r1868720714
##########
crates/iceberg/src/writer/file_writer/mod.rs:
##########
@@ -37,11 +37,11 @@ pub trait FileWriterBuilder<O = DefaultOutput>: Send +
Clone + 'static {
/// The associated file writer type.
type R: FileWriter<O>;
/// Build file writer.
- fn build(self) -> impl Future<Output = Result<Self::R>> + Send;
+ fn build(self, schema: SchemaRef) -> impl Future<Output = Result<Self::R>>
+ Send;
Review Comment:
Let's use original process of creating equality delete writer as example:
```
let equality_ids = vec![0_i32, 8];
let equality_config =
EqualityDeleteWriterConfig::new(equality_ids, Arc::new(schema),
None).unwrap();
let delete_schema =
arrow_schema_to_schema(equality_config.projected_arrow_schema_ref()).unwrap();
let projector = equality_config.projector.clone();
// prepare writer
let pb = ParquetWriterBuilder::new(
WriterProperties::builder().build(),
Arc::new(delete_schema),
file_io.clone(),
location_gen,
file_name_gen,
);
let mut equality_delete_writer =
EqualityDeleteFileWriterBuilder::new(pb)
.build(equality_config)
.await?;
```
We need to get the projected schema from EqualityDeleteWriterConfig.
Otherwise, the user have to project the schema by themselves. After remove
writer config, we need to other thing help us to project the schema and let
user get from it.
One pattern I realized for now is that the schema of file writer is always
rely on the iceberg writer which use it. So I think move it to the build
function can make thing looks more simple.🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]