jecsand838 commented on code in PR #8492:
URL: https://github.com/apache/arrow-rs/pull/8492#discussion_r2389932806
##########
arrow-avro/src/codec.rs:
##########
@@ -1756,84 +1818,97 @@ impl<'a> Maker<'a> {
ensure_names_match(
"Record",
writer_record.name,
+ writer_record.namespace,
&writer_record.aliases,
reader_record.name,
+ reader_record.namespace,
&reader_record.aliases,
)?;
let writer_ns = writer_record.namespace.or(namespace);
let reader_ns = reader_record.namespace.or(namespace);
let reader_md = reader_record.attributes.field_metadata();
- let writer_index_map: HashMap<&str, usize> = writer_record
- .fields
- .iter()
- .enumerate()
- .map(|(idx, wf)| (wf.name, idx))
- .collect();
+ // Build writer lookup and ambiguous alias set.
+ let (writer_lookup, ambiguous_writer_aliases) =
Self::build_writer_lookup(writer_record);
Review Comment:
**(Related to bug 1) Alias resolution + defaults:** These changes makes
alias matching spec‑compliant and handles defaults correctly when a reader
field isn’t present in the writer schema.
* `full_name_set(..)` builds canonical full names for a type (name +
namespace) and its aliases, and names_match(..) now compares these sets rather
than raw strings — this allows both fully‑qualified and namespace‑relative
aliases to match per the Avro spec.
* `ensure_names_match(..)` is updated to pass both writer and reader
namespaces/aliases to names_match(..).
* In `Maker::resolve_records(..)` we now pre-build a writer lookup that
includes aliases, detect ambiguous writer aliases, and, in strict_mode, error
out if a reader alias would map to multiple writer fields.
When a reader field has no writer match, we now require an explicit default
or (if the reader union is `["null", T]`) synthesize default `null` for the
union‑first branch.
--
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]
