li3zhi4 commented on PR #11633:
URL: https://github.com/apache/seatunnel/pull/11633#issuecomment-5174817259
Thanks @DanielLeens for the thorough re-review and for withdrawing the
stacked-branch concern — no apology needed. All four findings are addressed in
the new head `423ac992de`:
**Issue 1 (nested ROW consulting top-level defaults):** `createRowConverter`
now takes an explicit `Column[]` parameter: the root row converter receives the
real column array, while the nested-ROW call site (`case ROW`) passes `null`,
so defaults are scoped to the top level only. New test
`testDefaultValueNotAppliedToNestedRowFields` locks this in
(`{"id":1,"address":{"zip":100}}` → `address.city` stays `null`, not the
top-level default).
**Issue 2 (explicit null under `failOnMissingField = true`):**
`convertField` now treats an explicit JSON `null` as default-eligible but not
as "missing": `failOnMissingField` only throws when the field is genuinely
absent (`field == null`); a `NullNode` without a default keeps returning `null`
as before, and with a default applies it. New test
`testExplicitNullWithFailOnMissingField` covers both cases plus the
still-throwing missing-field path.
**Issue 3 (physical-column index alignment):** `JsonDeserializationSchema`
now builds the `Column[]` with `filter(Column::isPhysical)`, matching
`AbstractSchema#toPhysicalRowDataType` so positional indexing cannot drift when
metadata/computed columns exist.
**Issue 4 (per-record default re-conversion):** each column default is
pre-converted once at converter construction into a `defaultValues[]` array;
the hot path just reads the cached value, and misconfigured defaults fail fast
at job start.
Also included: e2e now clears the result list inside the Awaitility retry
and closes the Kafka consumer in `tearDown()`, and the EN/ZH docs note that
`defaultValue` takes precedence over `failOnMissingField` and explicit `null`
is never treated as missing.
Verification: `JsonDefaultValueTest` 10/10, full `seatunnel-format-json`
module 56/56 green, `spotless:apply` clean. Branch is up to date with `dev`
(behind 0).
--
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]
