SEPURI-SAI-KRISHNA opened a new pull request, #13061:
URL: https://github.com/apache/seatunnel/pull/13061
Yes, glad to contribute to Apache SeaTunnel.
### Purpose of this pull request
Closes #13057.
When the Lance sink created a dataset, `SchemaUtils.convertSchema` declared
`BIGINT` columns as Arrow `int32`, and `IntTypeWriter` wrote them with
`Number.intValue()`. Any value outside the signed 32-bit range was wrapped
without an error: `3000000000` was stored as `-1294967296`, and the epoch
millisecond `1739586083123` as `124328243`.
`LanceTypeMapper.convertJsonArrowType`, used by the catalog's `createTable`,
had the same `int32` mapping.
**The fix.** Both mappings now declare `BIGINT` as `int64`. The writer
already handles 64-bit columns (`IntTypeWriter` writes them with
`longValue()`), and the catalog already turns `int64` into `Int(64)`, so no
writer change is needed.
Unchanged: `TINYINT`, `SMALLINT` and `INT` stay `int32`, and every option
stays the same.
Not covered, on purpose:
- A dataset created by an earlier version keeps its `int32` column, because
the sink writes to the schema of a dataset that already exists
(`LanceSinkWriter.initializeDataset`). Large values written there still wrap.
The documentation and the upgrade note say so. If maintainers prefer, a
follow-up can make a write to an `int32` column fail when the value does not
fit, instead of wrapping it.
- `LanceTypeMapper.convertDataType`, the read direction used by
`LanceCatalog.getTable`, still reports `int64` as `INT`. That is a separate
path and is unchanged here.
### Does this PR introduce any user facing change?
Yes. A dataset the Lance sink creates now has an `int64` column for
`BIGINT`, where it had `int32`, and large values are stored correctly. The type
mapping table and its note in the en and zh sink documentation are updated, and
the change is recorded in `incompatible-changes.md` in en and zh.
### How was this patch tested?
**Unit tests**, two new cases in the existing `FragmentConverterTest`:
- `shouldKeepEveryBitOfABigint`: the schema `SchemaUtils.convertSchema`
builds when the sink creates the dataset, then
`FragmentConverter.convertToVectorSchemaRoot` with `3000000000`, `-3000000000`,
`1739586083123`, `Long.MAX_VALUE` and `Long.MIN_VALUE`. It asserts the column
is `Int(64)` and every value is stored unchanged.
- `shouldDeclareABigintColumnAsInt64InTheCatalogSchema`:
`SchemaUtils.convertJsonArrowSchema`, which the catalog's `createTable` uses,
declares a `BIGINT` column as `int64`.
Mutation check: with `SchemaUtils.java` and `LanceTypeMapper.java` reverted
to `dev`, both tests fail, with `expected: <Int(64, true)> but was: <Int(32,
true)>` and `expected: <int64> but was: <int32>`.
**E2E.** `LanceIT.testBigintKeepsItsValue` runs the new
`fake_bigint_to_lance.conf`, which writes four rows with values outside the
int32 range, then reads the dataset back in the test JVM and compares every
value. It uses the same read-back as `testTimestampKeepsItsMicroseconds`, so it
fails rather than skips when the dataset cannot be read. On `dev` the column is
`int32`, so the read-back fails.
Docker is not available to me, so the job itself has not run locally.
`LanceIT.java` compiles, and the new configuration parses.
**Build checks**, JDK 17:
- `./mvnw clean test -pl seatunnel-connectors-v2/connector-lance`, with the
`surefire.jvm.args` the unit-test CI lane passes: 41 tests, 0 failures.
- `./mvnw spotless:check` on `connector-lance` and `connector-lance-e2e`:
clean.
- `./mvnw -B -T 1 clean test -D"license.skipAddThirdParty"=true -pl
seatunnel-ci-tools`: 34 tests, 0 failures.
### 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):
none added.
* [X] If necessary, please update the documentation to describe the new
feature: the Lance sink type mapping is updated in en and zh, and
`incompatible-changes.md` in en and zh.
* [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 changed, outside this fix.
4. Add e2e test for your connector: `LanceIT` is extended.
5. Update [connector-v2
doc](https://github.com/apache/seatunnel/tree/dev/docs/en/connectors): the
Lance sink document is updated.
* [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]
