c-thiel commented on code in PR #2188:
URL: https://github.com/apache/iceberg-rust/pull/2188#discussion_r3593297056
##########
crates/iceberg/src/avro/schema.rs:
##########
@@ -37,18 +37,55 @@ const FIELD_ID_PROP: &str = "field-id";
const KEY_ID: &str = "key-id";
const VALUE_ID: &str = "value-id";
const MAP_LOGICAL_TYPE: &str = "map";
+const VARIANT_LOGICAL_TYPE: &str = "variant";
// This const may better to maintain in avro-rs.
const LOGICAL_TYPE: &str = "logicalType";
struct SchemaToAvroSchema {
schema: String,
+ // Stack of enclosing field ids, used to derive unique record names for
+ // structural types (e.g. variant) — mirrors Java TypeToSchema.fieldIds.
+ field_ids: Vec<i32>,
}
type AvroSchemaOrField = Either<AvroSchema, AvroRecordField>;
impl SchemaVisitor for SchemaToAvroSchema {
type T = AvroSchemaOrField;
+ fn before_struct_field(&mut self, field: &NestedFieldRef) -> Result<()> {
Review Comment:
You're right to question it — I've dropped that change. The `field_ids`
tracking existed because variant→Avro is a *record* (metadata+value), and Avro
record names must be unique within a schema, so multiple variant columns needed
distinct names (`r{id}`) — which a primitive's bare scalar schema never needs.
But that's write-path machinery, and it's out of scope here: `variant()` now
just returns `FeatureUnsupported`, so the whole `field_ids`/before-after-field
bookkeeping is gone. Avro encoding lands with the write follow-up.
--
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]
