wgzhao opened a new pull request, #12355:
URL: https://github.com/apache/seatunnel/pull/12355
### Purpose of this pull request
Fixes #12354.
**Symptom.** A MySQL `SET` or `ENUM` column that reaches a MySQL-CDC source
through the schema-change path gets a source type of `SET(5)` / `ENUM(1)`
instead of `SET('a','b','c')` / `ENUM('x','y')`. With `schema_save_mode =
CREATE_SCHEMA_WHEN_NOT_EXIST` on a MySQL sink, the generated `CREATE TABLE`
contains `` `col` SET(5) NULL ``, and MySQL rejects it with a syntax error.
**Mechanism, layer by layer.**
1. Debezium reports the bare type name for these two types and keeps the
option list on the column. From
`io.debezium.connector.mysql.antlr.listener.ColumnDefinitionParserListener`
(SeaTunnel ships its own copy with the same logic):
```java
if (dataType.name().equalsIgnoreCase("SET")) {
int optionsSize = ...;
columnEditor.length(Math.max(0, optionsSize * 2 - 1)); // options +
commas, not a DDL length
}
...
if (dataTypeName.equals("ENUM") || dataTypeName.equals("SET")) {
columnEditor.type(dataTypeName);
columnEditor.enumValues(collectionOptions); // raw SQL literals,
quotes included
}
```
So `SET('a','b','c')` arrives as `typeName = "SET"`, `length = 5`,
`enumValues = ["'a'","'b'","'c'"]`.
2. `SeatunnelDDLParser.toSeatunnelColumnWithFullTypeInfo` then rebuilds the
source type as `typeName + "(" + length + ")"`. That is correct for
`VARCHAR(255)` or `DECIMAL(10, 2)`, but for these two it renders the
bookkeeping count: `SET(5)`, `ENUM(1)`.
3. `MysqlCreateTableSqlBuilder.buildColumnIdentifySql` writes
`Column#getSourceType()` verbatim for MySQL targets, so the value reaches the
generated DDL.
**Fix.** Override `getSourceColumnTypeWithLengthScale` in the MySQL ALTER
listener to render the option list when the column carries one, and fall back
to the base implementation otherwise.
**Why the fix is not in `MySqlTypeUtils`.** That is where I first tried, and
it does not work: `toSeatunnelColumnWithFullTypeInfo` overwrites whatever
`toSeatunnelColumn` produces, so a change there is dead code on this path.
`MySqlTypeUtils` produces the parameterless type by design, and the component
that adds the parameters is the one that has to know that `SET` / `ENUM` take
options rather than a length. I am calling this out because `MySqlTypeUtils` is
the obvious place to look, and the regression test below is what caught my
first attempt.
**Scope.** The snapshot path is not affected: table structures discovered at
job start come from the JDBC catalog and carry the full
`information_schema.COLUMN_TYPE` (`set('a','b','c')`). Only columns arriving
through `ALTER TABLE` / `MODIFY COLUMN` while the job is running go through the
reconstruction above. The override lives in the MySQL listener rather than in
the shared `SeatunnelDDLParser` default in `connector-cdc-base`, so no other
dialect is affected.
### Does this PR introduce _any_ user-facing change?
Yes - a bug fix, and only on the schema-change path:
- **Before**: a `SET` / `ENUM` column added or modified while the job is
running reached a MySQL sink as `SET(5)` / `ENUM(1)`, which the sink cannot
create.
- **After**: it reaches the sink as `SET('a','b','c')` / `ENUM('x','y')`.
No config option, default value, public API, SPI contract, serialization
format or checkpoint state is touched. Every other type keeps the previous
rendering, because the base implementation is still used whenever the column
carries no option list.
### How was this patch tested?
The regression test drives the real Debezium DDL parser rather than a
hand-built column, so it exercises the production path end to end:
```java
parser.parse("ALTER TABLE products ADD COLUMN c_set SET('a','b','c') NULL, "
+ "ADD COLUMN c_enum ENUM('x','y') NULL", new Tables());
```
and asserts that the resulting
`AlterTableAddColumnEvent.getColumn().getSourceType()` is `SET('a','b','c')` /
`ENUM('x','y')` while the data type stays `STRING`.
I confirmed it is a real regression test by reverting only the override and
re-running it:
```
expected: <SET('a','b','c')> but was: <SET(5)>
```
DDL validity, checked against a throwaway MySQL 9.7.2 (the grammar is the
same on 8.x):
```
CREATE TABLE t (c SET(5)); -> ERROR 1064 (42000): ... right syntax
to use near '5))'
CREATE TABLE t (c SET); -> ERROR 1064 (42000): ... right syntax
to use near ')'
CREATE TABLE t (c SET('a','b','c')); -> Query OK
```
Full unit test suite of the connector:
```
$ mvn -pl seatunnel-connectors-v2/connector-cdc/connector-cdc-mysql test
Tests run: 37, Failures: 0, Errors: 0, Skipped: 0
```
`spotless:apply` is clean and does not reformat the change.
No E2E case is added. Reproducing this needs an `ALTER TABLE` scheduled in
the middle of a running CDC job, while the test above already drives the exact
production path (Debezium DDL parser -> ALTER listener -> source type); I am
happy to add one if reviewers would prefer it.
### Check list
* [x] If any new Jar binary package adding in your PR, please add License
Notice according
[New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/developer/new-license.md)
* [x] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [x] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR.
* [x] If you are contributing the connector code, please check that the
following files are updated:
1. Update
[plugin-mapping.properties](https://github.com/apache/seatunnel/blob/dev/plugin-mapping.properties)
and add new connector information in it
2. Update the pom file of
[seatunnel-dist](https://github.com/apache/seatunnel/blob/dev/seatunnel-dist/pom.xml)
3. Add ci label in
[label-scope-conf](https://github.com/apache/seatunnel/blob/dev/.github/workflows/labeler/label-scope-conf.yml)
4. Add e2e testcase in
[seatunnel-e2e](https://github.com/apache/seatunnel/tree/dev/seatunnel-e2e/seatunnel-connector-v2-e2e/)
5. Update connector
[plugin_config](https://github.com/apache/seatunnel/blob/dev/config/plugin_config)
---
Related: #10451 (the reported failure), #10453 and #12333 (the `SET
UNSIGNED` work on the same code path; this issue was raised during the review
of #10453 and is independent of both).
--
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]
