mbutrovich commented on code in PR #54:
URL: https://github.com/apache/datafusion-iceberg/pull/54#discussion_r4232862252
##########
crates/datafusion/src/physical_plan/write.rs:
##########
@@ -103,9 +106,24 @@ impl IcebergWriteExec {
))
}
- // Create a record batch with serialized data files
- fn make_result_batch(data_files: Vec<String>) -> Result<RecordBatch> {
- let files_array = Arc::new(StringArray::from(data_files)) as ArrayRef;
+ // Each row holds one Avro container, amortizing its schema over all files
from a task.
+ fn make_result_batch(
+ data_files: Vec<DataFile>,
+ partition_type: &StructType,
+ format_version: FormatVersion,
+ ) -> Result<RecordBatch> {
+ if data_files.is_empty() {
+ return Ok(RecordBatch::new_empty(Self::make_result_schema()));
+ }
+
+ let mut buffer = Vec::new();
+ write_data_files_to_avro(&mut buffer, data_files, partition_type,
format_version)
Review Comment:
Should we bump the iceberg-rust rev to
[`5b82ab99a`](https://github.com/apache/iceberg-rust/commit/5b82ab99aeaf50bf4f5b3b776ac9cd6380a5f3e3)
(apache/iceberg-rust#3354) before this lands? At the current rev, `8cb2adedd`,
[`avro_fixed_schema`](https://github.com/apache/iceberg-rust/blob/8cb2adeddb4f8da1ca7bd86ca303337f011c12e2/crates/iceberg/src/avro/schema.rs#L301-L310)
defines the [named Avro
type](https://avro.apache.org/docs/1.12.0/specification/#names) `fixed_{L}`
once per field. For a partition with two `fixed[4]` fields, `make_result_batch`
returns `Two named schema defined for same fullname: fixed_4.`, while the JSON
path handled that partition. #3354 defines each named type once. It also moves
iceberg-rust to `apache-avro` 0.22 and reads manifest entries without building
`apache_avro::Value`s, which made `Manifest::parse_avro` about 5.7x faster in
its benchmark. Scan planning loads manifests through that reader.
--
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]
