DanielLeens commented on PR #11493: URL: https://github.com/apache/seatunnel/pull/11493#issuecomment-6624347733
Thanks @SEZ9. Here are the confirmations against `ff53cb15bf5`. All four of your points are covered on this head. I also list below what I still owe from my own review of this head, because it overlaps with your F1/F2. **F1, opt-out coverage and docs.** - Doris `JSON`/`JSONB`: `handle_json_as_string` is applied in `DorisSourceFactory.java:113-117` via `LegacyJsonTypeUtil.mapJsonColumnsToString`. - Snowflake `VARIANT`: the option is applied by the Jdbc source in `JdbcSource.java:72-82`. - StarRocks: the StarRocks source has no inferred schema. `StarRocksSourceTableConfig.java:58` builds it only from the user-declared `schema`, so declaring the column as `string` keeps the old behavior. The mapping change there affects only `StarRocksCatalog` and automatic table creation. - `docs/en/introduction/concepts/incompatible-changes.md:10-15` (zh mirrored) states the default change, names the option for Doris and the Jdbc/Snowflake source, and explains the StarRocks case. **F2, validation contract.** The documented position is in `docs/en/introduction/concepts/schema-feature.md:161` (zh same line). `json` is String-backed, and the type guarantees the representation, not the validity of the text. Validity is enforced where the text is parsed: the JSON format, the StarRocks JSON Stream Load serializer, and Snowflake `PARSE_JSON`. I'm not adding validation at the type boundary. One correction to that sentence is owed (see the list below). **F3, StarRocks CSV fail-fast.** The `StarRocksSink` constructor (`StarRocksSink.java:71`) calls `validateJsonColumnsAgainstLoadFormat` (`:147-169`). The exact message is: `StarRocks sink table %s.%s has native JSON column(s) %s, but the Stream Load format is CSV, which does not support JSON-formatted columns. Set `starrocks.config.format = "JSON"` for this sink, or convert the field(s) to STRING upstream (for example `CAST(field AS VARCHAR)` in a Calcite Transform).` `StarRocksIT.java:351-363` asserts both `StarRocks sink table test.json_csv_rejected has native JSON column(s) [payload]` and `starrocks.config.format = "JSON"`. The docs tell users to switch the format: `docs/en/connectors/sink/StarRocks.md:243`, and `docs/zh/connectors/sink/StarRocks.md:238`. **F4, Snowflake user-defined query.** This path fails at sink initialization rather than relying on docs alone. `JdbcSink.java:114` calls `dialect.validateUserDefinedQuery`, and `SnowflakeDialect.java:125-151` rejects a custom `query` that has JSON fields but no `PARSE_JSON`, naming the fields. It is tested by `SnowflakeDialectTest#userDefinedQueryWithoutParseJsonIsRejectedForJsonFields` and `#userDefinedQueryWithParseJsonOrWithoutJsonFieldsIsAccepted`. `docs/en/connectors/sink/Snowflake.md:56` documents that the user wraps placeholders with `PARSE_JSON(?)`. It also documents the limitation that the check only looks for `PARSE_JSON` somewhere in the query, not at every placeholder. **Still owed in a follow-up commit** (from my self-review of this head; these overlap with your F1 blast-radius point and F2): 1. Issue 12: add a `JSON` case next to `STRING` in `TextSerializationSchema` (`:213`, plus the nested case at `:239`) and in `TextDeserializationSchema` (`:288`). It is a pass-through, because the value is already a `String`. This fixes Doris CSV Stream Load and text-format file sinks, which today fail at the first row with `not supported for parsing this type [JSON]`. I'll add one text-format JSON round-trip test. 2. Issue 13 (docs): in both `incompatible-changes.md` pages, state that generic JDBC sinks (MySQL, PostgreSQL and others) cannot auto-create a table for a `JSON` column, and point users to `handle_json_as_string` or a CAST. 3. F2 doc correction in `schema-feature.md:161` (en and zh): list the Doris JSON Stream Load path as a parsing boundary next to StarRocks, and don't imply that the Doris CSV path validates JSON. -- 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]
