wgzhao commented on PR #10453:
URL: https://github.com/apache/seatunnel/pull/10453#issuecomment-6105233123

   Thanks for the follow-up — two notes and one answer.
   
   The fixes are already in: head `9064cf692` (a single commit) plus a 
rewritten PR description landed yesterday shortly after my plan comment, and 
Daniel re-reviewed that head and confirmed each of the eight points is 
addressed. So 1/2 (normalized `dataType` passed into `maxOptionListLength`, 
with the sizing path asserted as the SET-style 37 rather than the ENUM-style 
23), 4 (JDBC `Mysql.md` en/zh now list `SET`), 5 (the fixture drives 
`dataType("SET")` + `unsigned(true)`), 6 (the case moved into 
`MySqlTypeUtilsIntTypeNarrowingTest` and builds a real `Column.editor()`), 7 
(the comment now claims only that the synthetic suffix is dropped) and 8 (the 
warn carries the column name and type) are all in that commit.
   
   On the "cut off" sentence: the comment is not truncated on my side — it 
continues "...applies it to both `MySqlCatalog` and `OceanBaseMySqlCatalog`, 
and covers the shape end-to-end in 
`JdbcMysqlIT.testSetAndEnumColumnWithUnsignedWordInValueList()`". What you 
quoted stops at `OceanBaseMySqlCatalog`, so it looks like a 
notification/rendering truncation rather than the comment.
   
   Coexist or replace: they are meant to coexist, and neither depends on the 
other. #12333 removes the producer of the synthetic name on the catalog path, 
so once it lands nothing in-tree should synthesize `SET UNSIGNED`; this PR 
stays as the defensive arm for a synthetic key supplied from outside (a caller 
using the converter directly, or a Debezium-reported name on the CDC path). I 
can say that explicitly in the constant's comment if it helps.
   
   The PR description now points at #12333 (the root-cause split) and #12354 
(the binlog value-list limitation).
   
   One thing worth flagging while you re-check: the `Build` on this head is red 
only in `elasticsearch-connector-it (11)/(17)` — 
`ElasticsearchIT#testElasticsearchWithScrollSlice` / 
`testElasticsearchWithPITSlice(Queued)` — which also fails on `dev`'s own last 
Build (run 37927712604), and rerunning just those jobs failed the same way. 
Nothing in this diff touches Elasticsearch.
   


-- 
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]

Reply via email to