CTTY commented on code in PR #1769:
URL: https://github.com/apache/iceberg-rust/pull/1769#discussion_r2457481668
##########
crates/iceberg/src/arrow/record_batch_partition_splitter.rs:
##########
@@ -40,62 +43,83 @@ use crate::{Error, ErrorKind, Result};
pub struct RecordBatchPartitionSplitter {
schema: SchemaRef,
partition_spec: PartitionSpecRef,
- projector: RecordBatchProjector,
+ projector: Option<RecordBatchProjector>,
Review Comment:
I think we can reuse the PartititonValueCalculator
[here](https://github.com/apache/iceberg-rust/blob/915778ebb3245f611ddb0b16076bc7c573a3ea56/crates/integrations/datafusion/src/physical_plan/project.rs#L177).
Are you suggesting that we should have "calculate" and "split" two functions
within the splitter? I think that way it would be hard for people to tell when
should they call `calculate` before calling `split`.
Maybe this would look better?
```rust
impl RecordBatchPartitionSplitter {
pub fn new(
iceberg_schema: SchemaRef,
partition_spec: PartitionSpecRef,
// if some, then calculate the partition value, otherwise use
`_partition`
calculator: Option<PartitionValueCalculator>,
)
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
