DanielLeens commented on PR #12355:
URL: https://github.com/apache/seatunnel/pull/12355#issuecomment-5812770981
Just following the thread here since I was tagged upstream in this
conversation — no new commit landed since my last review, so this isn't a
re-review, just a quick cross-check on the exchange above.
@SEZ9's four questions and @wgzhao's answers line up exactly with what I
independently re-derived in my own approval on `4f6663a9f`: I hand-verified the
same five length values against
`MySqlTypeUtils.maxOptionListLength`/`unquotedValueLength` (`SET('a','b','c')`
-> 5, `ENUM('x','y')` -> 1, `ENUM('active','inactive')` -> 8, `SET('only')` ->
4, `SET('a,b','it''s')` -> 8), and I can confirm the `getColumnLength()`
assertions for all of those shapes are present in
`CustomMySqlAntlrDdlParserTest#testParseAlterTableAddSetAndEnumColumnKeepsOptionList`
on the current head, not just `getSourceType()` — so point 3 checks out as
described. The two Javadocs
(`CustomAlterTableParserListener#getSourceColumnTypeWithLengthScale` and the
test) also already use the fully-qualified
`org.apache.seatunnel.api.table.catalog.Column#getSourceType()` and describe
`SET(5)`/`ENUM(1)` as the pre-fix value, matching point 4.
Nothing further from me — my approval stands unchanged on `4f6663a9f`.
@SEZ9, happy to let you close out your own review once you've confirmed these
against the head yourself.
--
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]
