SEPURI-SAI-KRISHNA opened a new pull request, #13040:
URL: https://github.com/apache/seatunnel/pull/13040
Yes, glad to contribute to Apache SeaTunnel.
### Purpose of this pull request
Closes #13038.
Databend has no TIME type. Its type parser defines 29 column type rules and
none of them is a
standalone TIME: the token appears only inside `TIMESTAMP WITH TIME ZONE`,
and there is no
`TableDataType::Time` variant for one to be held or reported as.
`DatabendTypeConverter` returned
`"TIME"` anyway, and `SchemaChangeManager.applyAddColumn` appends that value
straight into the
statement it builds, so a schema change event carrying a TIME column produced
```sql
ALTER TABLE `default`.`orders` ADD COLUMN `started_at` TIME
```
which Databend cannot parse. `applyModifyColumn` builds its statement the
same way.
The path is reachable from a common setup. `MySqlTypeConverter` maps a MySQL
`TIME` column to
`LocalTimeType.LOCAL_TIME_TYPE`, so with a MySQL CDC source, a Databend sink
and
`enable_schema_evolution = true`, an `ALTER TABLE ... ADD COLUMN t TIME`
upstream reaches
`DatabendSinkWriter.applySchemaChange` and then this converter.
The type is now carried as text, which is what the Doris and StarRocks
converters do for a target
that has no TIME type. Those two use `VARCHAR(8)`; this uses `STRING`
instead, because
`LocalTime.toString()` runs from 5 to 18 characters depending on the
sub-second part
(`12:34` through `12:34:56.123456789`), and Databend's String is unbounded,
so encoding a width of
8 would describe a limit that does not exist and would be wrong for a time
carrying nanoseconds.
Text is also what the value path already produces:
`DatabendSinkWriter.convertRowToJson` falls
through to `value.toString()` for a `LocalTime`, so the value arrives as
`"12:34:56"`, which fills a
text column but would not parse into a TIMESTAMP.
The two source documentation tables listed `TIME | TIME`, which described
nothing: no Databend
column is ever a TIME, and `DatabendCatalog.convertDatabendType` has no
`case "TIME"` to read one
with either. Those rows are removed.
### Does this PR introduce any user facing change?
Yes. A schema change event that adds or modifies a TIME column now succeeds,
creating a text
column, where before the statement failed to parse. No option, default or
SPI signature changed.
### How was this patch tested?
`DatabendTypeConverterTest` is new. `DatabendTypeConverter` had no test, and
putting these
assertions inside `SchemaChangeManagerTest` would have misnamed them, since
what is under test is
the converter rather than the manager. `convertToDatabendType` is already
`public static`, so no
visibility change was needed.
Four tests, and two of them fail against the code as it stood, which is the
check that they pin the
fix rather than the bug:
```
DatabendTypeConverterTest.timeIsCarriedAsTextBecauseDatabendHasNoTimeType
expected: <STRING> but was: <TIME>
DatabendTypeConverterTest.theColumnNameAndNullabilityAreCarriedThrough
expected: <STRING> but was: <TIME>
```
The other two, `theOtherTemporalTypesAreUnchanged` and
`theRestOfTheMappingIsUnchanged`, pass both
before and after, pinning DATE, TIMESTAMP, STRING, BOOLEAN, INT, BIGINT and
DECIMAL against drift.
Run locally on JDK 17:
- `./mvnw test -pl seatunnel-connectors-v2/connector-databend` : 24 tests, 0
failures.
- `./mvnw -B -T 1 test -D"license.skipAddThirdParty"=true -pl
seatunnel-ci-tools -am` : 0 failures,
which covers the Markdown check over the two edited documents.
- `./mvnw spotless:apply` : clean.
**On the E2E requirement, to be explicit rather than quiet about it:** no
existing IT in
`connector-databend-e2e` drives a schema change event.
`databend_schema_evolution.conf` sets
`enable_schema_evolution = true` but its source is a `FakeSource` with a
fixed schema, so it emits
no `AlterTableAddColumnEvent` and exercises the `Catalog.createTable` path
instead;
`DatabendCDCSinkIT` covers upserts and deletes, not DDL. Driving this code
from an E2E needs a CDC
source that emits a mid-stream DDL, which would mean adding a MySQL
container and a MySQL CDC job to
that module. I did not add that here: Docker is unavailable in my
environment, so I could not run
it even once, and a large unexecuted E2E seemed worse than saying so
plainly. Happy to add it in
this PR if a maintainer would rather have it.
### Related follow up, deliberately not in this PR
`DatabendCatalog.toDatabendTypeString` returns `"TIME"` as well, on the
`Catalog.createTable` path,
and the two sink documentation tables describe that mapping. I left both out
because that file is
being changed in #13023 and the test class those assertions belong in does
not exist on `dev` yet.
I will raise it separately once #13023 is resolved, so neither PR has to
resolve a conflict in the
other's new test file.
### Check list
* [X] Code changed are covered with tests, or it does not need tests for
reason
* [X] If any new Jar binary package adding in your PR, please add License
Notice according to
[License
Notice](https://github.com/apache/seatunnel/blob/dev/docs/en/contribution/new-license.md)
* [X] If necessary, please update the documentation to describe the new
feature: the two source
tables are corrected here.
* [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):
not needed, no new connector.
2. Update the pom file of
[seatunnel-dist](https://github.com/apache/seatunnel/blob/dev/seatunnel-dist/pom.xml):
not needed, no new module.
3. Add ci label in
[label-scope-conf](https://github.com/apache/seatunnel/blob/dev/.github/workflows/labeler/label-scope-conf.yml):
not needed, connector-databend is already labelled.
4. Add e2e test for your connector: see the note above, no IT drives this
path today.
5. Update [connector-v2
doc](https://github.com/apache/seatunnel/tree/dev/docs/en/connectors): done.
* [X] Update the `release-note`: not applicable, the file was removed from
`dev` in #9304.
--
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]
