SEZ9 commented on PR #11493:
URL: https://github.com/apache/seatunnel/pull/11493#issuecomment-6183475935
Thanks @davidzollo — agreed that the typed `Mockito.<TableSchema>any()`
matcher in 58d1cf0d19d resolves the `testCompile` ambiguity introduced by the
new `TableSchema` overload.
Two things are still outstanding before a re-review makes sense:
1. **CI on the current head.** The Build on 58d1cf0d19d has not produced a
green signal: the run was cancelled by fail-fast after two failures, so the E2E
lanes (`connector-file-local-it`, `doris-connector-it`, `kafka`, `starrocks`)
never executed on this head. Could you triage those two failures (or re-run if
they look unrelated) and share the outcome?
2. **The four earlier findings remain open**, since the latest commit only
touches the two `JdbcSinkWriterTest` stubs:
- **PR11493-F1 (HIGH, Compatibility)** — Snowflake `VARIANT`, Doris
`JSON`/`JSONB` and StarRocks `JSON` source columns silently change from
`STRING` to the new JSON logical type with no opt-out. Please add an opt-out to
keep the legacy `STRING` mapping, or at minimum document this in the
incompatible-changes doc with an upgrade note.
- **PR11493-F2 (MEDIUM, Logic)** — the JSON type is physically a plain
`String`, so non-JSON text can reach a native JSON sink; validation appears to
exist only at the StarRocks load boundary. Please clarify in the schema-feature
doc whether validity is a guarantee of the type or the sink's responsibility,
and align the native JSON sinks accordingly.
- **PR11493-F3 (MEDIUM, Robustness)** — StarRocks sink with native JSON
columns and the default CSV Stream Load format only fails at load time; this
should be a fail-fast validation at sink initialization with a clear message,
plus a note in the StarRocks sink doc.
- **PR11493-F4 (MEDIUM, Robustness)** — Snowflake sink with a
user-defined query silently skips `PARSE_JSON` wrapping for JSON fields. Please
either fail fast in that case or document the limitation in the Snowflake sink
doc.
Happy to re-review once CI is green on the head and the above are addressed.
<!-- streview-comment:1766 -->
--
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]