SEZ9 commented on PR #12397: URL: https://github.com/apache/seatunnel/pull/12397#issuecomment-6623137327
Quick follow-up on head `d289489598`. CI: the fork run (https://github.com/goutamadwant/seatunnel/actions/runs/41352850406) failed only in `Run / engine-v2-it (17, ubuntu-latest)` on `SystemLoadAllocateStrategyIT.testSystemLoadStrategy` with an Awaitility `ConditionTimeout`. That IT exercises Zeta slot allocation and does not touch the Spark translation layer this PR changes, so I don't attribute it to this diff. Please use "Re-run failed jobs" on that run page; if it fails again, drop the new run link here and we'll look at it together. No unrelated changes should be added to this PR to chase it. Source side: my conclusion from the previous review is unchanged, so the open items are still the ones raised there. To move this toward approval, could you confirm the status of each? 1. **`reconvertArray`** – the three overloads fill a typed array from the declared element class with no element-class guard, so a declared-vs-runtime mismatch surfaces as a bare `ArrayStoreException` with no field/type context. A guard that throws a contextual error is the main remaining ask. 2. **`TypeConverterUtils.convert(Spark ArrayType)`** – silently returning `ArrayType.of(elementType)` for temporal element types yields lossy `ARRAY<BIGINT>` / `ARRAY<DECIMAL(20,6)>` for TIME / TIMESTAMP_TZ arrays. Either reject those explicitly or document why it's acceptable. 3. **`SeaTunnelArrayType`** – please add Javadoc, consider a utility-style name, and reference the parser defect (`new ArrayType<>(MapType.class, ...)`) it works around. 4. **`SeaTunnelArrayType.runtimeClass`** – the comment should also mention that ignoring `ArrayType.getTypeClass()` covers the mis-declared `LocalTimeType[].class` constants, not just the parsed array-of-map case. 5. **`incompatible-changes.md`** – the null-map-element change (`null` instead of `{}`) should spell out the NPE failure mode for sink writers that iterate `Map[]` without a null check, and the component should be named via the module path convention used by the sibling entries. 6. **`docs/en/engines/spark.md` (and zh mirror)** – the "Nested Arrays" section should drop the release-note / PR-scope prose ("Previously ...", "This array-conversion fix does not ...") and read as reference documentation. If any of these are already addressed in a pending push, a short note per item is enough. Thanks for your patience! <!-- streview-comment:2275 --> -- 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]
