zhengxiang378928908-code commented on issue #12814:
URL: https://github.com/apache/seatunnel/issues/12814#issuecomment-6207773389
@SEZ9 @DanielLeens My lean is **C for top-level fields + B for nested
types**. Below is how it would cover each path in
`seatunnel-translation-spark-common`, from reading the current `dev` code.
**One thing that shapes the design:** row conversion in both directions is
driven by the SeaTunnel row type of the table, not by the Spark `StructType`.
`InternalRowConverter.convert` / `reconvert` switch on
`SeaTunnelDataType.getSqlType()`, and `MultiTableManager` keeps one serializer
per table id built from its `CatalogTable`. I found no main-code caller of
`TypeConverterUtils.convert(DataType)` (Spark schema → SeaTunnel) outside the
class itself, so the sink side does not rebuild its schema from Spark metadata
today. The metadata in C is still worth carrying so that the Spark schema
round-trips on its own, but the value conversion does not depend on it.
**Proposed handling per path** (`DECIMAL(p, s)` with `p > 38`, called "wide
decimal" below):
1. **Schema (SeaTunnel → Spark), `TypeConverterUtils.convert`:**
- Top-level field, including fields of a nested `ROW` (they go through
`convert(SeaTunnelRowType)` and get `StructField` metadata): `StringType` plus
metadata `seatunnel_decimal_precision = p`, `seatunnel_decimal_scale = s`.
- Element of `ARRAY`, or key/value of `MAP`: fail before any row is read
with a `SeaTunnelRuntimeException` that names the table, the column path (e.g.
`t.col.element`, `t.col.key`), the declared precision, the Spark limit of 38,
and the workaround (`Sql` transform `CAST(col AS STRING)`, or `CAST(... AS
DECIMAL(38, 0))` if the range allows).
2. **Row write (SeaTunnel → Spark), `InternalRowConverter.convert`, `case
DECIMAL`:** wide decimal → `UTF8String.fromString(value.toPlainString())`; `p
<= 38` unchanged (`Decimal.apply`).
3. **Row read (Spark → SeaTunnel), `InternalRowConverter.reconvert`, `case
DECIMAL`:** `engineRow.get(i, convert(fieldType))` returns a `UTF8String` for a
wide decimal, so add `UTF8String` → `new BigDecimal(field.toString())`, then
check that the scale matches the declared type. Side note: this case currently
falls through into `case ARRAY` when the value is neither `Decimal` nor
`BigDecimal`; the PR would add the missing `break` / error.
4. **Schema read-back (Spark → SeaTunnel), `convert(StructType)`:** if the
decimal metadata is present, return `DecimalType(p, s)`, the same way the
`LOGICAL_TIME_TYPE_FLAG` and `TIMESTAMP_TZ` flags are handled now.
5. **Nested types:** covered by the preflight failure in (1).
**Open trade-off on nested types.** Because rows are converted from the
SeaTunnel type, `ARRAY` elements and `MAP` keys/values could use the same
string carrier (`ArrayType(StringType)`, with the converters handling the
elements). The only loss is that the Spark schema alone would not show the
original type, since array elements and map entries cannot carry `StructField`
metadata. With B for nested types, a Doris `ARRAY<largeint>` / `MAP<...,
largeint>` column on Spark goes from working on `DECIMAL(20, 0)` to a clear
failure. That is still a regression for those columns. I'd keep nested as B in
the first PR to limit scope, unless you'd rather cover nested with the carrier
from the start.
**Tests and docs for that PR:**
- Unit tests for schema conversion and row round trip at the top level,
inside a nested `ROW`, and the preflight failure for `ARRAY` / `MAP`. Values:
`-2^127`, `2^127 - 1`, `0`, `NULL`, and a non-zero scale such as `DECIMAL(50,
10)`. Run on Spark 2.4 and 3.3.
- Keep the `DorisIT` / `DorisMultiReadIT` Spark containers enabled, and add
a Spark case for a wide decimal reaching a sink with the full value.
- EN/ZH docs for Spark behavior with `DECIMAL` precision > 38, plus an
`incompatible-changes.md` entry for the Spark-visible type change.
Still open from the issue: whether the MySQL `DECIMAL(65)` → `DECIMAL(38,
18)` clamp belongs here or in a separate issue. I'd track it separately,
because it is a source-side clamp that happens on every engine, not only Spark.
--
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]
