MindOfqc commented on PR #13012:
URL: https://github.com/apache/seatunnel/pull/13012#issuecomment-6570668060
Follow-up on Issues 1 and 6 from your review — both are addressed in
`59e31563`, which is the commit I promised in my previous comment.
**Issue 1: I took your first option and dropped the wildcard fixture.**
`jdbc_mysql_where_condition_wildcard.conf` handed the helper `select * from
source.unicode_column_table` — pure ASCII, so the pre-fix
`toUpperCase(Locale.ROOT)` copy was length-preserving and the old and new code
behave identically on it. It also duplicated the `select * from source.table2`
+ `where c_int >= 0` case that
`jdbc_mysql_source_and_sink_with_multiple_tables.conf` already runs. Your
second option (move the case-expanding identifier into the projection) needs
`select c_varchar as \`größe\`, t.* from source.unicode_column_table t`, and
that projection has five columns against a four-column sink, so it also forces
a new alias column into `CREATE_UNICODE_TABLE_SQL`, `initUnicodeTableData` and
`assertUnicodeTableProjection` — all of which feed the inserted-column fixture
as well. I cannot run the MySQL container here (no Docker on this machine), so
resharing that fixture blind looked worse than rem
oving the misleading one.
Nothing is lost at the level that actually discriminates: the Unicode
wildcard path stays pinned by unit tests in `SqlWhereConditionHelperTest` —
`testHasTopLevelSelectWildcard_UnicodeUppercaseExpansion` (line 1566, `select
id as maß, t.* from t`) and its uppercase-keyword twin at line 1575 — plus the
two ligature cases added in `b70ce8e9` at lines 1629 and 1637. Those assert on
the exact string `SqlWhereConditionHelper` receives, which is what the e2e
fixture could not do with an ASCII query.
**Issue 6 is resolved by the same commit.** With the wildcard job gone the
remaining case is one job per method, so
`testMysqlJdbcWhereConditionWildcardAndInsertedColumn` became:
```java
@TestTemplate
public void
testMysqlJdbcWhereConditionInsertsMissingColumnBeforeMainFrom(TestContainer
container)
throws IOException, InterruptedException, SQLException {
clearUnicodeSinkTable();
Container.ExecResult insertedResult =
container.executeJob("/jdbc_mysql_where_condition_inserted_column.conf");
Assertions.assertEquals(0, insertedResult.getExitCode(),
insertedResult.getStderr());
assertUnicodeTableProjection();
}
```
That is the #12998 regression itself (the inserted `c_int` before the main
`FROM`, read back through the positional
`AbstractJdbcRowConverter.toInternal`), and it now has an unambiguous signal —
no earlier job can mask it. If you would rather keep a wildcard case that
actually reproduces, say the word and I will add the `größe` alias column to
the Unicode DDL and the projection assertion in one commit; it just needs a
real MySQL run behind it.
Verification I was able to do here, and what I could not:
- `grep -rn "jdbc_mysql_where_condition_wildcard"` over the repo → 0
remaining references; `testMysqlJdbcWhereConditionWildcardAndInsertedColumn` →
0 callers, so the rename leaves nothing dangling.
- `SqlWhereConditionHelperTest` is unchanged by this commit and still green:
`mvn -o -pl seatunnel-connectors-v2/connector-jdbc
-Dtest=SqlWhereConditionHelperTest -Dspotless.check.skip=true test` → `Tests
run: 127, Failures: 0, Errors: 0`.
- `mvn -o -pl <connector-jdbc-e2e-part-1> test-compile` cannot run offline
in this clone: the module's test scope wants `seatunnel-starter` and
`seatunnel-spark-3-starter` `3.1.0-SNAPSHOT`, which are not in the local
repository, and `-am` is not an option here. So compilation of the IT is
CI-verified, not locally verified. No new dependencies, no new test class, and
the method bodies are a subset of what was there before.
- Spotless still cannot run on JDK 21 in this environment, so the Java edit
is hand-formatted to AOSP/100 columns; the two edited lines are within the
limit and the surrounding style is untouched.
Head is now `59e31563`. It changes only the e2e test class and deletes the
fixture, so the source-side behaviour is exactly what you reviewed at
`4895cefa0bde` plus the ASCII keyword helper. The exact-head `Build` is still
gated on the fork-side Actions restriction, which I have now packaged for
GitHub Support rather than worked around.
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]
