DanielLeens commented on PR #12355: URL: https://github.com/apache/seatunnel/pull/12355#issuecomment-5852877268
Thanks for looping me back in, @SEZ9 and @wgzhao — I've been following the last few days of back-and-forth. To save you some digging: nothing has changed here since my last two posts on this exact head (`4f6663a9f`): - My full re-review and approval: https://github.com/apache/seatunnel/pull/12355#pullrequestreview-5289848923 (2026-09-23), where I independently re-derived the `columnLength` fix in `MySqlTypeUtils` by hand. - My follow-up cross-check: https://github.com/apache/seatunnel/pull/12355#issuecomment-5812770981 (2026-09-24), where I re-verified the same five length values (`SET('a','b','c')` -> 5, `ENUM('x','y')` -> 1, `ENUM('active','inactive')` -> 8, `SET('only')` -> 4, `SET('a,b','it''s')` -> 8) against `MySqlTypeUtils.maxOptionListLength`/`unquotedValueLength`, and confirmed the `getColumnLength()` assertions and the two fully-qualified Javadoc references were present on this same head. @SEZ9, your three pointers today line up with @wgzhao's two earlier per-item answers — https://github.com/apache/seatunnel/pull/12355#issuecomment-5787984776 and https://github.com/apache/seatunnel/pull/12355#issuecomment-5806847189 — which is also what I checked against independently above, so there shouldn't be anything left to reconcile: the derived length does land in `Column.columnLength` via `MySqlTypeUtils.java:165-168` -> `MySqlTypeConverter.java:247-253` (not just in `getSourceType()`), and the boundary shapes (multi-character options, the single-option `SET`, the embedded-comma/escaped-quote option, and the `CHARACTER SET` clause) each carry both a `getSourceType()` and a `getColumnLength()` assertion in `CustomMySqlAntlrDdlParserTest`. For the record: no new commit has landed since my approval, `Build` is currently green on `4f6663a9f` (I just re-checked the status rollup myself), and from my side there are no remaining code-side blockers. The only outstanding item is procedural, not technical: per the `dev` branch ruleset a contributor approval — mine included — doesn't satisfy branch protection, so this genuinely needs an approving review (and merge) from an account with write access. @SEZ9, whenever your own pass on the diff lines up with the above, that's the piece that unblocks this. -- 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]
