SEZ9 commented on PR #11493: URL: https://github.com/apache/seatunnel/pull/11493#issuecomment-6207402839
Status looks accurate: the only change on `58d1cf0d19d` is the typed `Mockito.<TableSchema>any()` in the two `JdbcSinkWriterTest` stubs, so all four findings from the previous round remain open. Noting what I'd like to see in the next push so re-review is quick: - **F1 (compatibility):** Either a config-level opt-out that keeps Snowflake `VARIANT` / Doris `JSON`/`JSONB` / StarRocks `JSON` mapped to `STRING`, or, if you decide the new mapping is unconditional, an explicit entry in `docs/en/introduction/concepts/incompatible-changes.md` describing the upgrade impact and the migration path. Please say which route you are taking. - **F2 (logic):** A clear statement in `docs/en/introduction/concepts/schema-feature.md` of what the JSON logical type guarantees. If validity is the sink's responsibility, say so; if it is a property of the type, the validation needs to live somewhere other than only the StarRocks load boundary. - **F3 (robustness):** Fail-fast at StarRocks sink initialization when the target table has native JSON columns and the Stream Load format is the default CSV, rather than surfacing the rejection at load time. - **F4 (robustness):** For the Snowflake sink with a user-defined query, either apply `PARSE_JSON` wrapping for JSON fields or fail at sink initialization with an actionable message, instead of letting the server reject mid-run. On CI: merging current `dev` to pick up the `Dead links` and `unit-test (8, ubuntu-latest)` fixes before the next push is the right call, since fail-fast on run 38351233448 meant the E2E lanes never executed on this head. Please include the E2E outcome (Doris, StarRocks, Snowflake and the format tests) in the ping when the code is up, and I will do the full re-review then. <!-- streview-comment:1792 --> -- 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]
