fskorgen opened a new issue, #8216:
URL: https://github.com/apache/hop/issues/8216
### Apache Hop version?
2.19
### Java version?
21
### Operating system
Windows
### What happened?
Regression against 2.18 — introduced by `7ab2b1266` ("issue #7251:
fine-tuning of DDL generation", #7287).
The new hard-coded JDBC mapper redefines what `length` means for
DECIMAL/NUMERIC:
```java
// Database.java, getDataTypeFromKnownSqlType
length = rm.getPrecision(columnIndex);
precision = rm.getScale(columnIndex);
// Precision in the database means the number of significant digits.
// In Hop, it means the digits before the decimal.
if (length > 0 && length > precision) {
length -= precision;
}
```
`DECIMAL(24,15)` now yields Hop length 9, `DECIMAL(10,6)` yields length 4.
Before 2.19 both kept
the database precision as `length`.
The commit changed `Database.java`, `Const.java` and the docs only. No
`getFieldDefinition`
implementation was migrated to the new meaning. 30 dialects emit a scaled
`DECIMAL`/`NUMERIC`
of their own in 2.19.0, and 29 of them still pass Hop `length` through as
the database precision:
`as400`, `cache`, `clickhouse`, `db2`, `derby`, `doris`, `duckdb`,
`exasol4`, `firebird`,
`generic`, `h2`, `hive`, `hypersonic`, `impala`, `informix`, `ingres`,
`interbase`, `iris`,
`kingbasees`, `monetdb`, `mssql`, `mysql`, `sapdb`, `singlestore`,
`snowflake`, `sybase`,
`sybaseiq`, `teradata`, `universe`.
Only PostgreSQL matches the new meaning — `NUMERIC(length + precision,
precision)`
(`PostgreSqlDatabaseMeta`).
Subclasses that do not override `getFieldDefinition` follow their parent:
`mssqlnative`, `mariadb`
and `infobright` inherit the old meaning, `greenplum`, `redshift` and
`cockroachdb` inherit
PostgreSQL's.
### Steps to reproduce
No server needed — an in-memory H2 database is enough to read the metadata,
and `getFieldDefinition`
shows what each dialect would write back:
1. `create table t (d decimal(24,15), e decimal(10,6), g int)` on an
in-memory H2.
2. `Database.getQueryFields("select * from t")` gives:
```
d Number length=9 precision=15 (origType=3 DECIMAL)
e Number length=4 precision=6 (origType=3 DECIMAL)
g Integer length=9 precision=0
```
3. Ask the dialects what they write back for that same row metadata:
```
MSSQL / MSSQLNATIVE d -> DECIMAL(9,15) e -> DECIMAL(4,6)
MYSQL d -> DOUBLE e -> DOUBLE
POSTGRESQL d -> NUMERIC(24, 15) e -> NUMERIC(10, 6)
```
Only PostgreSQL round-trips. SQL Server rejects both `DECIMAL(9,15)` and
`DECIMAL(4,6)` because the
scale exceeds the precision. On MySQL the effect is quieter: the branch is
`length > 15 ? DECIMAL(length, precision) : DOUBLE`, so a column that used
to come back as an exact
`DECIMAL(24,15)` now becomes floating-point `DOUBLE`.
A pipeline that read and re-created a table in 2.18 therefore no longer
round-trips.
### The same change also alters the in-memory Hop type, on every database
The type choice below the subtraction is `if (length > 15 || precision > 15)
valtype =
TYPE_BIGNUMBER;`, and it now tests the reduced length. An exact decimal
whose integer part is 15
digits or fewer therefore drops from BigNumber (`java.math.BigDecimal`) to
Number
(`java.lang.Double`), silently, on read:
| Column | 2.19 (default) | `HOP_DB_DDL_COMPATIBLE=true` |
|---|---|---|
| `decimal(16,1)` | Number, length 15 | BigNumber, length 16 |
| `decimal(18,10)` | Number, length 8 | BigNumber, length 18 |
| `decimal(24,15)` | Number, length 9 | BigNumber, length 24 |
| `decimal(20,2)` | BigNumber, length 18 | BigNumber, length 20 |
| `decimal(30,20)` | BigNumber, length 10 | BigNumber, length 30 |
A `double` holds roughly 15-17 significant decimal digits, so every value in
a `decimal(24,15)`
column now goes through a type that cannot represent it exactly. This
affects reading alone — no DDL
generation, no SQL Server — and applies to any JDBC source with such a
column. The rows above were
produced against an in-memory H2 by flipping only `HOP_DB_DDL_COMPATIBLE`.
Reproduced against Apache Hop 2.19.0 using an in-memory H2 connection for
the read side and
`MsSqlServerDatabaseMeta`, `MySqlDatabaseMeta` and `PostgreSqlDatabaseMeta`
for DDL generation.
### Suggested fix
Two defects come out of the same three lines, and a fix has to address both.
*DDL generation.* Either revert the subtraction, or migrate every
`getFieldDefinition` (and any
other reader of `IValueMeta.getLength()` for numeric types) to the new
meaning. The first is the
backward-compatible one; the second is presumably what the change intended,
but it is a much wider
edit than one method: 29 dialects, plus every other reader of `getLength()`
on a numeric field.
*The read-side type change.* Migrating `getFieldDefinition` does not fix it.
The
`length > 15 || precision > 15` test sits below the subtraction and must be
evaluated against the
database precision whichever meaning `length` ends up carrying — otherwise
an exact
`decimal(24,15)` keeps arriving as `java.lang.Double` on every database, DDL
or no DDL. Reordering
the test above the subtraction is enough if the subtraction stays.
`HOP_DB_DDL_COMPATIBLE=true` restores the old behaviour, which is a good
escape hatch — but note it
is read once into a `static` field from a system property, so it cannot be
set per connection or
per pipeline.
### Issue Priority
Priority: 1
### Issue Component
Component: Database
--
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]
