blackmwk commented on code in PR #2916:
URL: https://github.com/apache/iceberg-rust/pull/2916#discussion_r4536361280
##########
crates/iceberg/src/spec/values/serde.rs:
##########
@@ -106,6 +109,7 @@ pub(crate) mod _serde {
Double(f64),
String(String),
Bytes(ByteBuf),
+ Fixed(#[serde(serialize_with =
"apache_avro::serde::fixed::serialize")] ByteBuf),
Review Comment:
No, we can't do avro specific conversion here. Instead, I think the correct
part is to override the
[to_value](https://github.com/apache/iceberg-rust/blob/14d4b5ffe1cc7deede7240c83864f55476da0764/crates/iceberg/src/spec/manifest/writer.rs#L473)
call, e.g. we should add an conversion from RawLiteralEnum to avro values. I'm
fine with keeping `Fixed` variant.
##########
crates/iceberg/src/avro/schema.rs:
##########
@@ -330,6 +329,20 @@ pub(crate) fn avro_fixed_schema(len: usize) ->
Result<AvroSchema> {
}))
}
+/// Build the Avro schema for the Iceberg `uuid` primitive type.
+///
+/// The Iceberg spec maps `uuid` to `{"type": "fixed", "size": 16,
"logicalType": "uuid"}`,
+/// i.e. 16 raw bytes, which is what iceberg-java writes.
+fn avro_uuid_schema() -> Result<AvroSchema> {
Review Comment:
```suggestion
fn avro_uuid_schema() -> AvroSchema {
```
We don't need the result here?
##########
crates/iceberg/src/avro/schema.rs:
##########
@@ -271,9 +272,7 @@ impl SchemaVisitor for SchemaToAvroSchema {
PrimitiveType::TimestampNs => AvroSchema::TimestampNanos,
PrimitiveType::TimestamptzNs => AvroSchema::TimestampNanos,
PrimitiveType::String => AvroSchema::String,
- // TODO(#2913): write the spec's `fixed[16]` uuid. The manifest
reader
- // already accepts both forms.
- PrimitiveType::Uuid => AvroSchema::Uuid(UuidSchema::String),
+ PrimitiveType::Uuid => self.define_once(avro_uuid_schema()?),
Review Comment:
This fixed iceberg uuid -> avro, don't we need to fix avro uuid to iceberg?
--
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]
