chovy-3012 commented on PR #13061:
URL: https://github.com/apache/seatunnel/pull/13061#issuecomment-6539230584
**Issue 1 (Medium, non-blocking) — Read path maps `int64` to SeaTunnel `INT`
(32-bit)**
Location: `LanceTypeMapper.java:48-57` (`convertDataType`)
```java
case "int": case "int8": case "int16": case "int32":
case "int64": // falls through with every int variant
case "uint8": ... case "uint64":
return BasicType.INT_TYPE; // 32-bit, regardless of width
```
Call chain: `LanceCatalog.getTable:160 → convertTableSchema:363 →
SchemaUtils.toSeaTunnelType:55 → convertDataType:43`. This PR fixes the write
direction (`BIGINT → int64`) but leaves the read direction unchanged, so
`getTable` reports a `BIGINT` column as `INT` (32-bit). Before this PR:
write=`int32` / read=`INT` — both wrong but width-consistent. After:
write=`int64` (correct) / read=`INT` (wrong) — the width mismatch is newly
introduced. A downstream sink building DDL from `LanceCatalog.getTable` would
create an `INT` column and truncate on insert; roundtrip fidelity is also
broken (create `BIGINT` → `getTable` returns `INT` → recreate yields `int32`,
reintroducing the truncation this PR fixes). Suggested fix:
```java
case "int64":
case "uint64":
return BasicType.LONG_TYPE;
```
Not raised by another reviewer.
---
**Issue 2 (Low, non-blocking) — Fallback converter collapses all
`ArrowType.Int` to `"int32"`**
Location: `LanceCatalog.java:449-450` (`convertArrowSchemaToJsonArrowSchema`)
```java
if (arrowType instanceof org.apache.arrow.vector.types.pojo.ArrowType.Int) {
jsonType.setType("int32"); // ignores getBitWidth()
}
```
When `describeTable` returns an empty schema, `getTable` (lines 173-186)
falls back to the dataset's Arrow schema and runs this converter, which
relabels any `ArrowType.Int` as `"int32"` regardless of bit width — so an
`int64` column becomes `"int32"` *before* reaching `convertDataType`. Even
after Issue 1 is fixed, this fallback still mislabels `int64`.
`convertJsonArrowTypeToArrowType:518-534` already handles every width
correctly, so the asymmetry looks like an oversight. Suggested fix:
```java
jsonType.setType(((ArrowType.Int) arrowType).getBitWidth() == 64 ? "int64" :
"int32");
```
Not raised by another reviewer.
---
**Issue 3 (Low, non-blocking) — E2E read-back keyed by `id` masks duplicate
rows**
Location: `LanceIT.java`, new `readBigints()` method
```java
Map<Integer, Long> values = new HashMap<>();
...
values.put(ids.get(i), bigints.get(i)); // keyed by id; duplicates
silently overwrite
```
If the sink ever duplicates rows (retry / `Append` misconfig), the `Map`
keeps only the last entry per id, so `assertEquals(expected, readBigints())`
can still pass. The sibling `testInsertAndCheckDataE2e:116` already shows the
row-count pattern. Suggested fix: add `assertEquals(expected.size(),
totalRows)` alongside the map equality assertion. Raised by DanielLeens.
--
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]
