tomnewton opened a new pull request, #39846:
URL: https://github.com/apache/beam/pull/39846
ClickHouseIO's `TableSchema` and column-type parser had no notion of
ClickHouse's fixed-point `Decimal` family, so `ClickHouseIO.getTableSchema()`
failed on the `DESCRIBE TABLE` output of any table containing a Decimal column
and pipelines carrying monetary/fixed-point values could not write to
ClickHouse at all.
This change adds first-class `Decimal(P, S)` support to `ClickHouseIO`:
* **Schema model** — new `TypeName.DECIMAL`; `ColumnType` carries `scale`
alongside the existing `precision` (validated: P in [1, 76], S in [0, P]), with
a `ColumnType.decimal(precision, scale)` factory. Bare `Decimal` is
`Decimal(10, 0)` and `Decimal(P)` is `Decimal(P, 0)`, matching ClickHouse's
defaults.
* **Parser** — JavaCC grammar rules for `Decimal[(P[, S])]` and the width
aliases `Decimal32(S)` / `Decimal64(S)` / `Decimal128(S)` / `Decimal256(S)`
(pinned to precisions 9/18/38/76 per ClickHouse's synonym rule; `DESCRIBE
TABLE` canonicalizes the aliases to `Decimal(P, S)` anyway). Reachable through
`Nullable(...)` and `Array(...)` via the existing `primitive()` rule. Lexical,
syntactic and range-validation failures all surface as the parser's uniform
`failed to parse` error.
* **Beam field-type mapping** — `Decimal(P, S)` maps to
`FixedPrecisionNumeric.of(P, S)` (base type `DECIMAL`, values are
`BigDecimal`), preserving the declared precision/scale through
`getEquivalentSchema` and giving an early, loud failure at Row construction for
values whose digits genuinely exceed the declared precision. This is the same
logical type JdbcIO uses for NUMERIC.
* **Writer** — serializes the unscaled value as a little-endian signed
integer whose width (32/64/128/256 bits) is chosen from the declared precision,
delegating to the ClickHouse client's `BinaryStreamUtils.writeDecimal`.
Fractional digits beyond the column scale are truncated toward zero, consistent
with ClickHouse's own "excessive digits in a fraction are discarded (not
rounded)" semantics; values exceeding the storage width fail loudly.
* **Defaults** — `ColumnType.parseDefaultExpression` handles Decimal
literals (`DEFAULT 1.23`), so `getTableSchema` no longer throws for tables with
defaulted Decimal columns, and `writeRow`'s existing null-substitution works
for them.
Tests:
* `TableSchemaTest` — parser cases for `Decimal(10, 2)`, `Decimal(5)`, bare
`Decimal`, all four width aliases, `Nullable(Decimal(10, 2))`,
`Array(Decimal(38, 10))`; uniform `failed to parse` errors for `Decimal(77,
2)`, `Decimal(0)`, `Decimal(9, 10)`, `Decimal(9, -1)` and `Decimal(abc)`;
factory range validation; schema-mapping tests including nullable; a test
pinning that Row construction under the mapped `FixedPrecisionNumeric` type
rejects values exceeding the declared precision (the layer that guards the
declared range, since the writer checks only the storage width);
`parseDefaultExpression` for positive and negative literals.
* `ClickHouseWriterTest` — byte-level tests locking the wire format for
every width bucket: little-endian unscaled Int32, scaling below the column
scale, 16 × 0xFF two's-complement sign extension for a negative Decimal128, a
32-byte Decimal256 encoding, truncation-toward-zero of excess fractional
digits, storage-width overflow rejection, and null/non-null through
`writeNullableValue`.
* `ClickHouseIOIT` — round-trip integration tests against the ClickHouse
test container for `Decimal(9, 2)`, `Decimal(18, 4)`, `Decimal(38, 10)` and
`Decimal(76, 20)` (including negative values), `Nullable(Decimal(10, 2))` with
a null row, `Array(Decimal(9, 2))`, a `DEFAULT 2.25` column exercised
end-to-end by writing a null row, a `getTableSchema` round-trip over a table
declared with the width aliases (verifying we parse the server's canonicalized
`DESCRIBE` output), the user-visible truncation of over-scaled values (`-1.239`
→ `-1.23`), and a test pinning that a binary insert exceeding the declared
precision but fitting the storage width is stored and read back verbatim
(ClickHouse checks the declared range only on conversion from a string).
fixes #39840
------------------------
Thank you for your contribution! Follow this checklist to help us
incorporate your contribution quickly and easily:
- [x] Mention the appropriate issue in your description (for example:
\`addresses #123\`), if applicable. This will automatically add a link to the
pull request in the issue. If you would like the issue to automatically close
on merging the pull request, comment \`fixes #<ISSUE NUMBER>\` instead.
- [x] Update \`CHANGES.md\` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more
tips on [how to make review process
smoother](https://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
To check the build health, please visit
[https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
--
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]
