MindOfqc commented on PR #13012:
URL: https://github.com/apache/seatunnel/pull/13012#issuecomment-6624229521
Answering your two items explicitly, with the F1–F6 mapping you asked for.
Your review listed them as "Issue 1"–"Issue 6" in `# Findings` and repeated the
same order under `Review conclusion → Remaining follow-ups`, so I am numbering
F1 = Issue 1 … F6 = Issue 6; if your tool assigns them differently, tell me and
I will re-label.
**Item 1 — the real test count.** The full-class summary on the current head
is
```
mvn -o -pl seatunnel-connectors-v2/connector-jdbc
-Dtest=SqlWhereConditionHelperTest -DfailIfNoTests=false
-Dspotless.check.skip=true test
Tests run: 127, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
```
The `Tests run: 1` you saw was a
`-Dtest=SqlWhereConditionHelperTest#methodA+methodB` filtered run that I used
to capture the red before fixing, not the class total. 127 is the actual count:
125 on this branch before my two confusable-character tests were added
(@BryanOnKeys independently reported 125/125 at `4895cefa`), plus the two new
ones.
**Item 2 — the mapping.**
| Your finding | Where it stands | Evidence |
|---|---|---|
| F1 wildcard e2e fixture cannot reproduce #12998 and duplicates the
existing `select *` case | **Addressed — by removing the fixture**, your first
option | `59e31563`; the Unicode wildcard path stays pinned by
`SqlWhereConditionHelperTest:1566` and `:1575` (`select id as maß, t.* from t`)
|
| F2 `regionMatches(true, …)` folds Unicode per character, so `ſ`/`ſt` read
as keyword letters | **Addressed — and yes, the helper is ASCII-only, that is
the point of its name** | `b70ce8e9`. `matchesAsciiKeyword` folds only when
`actual < 0x80`: `boolean foldable = actual < 0x80 &&
Character.toUpperCase(actual) == expected;`. `ſelect` and `diſtinct t.*` are
pinned as *not* keywords by the tests at `:1629` and `:1637` |
| F3 the after-`SELECT` fallback still reorders columns positionally |
**Deferred, not addressed here** — agreed with you and @DanielLeens that it
needs its own issue, because it changes behaviour for every query whose main
`FROM` the scanner cannot locate (e.g. `select a from(select ...) x`), not just
case-expanding ones | This PR removes the trigger at
`SqlWhereConditionHelper.java:538`; the fallback itself is untouched |
| F4 the auto-append behaviour is undocumented | **Addressed** | `431b9d81`,
the `where_condition` rows in `docs/en/connectors/source/Jdbc.md` and
`docs/zh/connectors/source/Jdbc.md`, including the UNION/INTERSECT/EXCEPT,
GROUP BY/HAVING, DISTINCT, LIMIT, window-function and CTE exclusions |
| F5 `columnList` sliced with lengths taken from an uppercased copy |
**Addressed — it is no longer uppercased-copy-based**, which is the specific
change you asked for | `b70ce8e9`: `if (matchesAsciiKeyword(columnList, 0,
"DISTINCT ")) { columnList = columnList.substring(9).trim(); } else if
(matchesAsciiKeyword(columnList, 0, "ALL ")) { columnList =
columnList.substring(4).trim(); }`. The prefix is now matched on the same
string that is sliced, so the offsets cannot be off by a ligature expansion,
and the comment at lines 241-242 is true for the whole method |
| F6 one `@TestTemplate` running both jobs hides which path broke |
**Addressed as a side effect of F1** — with the wildcard job removed there is
one job per method, so the #12998 regression has its own signal | `59e31563`,
`testMysqlJdbcWhereConditionInsertsMissingColumnBeforeMainFrom` |
So: F1, F2, F4, F5, F6 are addressed on the current head `5ca1cd51`; F3 is
the only one I am proposing to defer, and I will open it as a separate issue if
you would rather have it tracked than left in this thread.
On the CI side nothing has changed in my control since @DanielLeens' last
note: `422 Actions has been disabled for this repository.` on every dispatch
against my fork while `GET /repos/MindOfqc/seatunnel/actions/permissions` keeps
reporting `enabled: true`, and the fork run history still ends at
`2026-10-29T18:18:19Z`. Per Daniel's guidance I am not touching the head (no
empty commit, no `dev` sync) until Actions runs again or a committer triggers
`Build` for `5ca1cd51`.
Prepared with an AI coding agent.
--
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]
