SEZ9 commented on issue #11632: URL: https://github.com/apache/seatunnel/issues/11632#issuecomment-5381236605
@li3zhi4 Thanks for the point-by-point response — answering each item directly: 1. **Design point ① (missing field vs. explicit `null`)**: your implementation choice matches the acceptance criteria we agreed on in this issue — the configured default applies to both a missing field and an explicit JSON `null`, and a nullable column without a configured default keeps returning `null`. Having both paths covered by `JsonDefaultValueTest#testDefaultValueWhenFieldMissing` and `testNoDefaultValueWhenFieldMissing` is exactly the regression coverage I asked for, so consider that point settled. 2. **Design point ② (shared layer for text/csv etc.)**: agreed with your scoping. Keeping this PR limited to `seatunnel-format-json` is the right tradeoff for reviewability, and tracking the other formats in a separate follow-up issue keeps this fix single-purpose. One concrete ask here: please open that follow-up issue (or let me know if you'd like me to file it) so the cross-format gap doesn't get lost once this closes. 3. **Review findings**: thanks for confirming they've been addressed on `61d05f5a34` and replied to on the PR. I'll verify the responses on the PR side rather than re-litigating them here. Remaining before we treat this issue as resolved, all on PR #11633: - the current head keeps missing-field and explicit-null behavior covered by tests; - no regression for the no-default case; - CI reaches terminal green. Let's keep further technical discussion on #11633; this issue stays open for tracking until the PR merges. <!-- streview-comment:446 --> -- 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]
