SEZ9 commented on PR #11730: URL: https://github.com/apache/seatunnel/pull/11730#issuecomment-5358313330
Thanks everyone for the thorough back-and-forth here — this thread made the review very easy to follow. @siwen-yu, thanks for turning around the YashanDbDialect fix and the spotless pass so quickly, and @DanielLeens, I really appreciate the end-to-end re-review at `c0824e0` and the candid note about the XuguDialect guard you spotted at `c3fc5704`. Before I merge, two concrete asks: 1. **XuguDialect guard** — @siwen-yu, can you explicitly confirm that at `c0824e0` the pre-existing `nonUniqueKeyFields.isEmpty()` guard in `XuguDialect.getUpsertStatement()` (the one throwing `SeaTunnelException` before the `matchedClause` logic is reached) has been removed or adjusted, and that `JdbcAllKeyTableUpsertTest.testAllKeyTableOmitsEmptyUpdateSet()` passes for Xugu? @DanielLeens noted no open blockers on his side at `c0824e0`, but since his review comment surfaced this as dead code at `c3fc5704`, I'd like it confirmed on the record. 2. **Duplicate fix in #11861** — @DanielLeens flagged that #11861 addresses the same root cause (#11729) and touches the same dialect files. Since this PR additionally covers YashanDB and has already been reviewed end-to-end here, my inclination is to merge this one and close #11861 as a duplicate. If anyone sees a reason to prefer the other direction, please say so now. On merge mechanics: understood that @DanielLeens's `APPROVED` review doesn't satisfy the branch protection gate — once item 1 is confirmed, I'll add my own approval with write access and merge. Thanks again @siwen-yu, @DanielLeens, and @LeonYoah for the careful work on an easy-to-miss schema shape. <!-- streview-comment:408 --> -- 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]
