SEZ9 commented on PR #12355: URL: https://github.com/apache/seatunnel/pull/12355#issuecomment-5806330422
Thanks @wgzhao - and apologies for the delay, this fell off my radar. Happy to take another look at `4f6663a9f` now. Before I do, a few quick things so I can close each of the five points cleanly: 1. **Length leaking into `columnLength` (the first point)** - you mention the `SET`/`ENUM` length is now derived from the option list in `MySqlTypeUtils`. Can you confirm the rule you landed on (e.g. `ENUM` = length of the longest option, `SET` = sum of option lengths plus `n-1` separators), and whether it works off the raw quoted option text or the unquoted values? Escaped quotes / multi-byte options would give different answers depending on which one is used, so I want to check that against the test expectations. 2. **Per-item breakdown** - you reference a reply "just above" with the item-by-item breakdown, but I don't see it in the thread on my side. Could you re-post or link it? It would save me guessing which hunk maps to which point. 3. **Test coverage (points 3 and 4)** - the new shapes you list (multi-character, embedded comma, escaped quote, `CHARACTER SET`) cover the verbatim-join cases. Do the tests also assert `columnLength` (not only `getSourceType()`) for those shapes, and is the single-option `SET` case included? Those were the two remaining gaps I had flagged. 4. **Javadocs (points 2 and 5)** - just confirm the test Javadoc now describes the pre-fix value as `SET(5)` and refers to `Column#getSourceType()` on the SeaTunnel `Column`, and that the override's Javadoc uses a qualified reference so it doesn't resolve to the Debezium `Column`. If those all check out I don't expect anything further on the code side, and I'll leave a review on the head accordingly. No need to touch the branch in the meantime. <!-- streview-comment:1268 --> -- 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]
