zhang-arvin commented on PR #11867: URL: https://github.com/apache/seatunnel/pull/11867#issuecomment-6051667476
@DanielLeens Thanks for the detailed review — the SQL-shape findings are correct and I've fixed the highest-severity ones. Pushed head `9d9d4c6fa`. **Issue 1 (High, OR-chain parenthesization)** — fixed. Both lexicographic builders now wrap the whole OR chain in outer parentheses (and only parenthesize clauses after the first, so the result is `(a > ? OR (a = ? AND b >= ?))` instead of the precedence-broken `(a > ?) OR (...) AND ...`). Middle chunks can no longer leak rows past their upper bound. **Issue 4 (param count/binding for N>=3)** — fixed. The placeholder count is now `N*(N+1)/2` (clause *i* has *i* equality values + 1 comparison) and `bindLexicographicLowerBoundParams` binds clause-by-clause instead of the wrong `2N-1` sequence. **Issue 5 (tests could not detect the above)** — addressed. `MySqlUtilsTest` now asserts the **exact** generated SQL string (not substrings) for the middle/first/last split, plus a new 3-column middle-split case that locks the placeholder count. Full module suite green locally: `Tests run: 37, Failures: 0`. **Issue 7 (unrelated tests)** — fixed. Removed the two snapshot-only startup tests (`MySqlIncrementalSourceStartupConfigTest`) that were carried into this PR but belong to the snapshot-only work (they also referenced a helper not present on this branch and broke the unit-test job). Still open and not yet addressed in this push — I'd rather be explicit than claim otherwise: **Issue 2** (`getSplitColumns()` bypassing split-column selection semantics), **Issue 3** (binlog backfill boundary truncating composite keys to the first column), **Issue 6** (docs). I'll follow up on those next; flagging so you don't have to re-derive them. -- 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]
