viirya commented on PR #2635:
URL: https://github.com/apache/iceberg-rust/pull/2635#issuecomment-4848247496
Thanks @kevinjqliu — addressed the feedback in e845ae08:
- **Test readability / ordering.** Extracted the evolved schema into
`schema_with_added_nested_columns()` and reordered the test body so it reads
top-to-bottom as the story you described: (1) the old data file with only `id`,
(2) read it against the evolved schema, (3) expect the added list/map/struct
columns as typed NULLs. The big nested-schema definition no longer sits in
front of the actual logic.
- **Null shape assertion.** Added an assertion that the all-NULL struct
still carries its full nested shape — child fields `a` and the nested list `ys`
— rather than collapsing to an empty struct. This is exactly the property the
`new_null_array` approach guarantees over an enumerate-each-type fix (and what
@mbutrovich noted).
- **`new_null_array` for timestamp/decimal (your question on the `Some`
timestamp arm).** Confirmed: `new_null_array` builds from the `DataType`, and
timezone is part of `DataType::Timestamp(unit, tz)` and precision/scale part of
`Decimal128`, so the NULL array carries the exact type the old per-type
branches produced. I verified locally:
```
new_null_array(Timestamp(Nanosecond, Some("+08:00")), 1) -> dt =
Timestamp(Nanosecond, Some("+08:00")), null
new_null_array(Decimal128(20, 4), 1) -> dt =
Decimal128(20, 4), null
```
(REE wrapping isn't preserved, but the NULL-fill path uses the plain
schema type — REE is only applied on the constant/`Some` path — so no
regression.)
- **Comments.** Trimmed the over-explanatory comments on both NULL-fill
paths and noted the literal `1` is `new_null_array`'s row count.
- **Refactoring `create_primitive_array_repeated` (the repetition you
spotted) and the `create_primitive_*` rename / `Literal` widening for V3
default values** — agreed these are worth doing, keeping them as the follow-up
@advancedxy and I discussed above so this PR stays the focused #2618 fix.
--
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]
