SEZ9 commented on PR #11493: URL: https://github.com/apache/seatunnel/pull/11493#issuecomment-5390961407
Thanks for the follow-up, @DanielLeens. The test-only commit (`8ddb8ff420f0` → `79103480df52`) resetting the mounted `json-type` paths in `LocalFileIT.java` sounds reasonable, and I appreciate the note that local proof is limited to spotless — let's wait for the refreshed GitHub CI on the updated head before we call that lane settled. I also appreciate the independent re-verification against `79103480df52` (tail-appended `JSON` in `SqlType.java`, `BasicType.JSON_TYPE` backed by `String`). That confirms the mechanical enum-compatibility side. However, the four findings from the previous round are still open, and the test commit doesn't move any of them: 1. **F1 (HIGH, Compatibility):** Snowflake `VARIANT`, Doris `JSON`/`JSONB`, and StarRocks `JSON` source columns still silently switch from `STRING` to the new `JSON` logical type with no opt-out. Concrete ask: either add a config-level opt-out to preserve the legacy `STRING` mapping, or explicitly document in `docs/en/introduction/concepts/incompatible-changes.md` why an opt-out is not provided and what upgrading users must change. 2. **F2 (MEDIUM, Logic):** Since `JSON` is physically a plain `String`, nothing stops non-JSON text from reaching a native JSON sink; validation appears to exist only at the StarRocks load boundary. Your re-check confirming `TextSerializationSchema.java` still has no `case JSON:` in the `SqlType` switch reinforces that the type is pass-through today. Ask: state where validation is intended to live, and reflect that contract in `docs/en/introduction/concepts/schema-feature.md`. 3. **F3 (MEDIUM, Robustness):** StarRocks sink with native JSON columns plus the default CSV Stream Load format should be rejected fail-fast at sink initialization rather than failing at load time. Ask: add that init-time check and document the constraint in `docs/en/connectors/sink/StarRocks.md`. 4. **F4 (MEDIUM, Robustness):** Snowflake sink with a user-defined query silently skips `PARSE_JSON` wrapping for JSON fields, deferring failure to a mid-run server error. Ask: either apply the wrapping in the user-query path, or fail/warn clearly at startup and note the limitation in `docs/en/connectors/sink/Snowflake.md`. Once the CI signal on `79103480df52` comes back and there's a plan (code or docs) for each of the four items above, I'm happy to take another pass. <!-- streview-comment:520 --> -- 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]
