li3zhi4 commented on PR #11633: URL: https://github.com/apache/seatunnel/pull/11633#issuecomment-5211706252
Thanks @DanielLeens — and no apology needed on the carryover; the same-class gap is exactly what multi-round review is for. The new head `afb64bbee` addresses Issue 1: **Issue 1 (High — FLOAT_VECTOR default shares one ByteBuffer → `BufferUnderflowException` on the second row):** `FLOAT_VECTOR` is now part of the mutable-type set (`JsonToRowConverters.java:443-447`), so its default keeps its `JsonNode` and is re-converted per record instead of caching the single `ByteBuffer` (which carries a mutable read cursor). Added `testFloatVectorDefaultValueNotSharedAcrossRows`, in the same style as the ARRAY/MAP/BYTES test: it decodes two rows with a `FLOAT_VECTOR` default, asserts the `ByteBuffer` instances are distinct, and that each row can fully consume its own buffer (guarding against `BufferUnderflowException`). **Issue 2 (High, CI confirmation):** the fork's Actions run for this head is running; I'll report the `kafka-connector-it` result once it reaches a conclusion rather than claiming green on inspection. **Issue 3 (Low, optional — BYTES `clone()` on every field):** noted, but I'd rather keep the `clone()` in the BYTES converter than scope it to the default path: it guarantees BYTES is instance-safe in all consumption paths (not just defaults), costs one small allocation per BYTES field, and avoids splitting the mutable-handling logic across two places. Happy to narrow it if you prefer, but I'd treat it as not worth the extra branching. Verification: `JsonDefaultValueTest` 15/15 (new FLOAT_VECTOR test included), full `seatunnel-format-json` module 61/61 green, `KafkaJsonDefaultValueIT` e2e 1/1 passed locally, `spotless:check` clean. Branch is up to date with `dev`. -- 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]
