You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "ZENOTME (via GitHub)" <gi...@apache.org> on 2023/12/26 04:33:20 UTC
[PR] init writer framework [iceberg-rust]
ZENOTME opened a new pull request, #135:
URL: https://github.com/apache/iceberg-rust/pull/135
related issue: #34
I drafted a writer framework which has been implemented in icelake https://github.com/icelake-io/icelake/issues/243 and proved that it's extensible and flexible. The following is the introduction of this API:
## Target
The writer API is designed to be extensible and flexible. Each writer is decoupled and can be create and config independently. User can:
1. Combine different writer builder to build a writer which have complex write logic. Such as FanoutPartition + DataFileWrite or FanoutPartition + PosititionDeleteFileWrite.
2. Customize the writer and combine it with original writer builder to build a writer which
can process the data in a specific way.
## How it works
There are two kinds of writer and related builder:
1. `IcebergWriter` and `IcebergWriterBuilder`, they are focus on the data process logical.
If you want to support a new data process logical, you need to implement a new `IcebergWriter` and `IcebergWriterBuilder`.
2. `FileWriter` and `FileWriterBuilder`, they are focus on the physical file write.
If you want to support a new physical file format, you need to implement a new `FileWriter` and `FileWriterBuilder`.
The create process of iceberg writer is:
1. Create a `FileWriterBuilder`.
1a. Combine it with other `FileWriterBuilder` to get a new `FileWriterBuilder`.
2. Use FileWriterBuilder to create a `IcebergWriterBuilder`.
2a. Combine it with other `IcebergWriterBuilder` to get a new `IcebergWriterBuilder`.
3. Use `build` function in `IcebergWriterBuilder` to create a `IcebergWriter`.
## Simple Case 1: Create a data file writer using parquet file format.
```
// 1. Create a parquet file writer builder.
let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
// 2. Create a data file writer builder.
let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
// 3. Create a iceberg writer.
let iceberg_writer = DataFileWriterBuilder.build(schema).await?;
iceberg_writer.write(input).await?;
let write_result = iceberg_writer.flush().await?;
```
## Complex Case 2: Create a fanout partition data file writer using parquet file format.
```
// 1. Create a parquet file writer builder.
let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
// 2. Create a data file writer builder.
let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
// 3. Create a fanout partition writer builder.
let fanout_partition_writer_builder = FanoutPartitionWriterBuilder::new(DataFileWriterBuilder, partition_config);
// 4. Create a iceberg writer.
let iceberg_writer = fanout_partition_writer_builder.build(schema).await?;
iceberg_writer.write(input).await?;
let write_result = iceberg_writer.flush().await?;
```
## More case: may be the example in icelake
https://github.com/icelake-io/icelake/blob/949dda79d2ebdfa7ad07e2f88a67d01d7040c181/icelake/tests/insert_tests_v2.rs#L164
Feel free for any suggestions and I'm glad to modify them.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1894811712
> Trait is usually used to abstract things and hide concrete implementation. I don't quite understand the goal of this trait CurrentStatus and when it will be useful?
Sorry, Let me try to illustrate more clearly. Let me use a example to illustrate it:
We may have kinds of partition writer in the future: fanout partition writer will compute the partition and dispatcher them and precompute partition writer will accept the data with the partition value computed before. But both of them will expose the same metrics `PartitionMetrics`. Like following:
```
struct PartitionMetrics {
partition_num: usize
}
struct FanoutPartitionWriter
impl CurrentStatus<PartitionMetrics> for FanoutPartitionWriter
struct PrecomputePartitionWriter
impl CurrentStatus<PartitionMetrics> for PrecomputePartitionWriter
```
And through `CurrentStatus<PartitionMetrics>`, we can write a metrics monitor writer and accept both of them as a input.
```
struct PartitionMetricsMonitorWriter<W: IcebergWriter + CurrentStatus<PartitionMetrics>> {
writer: W
}
```
### Do we have another way?
Yes, we also can do like this:
```
pub trait CurrentFileStatus {
/// Get the current file path.
fn current_file_path(&self) -> String;
/// Get the current file row number.
fn current_row_num(&self) -> usize;
/// Get the current file written size.
fn current_written_size(&self) -> usize;
}
trait CurrentPartitionMetrics {
fn metrics() -> PartitionMetrics;
}
trait CurrentDataFileWriterMetrics {
fn metrics() -> DataFileWriterMetrics
}
```
But I feel that all these traits can be abstract as `pub trait CurrentStatus<T>` so that instead of defining more traits, we can just define more data types in the future. The benefit of this may not be obvious. So both of way look good to me.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1894915527
> > > This design is kind of too complicated for me. I still prefer the approach we used in icelake, and don't overuse too much traits for just saving code.
> >
> >
> > Actually, this design is to solve a problem I meet in icelake now.🤔 In icelake, we use an interface like the following:
> > ```
> > impl FanoutPartitionWriter {
> > fn metrics(&self) -> PartitionMetrics
> > }
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > And we have a corresponding monitor writer:
> > ```
> > struct ParititionMetricsMonitorWriter {
> > writer: FanoutPartitionWriter
> > }
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > But after I introduced a new partition writer: `PrecomputePartitionWriter` , I found that I couldn't reuse the `ParititionMetricsMonitorWriter` because it's coupled with `FanoutPartitionWriter`. **But the metrics(state) they expose are the same**, so I think maybe we need to abstract the interface that expose the metrics(state).
>
> These kinds of refactoring helps avoiding some code duplication, but I don't think this is good abstraction. Good abstraction should be clean and easy to understand. Even your example is not quite convincible, for example, what if the metrics of these two partition writers diverge someday in future, e.g. I want to count the latency of computing these partition calculation?
Sounds reasonable. Let's use the approach before now. We can solve code duplication when it has a good abstraction.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#discussion_r1445893518
##########
crates/iceberg/src/writer/mod.rs:
##########
@@ -0,0 +1,120 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg writer module.
+//!
+//! The writer API is designed to be extensible and flexible. Each writer is decoupled and can be create and config independently. User can:
+//! 1.Combine different writer builder to build a writer which have complex write logic. Such as FanoutPartition + DataFileWrite or FanoutPartition + PosititionDeleteFileWrite.
+//! 2.Customize the writer and combine it with original writer builder to build a writer which
+//! can process the data in a specific way.
+//!
+//! There are two kinds of writer and related builder:
+//! 1. `IcebergWriter` and `IcebergWriterBuilder`, they are focus on the data process logical.
+//! If you want to support a new data process logical, you need to implement a new `IcebergWriter` and `IcebergWriterBuilder`.
+//! 2. `FileWriter` and `FileWriterBuilder`, they are focus on the physical file write.
+//! If you want to support a new physical file format, you need to implement a new `FileWriter` and `FileWriterBuilder`.
+//!
+//! The create process of iceberg writer is:
+//! 1. Create a `FileWriterBuilder`.
+//! 1a. Combine it with other `FileWriterBuilder` to get a new `FileWriterBuilder`.
+//! 2. Use FileWriterBuilder to create a `IcebergWriterBuilder`.
+//! 2a. Combine it with other `IcebergWriterBuilder` to get a new `IcebergWriterBuilder`.
+//! 3. Use `build` function in `IcebergWriterBuilder` to create a `IcebergWriter`.
+//!
+//! # Simple Case 1: Create a data file writer using parquet file format.
+//! # TODO(Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a iceberg writer.
+//! let iceberg_writer = DataFileWriterBuilder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//! ```
+//!
+//! # Complex Case 2: Create a fanout partition data file writer using parquet file format.
+//! # TODO (Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a fanout partition writer builder.
+//! let fanout_partition_writer_builder = FanoutPartitionWriterBuilder::new(DataFileWriterBuilder, partition_config);
+//! // 4. Create a iceberg writer.
+//! let iceberg_writer = fanout_partition_writer_builder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//! ```
+
+use crate::{
+ spec::{DataContentType, Struct},
+ Result,
+};
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+pub mod file_writer;
+
+type DefaultInput = RecordBatch;
+
+/// The builder for iceberg writer.
+#[async_trait::async_trait]
+pub trait IcebergWriterBuilder<I = DefaultInput>: Send + Clone + 'static {
+ /// The associated writer type.
+ type R: IcebergWriter<I>;
+ /// Build the iceberg writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// The iceberg writer used to write data to iceberg table.
+#[async_trait::async_trait]
+pub trait IcebergWriter<I = DefaultInput>: Send + 'static {
+ /// The associated write result type.
+ type R: IcebergWriteResult;
+ /// Write data to iceberg table.
+ async fn write(&mut self, input: I) -> Result<()>;
+ /// Flush the writer and return the write result.
+ async fn flush(&mut self) -> Result<Vec<Self::R>>;
+}
+
+/// The write result of iceberg writer.
+pub trait IcebergWriteResult: Send + Sync + 'static {
Review Comment:
In this case, user can see the type of write_result directly and they can call write_result.build() to get the DataFile.
We also can expose a build interface to make it clear like:
```
trait IcebergWriteResult {
fn build(self) -> Vec<DataFile>
}
```
But I'm rethinking do we really need this abstract🤔
We use generic param here because we can have DeltaResult for delta writer like
```
struct DeltaResult {
data_file: Vec<DataFile>,
pos_delete_file: Vec<DataFile>,
eq_delete_file: Vec<DataFile>
}
```
But I find that it actually can [read the type from DataFile](https://iceberg.apache.org/spec/#schemas-and-data-types:~:text=int%20with%20meaning%3A%200%3A%20DATA%2C%201%3A%20POSITION%20DELETES%2C%202%3A%20EQUALITY%20DELETES) directly. So return `Vec<DataFile>` also can work.
So I think may be we can also discard the IcebergWriteResult, and directly use `write(&mut self, input: I) -> Vec<DataFileBuilder>` to make interface more simple. Because at less for now, in iceberg the return result of writer is always DataFile. I'm not sure should we preserve the extendable for it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1905338963
> > This may make this interface easier to make sense.
>
> The real code could be like:
>
> ```rust
> let writer = FileWriterHelper::new(builder_a)
> .layer(builder_b)
> .layer(builder_c)
> .finish(builder_d)
> .layer(builder_e)
> .layer(builder_f)
> .build()
> .await
> .unwrap();
> ```
>
> Which is harder to understand. The original design like the following is verbose but much easier to read even when we not designed the var names carefully.
>
> ```rust
> let builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
> let builder = DataFileWriterBuilder::new(builder,data_file_writer_config);
> let writer = builder.build(schema).await?;
> ```
Thanks for the feedback. Let's use the original design. It's just an experiment🤣.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME closed pull request #135: init writer framework
URL: https://github.com/apache/iceberg-rust/pull/135
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-2071290816
close by https://github.com/apache/iceberg-rust/pull/275
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1869428917
You can find how to write class diagram uml here: https://plantuml.com/class-diagram
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#discussion_r1454796676
##########
crates/iceberg/src/writer/file_writer/mod.rs:
##########
@@ -0,0 +1,41 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg File Writer
+
+use super::{CurrentFileStatus, DefaultOutput};
+use crate::Result;
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+/// File writer builder trait.
+#[allow(async_fn_in_trait)]
Review Comment:
It's better to use `impl Future` in trait so that we can make sure the future is `Send`:
```rust
fn build(self, schema: &SchemaRef) -> impl Future<Output=Result<Self::R>> + Send;
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#discussion_r1445872438
##########
crates/iceberg/src/writer/file_writer/mod.rs:
##########
@@ -0,0 +1,51 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg File Writer
+
+use super::{CurrentFileStatus, IcebergWriteResult};
+use crate::Result;
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+/// File writer builder trait.
+#[async_trait::async_trait]
+pub trait FileWriterBuilder: Send + Clone + 'static {
+ /// The associated file writer type.
+ type R: FileWriter;
+ /// Build file writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// File writer focus on writing record batch to different physical file format.(Such as parquet. orc)
+#[async_trait::async_trait]
+pub trait FileWriter: Send + 'static + CurrentFileStatus {
+ /// The associated file write result type.
+ type R: FileWriteResult;
+ /// Write record batch to file.
+ async fn write(&mut self, batch: &RecordBatch) -> Result<()>;
+ /// Close file writer.
+ async fn close(self) -> Result<Vec<Self::R>>;
+}
+
+/// File write result.
+pub trait FileWriteResult: Send + 'static {
+ /// The associated iceberg write result type.
+ type R: IcebergWriteResult;
+ /// Convert to iceberg write result.
+ fn to_iceberg_result(self) -> Self::R;
Review Comment:
Yes, I find that we can use IcebergWriteResult directly.🥵
##########
crates/iceberg/src/writer/file_writer/mod.rs:
##########
@@ -0,0 +1,51 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg File Writer
+
+use super::{CurrentFileStatus, IcebergWriteResult};
+use crate::Result;
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+/// File writer builder trait.
+#[async_trait::async_trait]
+pub trait FileWriterBuilder: Send + Clone + 'static {
+ /// The associated file writer type.
+ type R: FileWriter;
+ /// Build file writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// File writer focus on writing record batch to different physical file format.(Such as parquet. orc)
+#[async_trait::async_trait]
+pub trait FileWriter: Send + 'static + CurrentFileStatus {
+ /// The associated file write result type.
+ type R: FileWriteResult;
+ /// Write record batch to file.
+ async fn write(&mut self, batch: &RecordBatch) -> Result<()>;
+ /// Close file writer.
+ async fn close(self) -> Result<Vec<Self::R>>;
+}
+
+/// File write result.
+pub trait FileWriteResult: Send + 'static {
+ /// The associated iceberg write result type.
+ type R: IcebergWriteResult;
+ /// Convert to iceberg write result.
+ fn to_iceberg_result(self) -> Self::R;
Review Comment:
Yes, I find that we can use IcebergWriteResult directly.🥵 Good point!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1869321881
I would suggest to have a class hierarchy to demonstrate the whole picture of this design.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#discussion_r1448357734
##########
crates/iceberg/src/writer/file_writer/mod.rs:
##########
@@ -0,0 +1,51 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg File Writer
+
+use super::{CurrentFileStatus, IcebergWriteResult};
+use crate::Result;
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+/// File writer builder trait.
+#[async_trait::async_trait]
+pub trait FileWriterBuilder: Send + Clone + 'static {
+ /// The associated file writer type.
+ type R: FileWriter;
+ /// Build file writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// File writer focus on writing record batch to different physical file format.(Such as parquet. orc)
+#[async_trait::async_trait]
+pub trait FileWriter: Send + 'static + CurrentFileStatus {
+ /// The associated file write result type.
+ type R: FileWriteResult;
+ /// Write record batch to file.
+ async fn write(&mut self, batch: &RecordBatch) -> Result<()>;
+ /// Close file writer.
+ async fn close(self) -> Result<Vec<Self::R>>;
+}
+
+/// File write result.
+pub trait FileWriteResult: Send + 'static {
+ /// The associated iceberg write result type.
+ type R: IcebergWriteResult;
+ /// Convert to iceberg write result.
+ fn to_iceberg_result(self) -> Self::R;
Review Comment:
Yes, I also think the `FileWriteResult`/`IcebergWriterResult` is a little too complicated. What `FileWriter` returns is just a partial `DataFile`, so a `DataFileBuilder` would be enough.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#discussion_r1445854906
##########
crates/iceberg/src/writer/file_writer/mod.rs:
##########
@@ -0,0 +1,51 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg File Writer
+
+use super::{CurrentFileStatus, IcebergWriteResult};
+use crate::Result;
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+/// File writer builder trait.
+#[async_trait::async_trait]
+pub trait FileWriterBuilder: Send + Clone + 'static {
+ /// The associated file writer type.
+ type R: FileWriter;
+ /// Build file writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// File writer focus on writing record batch to different physical file format.(Such as parquet. orc)
+#[async_trait::async_trait]
+pub trait FileWriter: Send + 'static + CurrentFileStatus {
+ /// The associated file write result type.
+ type R: FileWriteResult;
+ /// Write record batch to file.
+ async fn write(&mut self, batch: &RecordBatch) -> Result<()>;
Review Comment:
The reason why we use `I` in `IcbergWriter<I>` is that for some writers it needs some specific input format rather than RecordBatch. E.g. the PositionDeleteWriter needs a write interface like `write(&mut self, file: &str, position: usize)`. To support this case, we make `IcebergWriter<I>`.
But all IcebergWriter will write the data into a file using FileWriter finally. In this time, IcebergWriter should convert the data into a RecordBatch and write them using FileWriter.
E.g. for PositionDeleteWriter, we will use `write(&mut self, file: &str, position: usize)`, and PositionDeleteWriter will batch these data like
```
---
file, position,
file, position,
...
---
```
, and when it accumulates enough data, it can convert all data into RecordBatch and write them using FileWriter.
For me, RecordBatch is a physical data representation, and `I` is like a logical representation. For data file writer, the logical representation and physical representation is the same. But position delete writer, they are different.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#discussion_r1449911450
##########
crates/iceberg/src/writer/mod.rs:
##########
@@ -0,0 +1,109 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg writer module.
+//!
+//! The writer API is designed to be extensible and flexible. Each writer is decoupled and can be create and config independently. User can:
+//! 1.Combine different writer builder to build a writer which have complex write logic. Such as FanoutPartition + DataFileWrite or FanoutPartition + PosititionDeleteFileWrite.
+//! 2.Customize the writer and combine it with original writer builder to build a writer which
+//! can process the data in a specific way.
+//!
+//! There are two kinds of writer and related builder:
+//! 1. `IcebergWriter` and `IcebergWriterBuilder`, they are focus on the data process logical.
+//! If you want to support a new data process logical, you need to implement a new `IcebergWriter` and `IcebergWriterBuilder`.
+//! 2. `FileWriter` and `FileWriterBuilder`, they are focus on the physical file write.
+//! If you want to support a new physical file format, you need to implement a new `FileWriter` and `FileWriterBuilder`.
+//!
+//! The create process of iceberg writer is:
+//! 1. Create a `FileWriterBuilder`.
+//! 1a. Combine it with other `FileWriterBuilder` to get a new `FileWriterBuilder`.
+//! 2. Use FileWriterBuilder to create a `IcebergWriterBuilder`.
+//! 2a. Combine it with other `IcebergWriterBuilder` to get a new `IcebergWriterBuilder`.
+//! 3. Use `build` function in `IcebergWriterBuilder` to create a `IcebergWriter`.
+//!
+//! # Simple Case 1: Create a data file writer using parquet file format.
+//! # TODO(Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a iceberg writer.
+//! let iceberg_writer = DataFileWriterBuilder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//!
+//! let data_file = write_result.into_iter().map(|builder|builder.build()).collect::<Vec<_>>();
+//! ```
+//!
+//! # Complex Case 2: Create a fanout partition data file writer using parquet file format.
+//! # TODO (Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a fanout partition writer builder.
+//! let fanout_partition_writer_builder = FanoutPartitionWriterBuilder::new(DataFileWriterBuilder, partition_config);
+//! // 4. Create a iceberg writer.
+//! let iceberg_writer = fanout_partition_writer_builder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//!
+//! let data_file = write_result.into_iter().map(|builder|builder.build()).collect::<Vec<_>>();
+//! ```
+
+use crate::{spec::DataFileBuilder, Result};
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+pub mod file_writer;
+
+type DefaultInput = RecordBatch;
+
+/// The builder for iceberg writer.
+#[async_trait::async_trait]
+pub trait IcebergWriterBuilder<I = DefaultInput>: Send + Clone + 'static {
+ /// The associated writer type.
+ type R: IcebergWriter<I>;
+ /// Build the iceberg writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// The iceberg writer used to write data to iceberg table.
+#[async_trait::async_trait]
+pub trait IcebergWriter<I = DefaultInput>: Send + 'static {
+ /// Write data to iceberg table.
+ async fn write(&mut self, input: I) -> Result<()>;
+ /// Flush the writer and return the write result.
+ async fn flush(&mut self) -> Result<Vec<DataFileBuilder>>;
Review Comment:
I think the `Vec<DataFileBuilder>` maybe not type safe. But I also don't think we need a common trait for `IcebergWriterResult`. How about make `IcebergWriter<I, R>`, where `R` is its return type? The reason I propose this is that, when `IcebergWriter` is used, the caller usually know its concrete type, and the most useful use case of this abstraction is that we can have different layers for the writer, which should not change it's input and return type.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1894871318
This design is kind of too complicated for me. I still prefer the approach we used in icelake, and don't overuse too much traits for just saving code.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#discussion_r1449890234
##########
crates/iceberg/src/writer/file_writer/mod.rs:
##########
@@ -0,0 +1,41 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg File Writer
+
+use super::CurrentFileStatus;
+use crate::{spec::DataFileBuilder, Result};
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+/// File writer builder trait.
+#[async_trait::async_trait]
Review Comment:
I think we can remove this now? We have upgraded to 1.75
##########
crates/iceberg/src/writer/mod.rs:
##########
@@ -0,0 +1,109 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg writer module.
+//!
+//! The writer API is designed to be extensible and flexible. Each writer is decoupled and can be create and config independently. User can:
+//! 1.Combine different writer builder to build a writer which have complex write logic. Such as FanoutPartition + DataFileWrite or FanoutPartition + PosititionDeleteFileWrite.
+//! 2.Customize the writer and combine it with original writer builder to build a writer which
+//! can process the data in a specific way.
+//!
+//! There are two kinds of writer and related builder:
+//! 1. `IcebergWriter` and `IcebergWriterBuilder`, they are focus on the data process logical.
+//! If you want to support a new data process logical, you need to implement a new `IcebergWriter` and `IcebergWriterBuilder`.
+//! 2. `FileWriter` and `FileWriterBuilder`, they are focus on the physical file write.
+//! If you want to support a new physical file format, you need to implement a new `FileWriter` and `FileWriterBuilder`.
+//!
+//! The create process of iceberg writer is:
+//! 1. Create a `FileWriterBuilder`.
+//! 1a. Combine it with other `FileWriterBuilder` to get a new `FileWriterBuilder`.
+//! 2. Use FileWriterBuilder to create a `IcebergWriterBuilder`.
+//! 2a. Combine it with other `IcebergWriterBuilder` to get a new `IcebergWriterBuilder`.
+//! 3. Use `build` function in `IcebergWriterBuilder` to create a `IcebergWriter`.
+//!
+//! # Simple Case 1: Create a data file writer using parquet file format.
+//! # TODO(Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a iceberg writer.
+//! let iceberg_writer = DataFileWriterBuilder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//!
+//! let data_file = write_result.into_iter().map(|builder|builder.build()).collect::<Vec<_>>();
+//! ```
+//!
+//! # Complex Case 2: Create a fanout partition data file writer using parquet file format.
+//! # TODO (Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a fanout partition writer builder.
+//! let fanout_partition_writer_builder = FanoutPartitionWriterBuilder::new(DataFileWriterBuilder, partition_config);
+//! // 4. Create a iceberg writer.
+//! let iceberg_writer = fanout_partition_writer_builder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//!
+//! let data_file = write_result.into_iter().map(|builder|builder.build()).collect::<Vec<_>>();
+//! ```
+
+use crate::{spec::DataFileBuilder, Result};
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+pub mod file_writer;
+
+type DefaultInput = RecordBatch;
+
+/// The builder for iceberg writer.
+#[async_trait::async_trait]
Review Comment:
Ditto.
##########
crates/iceberg/src/writer/mod.rs:
##########
@@ -0,0 +1,109 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg writer module.
+//!
+//! The writer API is designed to be extensible and flexible. Each writer is decoupled and can be create and config independently. User can:
+//! 1.Combine different writer builder to build a writer which have complex write logic. Such as FanoutPartition + DataFileWrite or FanoutPartition + PosititionDeleteFileWrite.
+//! 2.Customize the writer and combine it with original writer builder to build a writer which
+//! can process the data in a specific way.
+//!
+//! There are two kinds of writer and related builder:
+//! 1. `IcebergWriter` and `IcebergWriterBuilder`, they are focus on the data process logical.
+//! If you want to support a new data process logical, you need to implement a new `IcebergWriter` and `IcebergWriterBuilder`.
+//! 2. `FileWriter` and `FileWriterBuilder`, they are focus on the physical file write.
+//! If you want to support a new physical file format, you need to implement a new `FileWriter` and `FileWriterBuilder`.
+//!
+//! The create process of iceberg writer is:
+//! 1. Create a `FileWriterBuilder`.
+//! 1a. Combine it with other `FileWriterBuilder` to get a new `FileWriterBuilder`.
+//! 2. Use FileWriterBuilder to create a `IcebergWriterBuilder`.
+//! 2a. Combine it with other `IcebergWriterBuilder` to get a new `IcebergWriterBuilder`.
+//! 3. Use `build` function in `IcebergWriterBuilder` to create a `IcebergWriter`.
+//!
+//! # Simple Case 1: Create a data file writer using parquet file format.
+//! # TODO(Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a iceberg writer.
+//! let iceberg_writer = DataFileWriterBuilder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//!
+//! let data_file = write_result.into_iter().map(|builder|builder.build()).collect::<Vec<_>>();
+//! ```
+//!
+//! # Complex Case 2: Create a fanout partition data file writer using parquet file format.
+//! # TODO (Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a fanout partition writer builder.
+//! let fanout_partition_writer_builder = FanoutPartitionWriterBuilder::new(DataFileWriterBuilder, partition_config);
+//! // 4. Create a iceberg writer.
+//! let iceberg_writer = fanout_partition_writer_builder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//!
+//! let data_file = write_result.into_iter().map(|builder|builder.build()).collect::<Vec<_>>();
+//! ```
+
+use crate::{spec::DataFileBuilder, Result};
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+pub mod file_writer;
+
+type DefaultInput = RecordBatch;
+
+/// The builder for iceberg writer.
+#[async_trait::async_trait]
+pub trait IcebergWriterBuilder<I = DefaultInput>: Send + Clone + 'static {
+ /// The associated writer type.
+ type R: IcebergWriter<I>;
+ /// Build the iceberg writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// The iceberg writer used to write data to iceberg table.
+#[async_trait::async_trait]
+pub trait IcebergWriter<I = DefaultInput>: Send + 'static {
+ /// Write data to iceberg table.
+ async fn write(&mut self, input: I) -> Result<()>;
+ /// Flush the writer and return the write result.
+ async fn flush(&mut self) -> Result<Vec<DataFileBuilder>>;
Review Comment:
I think the `Vec<DataFileBuilder>` maybe not type safe. But I also don't think we need a common trait for `IcebergWriterResult`. How about make `IcebergWriter<I, R>`, where `R` is its return type. The reason I propose this is that, when `IcebergWriter` is used, the caller usually know its concrete type, and the most useful use case of this abstraction is that we can have different layers for the writer, which should not change it's input and return type.
##########
crates/iceberg/src/writer/mod.rs:
##########
@@ -0,0 +1,109 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg writer module.
+//!
+//! The writer API is designed to be extensible and flexible. Each writer is decoupled and can be create and config independently. User can:
+//! 1.Combine different writer builder to build a writer which have complex write logic. Such as FanoutPartition + DataFileWrite or FanoutPartition + PosititionDeleteFileWrite.
+//! 2.Customize the writer and combine it with original writer builder to build a writer which
+//! can process the data in a specific way.
+//!
+//! There are two kinds of writer and related builder:
+//! 1. `IcebergWriter` and `IcebergWriterBuilder`, they are focus on the data process logical.
+//! If you want to support a new data process logical, you need to implement a new `IcebergWriter` and `IcebergWriterBuilder`.
+//! 2. `FileWriter` and `FileWriterBuilder`, they are focus on the physical file write.
+//! If you want to support a new physical file format, you need to implement a new `FileWriter` and `FileWriterBuilder`.
+//!
+//! The create process of iceberg writer is:
+//! 1. Create a `FileWriterBuilder`.
+//! 1a. Combine it with other `FileWriterBuilder` to get a new `FileWriterBuilder`.
+//! 2. Use FileWriterBuilder to create a `IcebergWriterBuilder`.
+//! 2a. Combine it with other `IcebergWriterBuilder` to get a new `IcebergWriterBuilder`.
+//! 3. Use `build` function in `IcebergWriterBuilder` to create a `IcebergWriter`.
+//!
+//! # Simple Case 1: Create a data file writer using parquet file format.
+//! # TODO(Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a iceberg writer.
+//! let iceberg_writer = DataFileWriterBuilder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//!
+//! let data_file = write_result.into_iter().map(|builder|builder.build()).collect::<Vec<_>>();
+//! ```
+//!
+//! # Complex Case 2: Create a fanout partition data file writer using parquet file format.
+//! # TODO (Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a fanout partition writer builder.
+//! let fanout_partition_writer_builder = FanoutPartitionWriterBuilder::new(DataFileWriterBuilder, partition_config);
+//! // 4. Create a iceberg writer.
+//! let iceberg_writer = fanout_partition_writer_builder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//!
+//! let data_file = write_result.into_iter().map(|builder|builder.build()).collect::<Vec<_>>();
+//! ```
+
+use crate::{spec::DataFileBuilder, Result};
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+pub mod file_writer;
+
+type DefaultInput = RecordBatch;
+
+/// The builder for iceberg writer.
+#[async_trait::async_trait]
+pub trait IcebergWriterBuilder<I = DefaultInput>: Send + Clone + 'static {
Review Comment:
Ditto.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1898308549
Let's mark this pr as draft? cc @ZENOTME
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1905325893
I found a new way to make this API can be used like the following:
```
let writer = FileWriterHelper::new(MockFileWriterBuilder)
// build the file writer first
.layer(MockFileWriterBuilder)
.layer(MockFileWriterBuilder)
.finish(MockIcebergWirterBuilder)
// after finish, build the iceberg writer
.layer(MockIcebergWirterBuilder)
.layer(MockIcebergWirterBuilder)
.build()
.await
.unwrap();
```
This may make this interface easier to make sense.🤔 Also feel free to leave any suggestions or confusion, I'm glad to improve it to be easier to understand and use.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1869402828
<img width="956" alt="image" src="https://github.com/apache/iceberg-rust/assets/43447882/7b830b61-e898-49e6-85f4-eb11a47455b0">
I draw a diagram to demonstrate the design of writer. Please let me know if it still has some confused parts.
One point need to note is that the iceberg writer will be separate as **base file writer** and **functional writer logically**.
For base file writer, they are the base format in iceberg, like: data file, position delete file, equality delete file. They take a file writer by which they write their content into a physical format(parquet, orc, ...).
For functional writer, they are the higher level writer to provide more complex write logic. E.g. FanoutPartitionWriter. It act like https://github.com/apache/iceberg/blob/cbb50bfa5ad8cd490e991ff4e1d7bf7c025e3d5d/core/src/main/java/org/apache/iceberg/io/PartitionedFanoutWriter.java#L28. And it can be combined with other iceberg writer.
base file writer and functional writer is only distinguished logically. Both them are `IcebergWriter`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1882539731
> Sorry for being late. I drew a UML diagram, hope it can make the design clearer. Please let me know if there is something confused. 
I think this uml diagram is clear enough to demonstrate the architecture of writers. We have applied same design in icelake, and it works well. Looking forward to hear you thoughts? cc @Fokko @Xuanwo
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#discussion_r1445893518
##########
crates/iceberg/src/writer/mod.rs:
##########
@@ -0,0 +1,120 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg writer module.
+//!
+//! The writer API is designed to be extensible and flexible. Each writer is decoupled and can be create and config independently. User can:
+//! 1.Combine different writer builder to build a writer which have complex write logic. Such as FanoutPartition + DataFileWrite or FanoutPartition + PosititionDeleteFileWrite.
+//! 2.Customize the writer and combine it with original writer builder to build a writer which
+//! can process the data in a specific way.
+//!
+//! There are two kinds of writer and related builder:
+//! 1. `IcebergWriter` and `IcebergWriterBuilder`, they are focus on the data process logical.
+//! If you want to support a new data process logical, you need to implement a new `IcebergWriter` and `IcebergWriterBuilder`.
+//! 2. `FileWriter` and `FileWriterBuilder`, they are focus on the physical file write.
+//! If you want to support a new physical file format, you need to implement a new `FileWriter` and `FileWriterBuilder`.
+//!
+//! The create process of iceberg writer is:
+//! 1. Create a `FileWriterBuilder`.
+//! 1a. Combine it with other `FileWriterBuilder` to get a new `FileWriterBuilder`.
+//! 2. Use FileWriterBuilder to create a `IcebergWriterBuilder`.
+//! 2a. Combine it with other `IcebergWriterBuilder` to get a new `IcebergWriterBuilder`.
+//! 3. Use `build` function in `IcebergWriterBuilder` to create a `IcebergWriter`.
+//!
+//! # Simple Case 1: Create a data file writer using parquet file format.
+//! # TODO(Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a iceberg writer.
+//! let iceberg_writer = DataFileWriterBuilder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//! ```
+//!
+//! # Complex Case 2: Create a fanout partition data file writer using parquet file format.
+//! # TODO (Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a fanout partition writer builder.
+//! let fanout_partition_writer_builder = FanoutPartitionWriterBuilder::new(DataFileWriterBuilder, partition_config);
+//! // 4. Create a iceberg writer.
+//! let iceberg_writer = fanout_partition_writer_builder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//! ```
+
+use crate::{
+ spec::{DataContentType, Struct},
+ Result,
+};
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+pub mod file_writer;
+
+type DefaultInput = RecordBatch;
+
+/// The builder for iceberg writer.
+#[async_trait::async_trait]
+pub trait IcebergWriterBuilder<I = DefaultInput>: Send + Clone + 'static {
+ /// The associated writer type.
+ type R: IcebergWriter<I>;
+ /// Build the iceberg writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// The iceberg writer used to write data to iceberg table.
+#[async_trait::async_trait]
+pub trait IcebergWriter<I = DefaultInput>: Send + 'static {
+ /// The associated write result type.
+ type R: IcebergWriteResult;
+ /// Write data to iceberg table.
+ async fn write(&mut self, input: I) -> Result<()>;
+ /// Flush the writer and return the write result.
+ async fn flush(&mut self) -> Result<Vec<Self::R>>;
+}
+
+/// The write result of iceberg writer.
+pub trait IcebergWriteResult: Send + Sync + 'static {
Review Comment:
In this case, user can see the type of write_result directly and they can call write_result.build() to get the DataFile.
We also can expose a build interface to make it clear like:
```
trait IcebergWriteResult {
fn build(self) -> Vec<DataFile>
}
```
But I'm rethinking do we really need this abstract🤔
We use generic param here because we can have DeltaResult for delta writer like
```
struct DeltaResult {
data_file: Vec<DataFileBuilder>,
pos_delete_file: Vec<DataFileBuilder>,
eq_delete_file: Vec<DataFileBuilder>
}
```
But I find that it actually can [read the type from DataFile](https://iceberg.apache.org/spec/#schemas-and-data-types:~:text=int%20with%20meaning%3A%200%3A%20DATA%2C%201%3A%20POSITION%20DELETES%2C%202%3A%20EQUALITY%20DELETES) directly. So return `Vec<DataFileBuilder>` also can work.
So I think may be we can also discard the IcebergWriteResult, and directly use `write(&mut self, input: I) -> Vec<DataFileBuilder>` to make interface more simple. Because at less for now, in iceberg the return result of writer is always DataFile. I'm not sure should we preserve the extendable for it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1894895024
> > This design is kind of too complicated for me. I still prefer the approach we used in icelake, and don't overuse too much traits for just saving code.
>
> Actually, this design is to solve a problem I meet in icelake now.🤔 In icelake, we use an interface like the following:
>
> ```
> impl FanoutPartitionWriter {
> fn metrics(&self) -> PartitionMetrics
> }
> ```
>
> And we have a corresponding monitor writer:
>
> ```
> struct ParititionMetricsMonitorWriter {
> writer: FanoutPartitionWriter
> }
> ```
>
> But after I introduced a new partition writer: `PrecomputePartitionWriter` , I found that I couldn't reuse the `ParititionMetricsMonitorWriter` because it's coupled with `FanoutPartitionWriter`. **But the metrics(state) they expose are the same**, so I think maybe we need to abstract the interface that expose the metrics(state).
These kinds of refactoring helps avoiding some code duplication, but I don't think this is good abstraction. Good abstraction should be clean and easy to understand. Even your example is not quite convincible, for example, what if the metrics of these two partition writers diverge someday in future, e.g. I want to count the latency of computing these partition calculation?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1895000380
cc @Fokko @Xuanwo PTAL
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1895352135
> And we can work on the FileWriter part first if it looks good. How do you think? @Xuanwo @Fokko
+1
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1894775565
> Hi, I find that we can modify
>
> ```
> /// The current file status of iceberg writer. It implement for the writer which write a single
> /// file.
> pub trait CurrentFileStatus {
> /// Get the current file path.
> fn current_file_path(&self) -> String;
> /// Get the current file row number.
> fn current_row_num(&self) -> usize;
> /// Get the current file written size.
> fn current_written_size(&self) -> usize;
> }
> ```
>
> as
>
> ```
> pub trait CurrentStatus<T> {
> fn current_status(&self) -> T
> }
>
> impl CurrentStatus<CurrentFileStatus> for DataFileWriter {
> ...
> }
>
> impl CurrentStatus<DataFileMetrics> for DataFileWriter {
> ...
> }
> ```
>
> The following traits can also help export metrics in the future.
Trait is usually used to abstract things and hide concrete implementation. I don't quite understand the goal of this trait `CurrentStatus` and when it will be useful?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1893177443
Hi, I find that we can modify
```
/// The current file status of iceberg writer. It implement for the writer which write a single
/// file.
pub trait CurrentFileStatus {
/// Get the current file path.
fn current_file_path(&self) -> String;
/// Get the current file row number.
fn current_row_num(&self) -> usize;
/// Get the current file written size.
fn current_written_size(&self) -> usize;
}
```
as
```
pub trait CurrentStatus<T> {
fn current_status(&self) -> T
}
impl CurrentStatus<CurrentFileStatus> for DataFileWriter {
...
}
impl CurrentStatus<DataFileMetrics> for DataFileWriter {
...
}
```
The following traits can also help export metrics in the future.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1905335724
> This may make this interface easier to make sense.
The real code could be like:
```rust
let writer = FileWriterHelper::new(builder_a)
.layer(builder_b)
.layer(builder_c)
.finish(builder_d)
.layer(builder_e)
.layer(builder_f)
.build()
.await
.unwrap();
```
Which is harder to understand. The original design like the following is verbose but much easier to read even when we not designed the var names carefully.
```rust
let builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
let builder = DataFileWriterBuilder::new(builder,data_file_writer_config);
let writer = builder.build(schema).await?;
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1880805433
Sorry for being late. I drew a UML diagram, hope it can make the design clearer. Please let me know if there is something confused.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1869426878
I have worked together with @ZENOTME to implement such a class hierarchy in [icelake](https://github.com/icelake-io/icelake), so I'm familiar with this. I feel that maybe a UML diagram can describe this better? For example, the one to many relationship, the inheritance.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1894877728
> This design is kind of too complicated for me. I still prefer the approach we used in icelake, and don't overuse too much traits for just saving code.
Actually, this design is to solve a problem I meet in icelake now.🤔 In icelake, we use an interface like the following:
```
impl FanoutPartitionWriter {
fn metrics(&self) -> PartitionMetrics
}
```
And we have a corresponding monitor writer:
```
struct ParititionMetricsMonitorWriter {
writer: FanoutPartitionWriter
}
```
But after I introduced a new partition writer: `PrecomputePartitionWriter` , I found that I couldn't reuse the `ParititionMetricsMonitorWriter` because it's coupled with `FanoutPartitionWriter`. **But the metrics(state) they expose are the same**, so I think maybe we need to abstract the interface that expose the metrics(state).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1895304460
I feel that for this writer framework, we may need more discussion, so I can separate this framework as `IcebergWriter` and `FileWriter` parts.
The `IcebergWriter` part is about how to organize our writers, it's the core and more complicated.
The `FileWriter` part is an abstraction for kinds of file formats like `parquet`, and `orc`. It's more simple.
And we can work on the `FileWriter` part first if it looks good. How do you think? @Xuanwo @Fokko
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1895313954
> And we can work on the `FileWriter` part first if it looks good. How do you think? @Xuanwo @Fokko
LGTM! It's good to merge things in small chunks and polish them during the real usage.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#discussion_r1445761472
##########
crates/iceberg/src/writer/mod.rs:
##########
@@ -0,0 +1,120 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg writer module.
+//!
+//! The writer API is designed to be extensible and flexible. Each writer is decoupled and can be create and config independently. User can:
+//! 1.Combine different writer builder to build a writer which have complex write logic. Such as FanoutPartition + DataFileWrite or FanoutPartition + PosititionDeleteFileWrite.
+//! 2.Customize the writer and combine it with original writer builder to build a writer which
+//! can process the data in a specific way.
+//!
+//! There are two kinds of writer and related builder:
+//! 1. `IcebergWriter` and `IcebergWriterBuilder`, they are focus on the data process logical.
+//! If you want to support a new data process logical, you need to implement a new `IcebergWriter` and `IcebergWriterBuilder`.
+//! 2. `FileWriter` and `FileWriterBuilder`, they are focus on the physical file write.
+//! If you want to support a new physical file format, you need to implement a new `FileWriter` and `FileWriterBuilder`.
+//!
+//! The create process of iceberg writer is:
+//! 1. Create a `FileWriterBuilder`.
+//! 1a. Combine it with other `FileWriterBuilder` to get a new `FileWriterBuilder`.
+//! 2. Use FileWriterBuilder to create a `IcebergWriterBuilder`.
+//! 2a. Combine it with other `IcebergWriterBuilder` to get a new `IcebergWriterBuilder`.
+//! 3. Use `build` function in `IcebergWriterBuilder` to create a `IcebergWriter`.
+//!
+//! # Simple Case 1: Create a data file writer using parquet file format.
+//! # TODO(Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a iceberg writer.
+//! let iceberg_writer = DataFileWriterBuilder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//! ```
+//!
+//! # Complex Case 2: Create a fanout partition data file writer using parquet file format.
+//! # TODO (Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a fanout partition writer builder.
+//! let fanout_partition_writer_builder = FanoutPartitionWriterBuilder::new(DataFileWriterBuilder, partition_config);
+//! // 4. Create a iceberg writer.
+//! let iceberg_writer = fanout_partition_writer_builder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//! ```
+
+use crate::{
+ spec::{DataContentType, Struct},
+ Result,
+};
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+pub mod file_writer;
+
+type DefaultInput = RecordBatch;
+
+/// The builder for iceberg writer.
+#[async_trait::async_trait]
+pub trait IcebergWriterBuilder<I = DefaultInput>: Send + Clone + 'static {
+ /// The associated writer type.
+ type R: IcebergWriter<I>;
+ /// Build the iceberg writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// The iceberg writer used to write data to iceberg table.
+#[async_trait::async_trait]
+pub trait IcebergWriter<I = DefaultInput>: Send + 'static {
+ /// The associated write result type.
+ type R: IcebergWriteResult;
+ /// Write data to iceberg table.
+ async fn write(&mut self, input: I) -> Result<()>;
+ /// Flush the writer and return the write result.
+ async fn flush(&mut self) -> Result<Vec<Self::R>>;
+}
+
+/// The write result of iceberg writer.
+pub trait IcebergWriteResult: Send + Sync + 'static {
+ /// Set the content type of the write result.
+ fn set_content(&mut self, content: DataContentType) -> &mut Self;
+ /// Set the equality ids of the write result.
+ fn set_equality_ids(&mut self, equality_ids: Vec<i32>) -> &mut Self;
+ /// Set the partition of the write result.
+ fn set_partition(&mut self, partition_value: Struct) -> &mut Self;
+}
+
+/// The current file status of iceberg writer. It implement for the writer which write a single
+/// file.
+pub trait CurrentFileStatus {
Review Comment:
Should we introduce a separate trait for this, or would it be better to add a `status(&self) -> FileStatus` method?
Also, implementing APIs that require `&self` complicates making our type `Sync`, as it necessitates additional `Arc<Mutex>` wrappers. Could we have them accept `&mut self` instead? We don't have any use cases for browsing files, do we?
##########
crates/iceberg/src/writer/mod.rs:
##########
@@ -0,0 +1,120 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg writer module.
+//!
+//! The writer API is designed to be extensible and flexible. Each writer is decoupled and can be create and config independently. User can:
+//! 1.Combine different writer builder to build a writer which have complex write logic. Such as FanoutPartition + DataFileWrite or FanoutPartition + PosititionDeleteFileWrite.
+//! 2.Customize the writer and combine it with original writer builder to build a writer which
+//! can process the data in a specific way.
+//!
+//! There are two kinds of writer and related builder:
+//! 1. `IcebergWriter` and `IcebergWriterBuilder`, they are focus on the data process logical.
+//! If you want to support a new data process logical, you need to implement a new `IcebergWriter` and `IcebergWriterBuilder`.
+//! 2. `FileWriter` and `FileWriterBuilder`, they are focus on the physical file write.
+//! If you want to support a new physical file format, you need to implement a new `FileWriter` and `FileWriterBuilder`.
+//!
+//! The create process of iceberg writer is:
+//! 1. Create a `FileWriterBuilder`.
+//! 1a. Combine it with other `FileWriterBuilder` to get a new `FileWriterBuilder`.
+//! 2. Use FileWriterBuilder to create a `IcebergWriterBuilder`.
+//! 2a. Combine it with other `IcebergWriterBuilder` to get a new `IcebergWriterBuilder`.
+//! 3. Use `build` function in `IcebergWriterBuilder` to create a `IcebergWriter`.
+//!
+//! # Simple Case 1: Create a data file writer using parquet file format.
+//! # TODO(Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a iceberg writer.
+//! let iceberg_writer = DataFileWriterBuilder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//! ```
+//!
+//! # Complex Case 2: Create a fanout partition data file writer using parquet file format.
+//! # TODO (Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a fanout partition writer builder.
+//! let fanout_partition_writer_builder = FanoutPartitionWriterBuilder::new(DataFileWriterBuilder, partition_config);
+//! // 4. Create a iceberg writer.
+//! let iceberg_writer = fanout_partition_writer_builder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//! ```
+
+use crate::{
+ spec::{DataContentType, Struct},
+ Result,
+};
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+pub mod file_writer;
+
+type DefaultInput = RecordBatch;
+
+/// The builder for iceberg writer.
+#[async_trait::async_trait]
+pub trait IcebergWriterBuilder<I = DefaultInput>: Send + Clone + 'static {
+ /// The associated writer type.
+ type R: IcebergWriter<I>;
+ /// Build the iceberg writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// The iceberg writer used to write data to iceberg table.
+#[async_trait::async_trait]
+pub trait IcebergWriter<I = DefaultInput>: Send + 'static {
+ /// The associated write result type.
+ type R: IcebergWriteResult;
+ /// Write data to iceberg table.
+ async fn write(&mut self, input: I) -> Result<()>;
+ /// Flush the writer and return the write result.
+ async fn flush(&mut self) -> Result<Vec<Self::R>>;
+}
+
+/// The write result of iceberg writer.
+pub trait IcebergWriteResult: Send + Sync + 'static {
Review Comment:
The fact that the result only reveals a `set_xxx` API is somewhat confusing. How are users supposed to utilize these APIs?
```rust
// 4. Create a iceberg writer.
let iceberg_writer = fanout_partition_writer_builder.build(schema).await?;
iceberg_writer.write(input).await?;
let write_result = iceberg_writer.flush().await?;
```
How `write_result` will be used?
##########
crates/iceberg/src/writer/file_writer/mod.rs:
##########
@@ -0,0 +1,51 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg File Writer
+
+use super::{CurrentFileStatus, IcebergWriteResult};
+use crate::Result;
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+/// File writer builder trait.
+#[async_trait::async_trait]
+pub trait FileWriterBuilder: Send + Clone + 'static {
+ /// The associated file writer type.
+ type R: FileWriter;
+ /// Build file writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// File writer focus on writing record batch to different physical file format.(Such as parquet. orc)
+#[async_trait::async_trait]
+pub trait FileWriter: Send + 'static + CurrentFileStatus {
+ /// The associated file write result type.
+ type R: FileWriteResult;
+ /// Write record batch to file.
+ async fn write(&mut self, batch: &RecordBatch) -> Result<()>;
+ /// Close file writer.
+ async fn close(self) -> Result<Vec<Self::R>>;
+}
+
+/// File write result.
+pub trait FileWriteResult: Send + 'static {
+ /// The associated iceberg write result type.
+ type R: IcebergWriteResult;
+ /// Convert to iceberg write result.
+ fn to_iceberg_result(self) -> Self::R;
Review Comment:
What transform will we do in this trait? Can we return `IcebergWriteResult` directly?
##########
crates/iceberg/src/writer/file_writer/mod.rs:
##########
@@ -0,0 +1,51 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg File Writer
+
+use super::{CurrentFileStatus, IcebergWriteResult};
+use crate::Result;
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+/// File writer builder trait.
+#[async_trait::async_trait]
+pub trait FileWriterBuilder: Send + Clone + 'static {
+ /// The associated file writer type.
+ type R: FileWriter;
+ /// Build file writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// File writer focus on writing record batch to different physical file format.(Such as parquet. orc)
+#[async_trait::async_trait]
+pub trait FileWriter: Send + 'static + CurrentFileStatus {
+ /// The associated file write result type.
+ type R: FileWriteResult;
+ /// Write record batch to file.
+ async fn write(&mut self, batch: &RecordBatch) -> Result<()>;
Review Comment:
What's the relation of `RecordBatch` with the `I` in `IcebergWriter<I>`?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#discussion_r1445904375
##########
crates/iceberg/src/writer/mod.rs:
##########
@@ -0,0 +1,120 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Iceberg writer module.
+//!
+//! The writer API is designed to be extensible and flexible. Each writer is decoupled and can be create and config independently. User can:
+//! 1.Combine different writer builder to build a writer which have complex write logic. Such as FanoutPartition + DataFileWrite or FanoutPartition + PosititionDeleteFileWrite.
+//! 2.Customize the writer and combine it with original writer builder to build a writer which
+//! can process the data in a specific way.
+//!
+//! There are two kinds of writer and related builder:
+//! 1. `IcebergWriter` and `IcebergWriterBuilder`, they are focus on the data process logical.
+//! If you want to support a new data process logical, you need to implement a new `IcebergWriter` and `IcebergWriterBuilder`.
+//! 2. `FileWriter` and `FileWriterBuilder`, they are focus on the physical file write.
+//! If you want to support a new physical file format, you need to implement a new `FileWriter` and `FileWriterBuilder`.
+//!
+//! The create process of iceberg writer is:
+//! 1. Create a `FileWriterBuilder`.
+//! 1a. Combine it with other `FileWriterBuilder` to get a new `FileWriterBuilder`.
+//! 2. Use FileWriterBuilder to create a `IcebergWriterBuilder`.
+//! 2a. Combine it with other `IcebergWriterBuilder` to get a new `IcebergWriterBuilder`.
+//! 3. Use `build` function in `IcebergWriterBuilder` to create a `IcebergWriter`.
+//!
+//! # Simple Case 1: Create a data file writer using parquet file format.
+//! # TODO(Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a iceberg writer.
+//! let iceberg_writer = DataFileWriterBuilder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//! ```
+//!
+//! # Complex Case 2: Create a fanout partition data file writer using parquet file format.
+//! # TODO (Implement this example)
+//! ```ignore
+//! // 1. Create a parquet file writer builder.
+//! let parquet_writer_builder = ParquetFileWriterBuilder::new(parquet_file_writer_config);
+//! // 2. Create a data file writer builder.
+//! let DataFileWriterBuilder = DataFileWriterBuilder::new(parquet_writer_builder,data_file_writer_config);
+//! // 3. Create a fanout partition writer builder.
+//! let fanout_partition_writer_builder = FanoutPartitionWriterBuilder::new(DataFileWriterBuilder, partition_config);
+//! // 4. Create a iceberg writer.
+//! let iceberg_writer = fanout_partition_writer_builder.build(schema).await?;
+//!
+//! iceberg_writer.write(input).await?;
+//!
+//! let write_result = iceberg_writer.flush().await?;
+//! ```
+
+use crate::{
+ spec::{DataContentType, Struct},
+ Result,
+};
+use arrow_array::RecordBatch;
+use arrow_schema::SchemaRef;
+
+pub mod file_writer;
+
+type DefaultInput = RecordBatch;
+
+/// The builder for iceberg writer.
+#[async_trait::async_trait]
+pub trait IcebergWriterBuilder<I = DefaultInput>: Send + Clone + 'static {
+ /// The associated writer type.
+ type R: IcebergWriter<I>;
+ /// Build the iceberg writer.
+ async fn build(self, schema: &SchemaRef) -> Result<Self::R>;
+}
+
+/// The iceberg writer used to write data to iceberg table.
+#[async_trait::async_trait]
+pub trait IcebergWriter<I = DefaultInput>: Send + 'static {
+ /// The associated write result type.
+ type R: IcebergWriteResult;
+ /// Write data to iceberg table.
+ async fn write(&mut self, input: I) -> Result<()>;
+ /// Flush the writer and return the write result.
+ async fn flush(&mut self) -> Result<Vec<Self::R>>;
+}
+
+/// The write result of iceberg writer.
+pub trait IcebergWriteResult: Send + Sync + 'static {
+ /// Set the content type of the write result.
+ fn set_content(&mut self, content: DataContentType) -> &mut Self;
+ /// Set the equality ids of the write result.
+ fn set_equality_ids(&mut self, equality_ids: Vec<i32>) -> &mut Self;
+ /// Set the partition of the write result.
+ fn set_partition(&mut self, partition_value: Struct) -> &mut Self;
+}
+
+/// The current file status of iceberg writer. It implement for the writer which write a single
+/// file.
+pub trait CurrentFileStatus {
Review Comment:
> Should we introduce a separate trait for this, or would it be better to add a status(&self) -> FileStatus method?
The reason e introduce it as a separate trait is that for some writer which may write multiple file at once, so we should not call this trait for them. E.g fanout partition writer.
> Could we have them accept &mut self instead?
This interface is read-only. Make it `&mut self` seems weird for me.🤔 And I wonder in which case the user want to use `Arc<Writer>`? Seems it's a rare example for me.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] init writer framework [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1888422180
To make the design more simple, I have removed the **WriteResult trait.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org