SEZ9 commented on PR #11633: URL: https://github.com/apache/seatunnel/pull/11633#issuecomment-5225133456
Thanks @li3zhi4 for the quick turnaround — the fixes for the three findings look right in principle: 1. **Nested ROW scoping** — passing the real `Column[]` only to the root converter and `null` at the nested-ROW call site is the right fix, and `testDefaultValueNotAppliedToNestedRowFields` locking in the `address.city` behavior is exactly the regression guard I wanted. 2. **Explicit `null` vs. missing under `failOnMissingField`** — distinguishing `field == null` (throw) from `NullNode` (default-eligible, otherwise `null` as before) matches the semantics we discussed, and `testExplicitNullWithFailOnMissingField` covers all three paths. 3. **Physical-column alignment** — filtering with `Column::isPhysical` to mirror `AbstractSchema#toPhysicalRowDataType` closes the index-drift risk. Your comment appears to have been cut off after "**I...**", so I can't see how you handled the fourth point. To restate the remaining ask: **default values are currently normalized via `JsonUtils.toJsonNode()` + the field converter on every record**. Please pre-convert each configured default once at converter construction time and cache the result. This has two benefits: - avoids per-record serialization overhead on the hot path, and - fails fast at job startup with a clear error if a configured `defaultValue` is incompatible with the column type, instead of surfacing (or silently misbehaving) mid-stream. Two smaller items before I approve: - Since applying defaults on explicit `null` is a behavior change for existing jobs that already have `defaultValue` configured, please add a short note in the EN/ZH schema-feature docs (and we should flag it in the release notes) so users aren't surprised. - Please confirm the full `seatunnel-format-json` suite and `KafkaJsonDefaultValueIT` are green on the new head, since the last push reset CI. Once the construction-time pre-conversion is in, I'll do a final pass. Nice work on this. <!-- streview-comment:93 --> -- 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]
