SteNicholas opened a new issue, #229: URL: https://github.com/apache/paimon-cpp/issues/229
### Search before asking - [x] I searched in the [issues](https://github.com/apache/paimon-cpp/issues) and found nothing similar. ### Motivation Java Paimon and paimon-cpp write the same logical `DECIMAL` column to two different Parquet physical types. Java Paimon (`paimon-format/src/main/java/org/apache/paimon/format/parquet/ParquetSchemaConverter.java`, `case DECIMAL`): - precision <= 9 → `INT32` - precision <= 18 → `INT64` - otherwise → `FIXED_LEN_BYTE_ARRAY` paimon-cpp: `ParquetWriterBuilder::PrepareWriterProperties` (`src/paimon/format/parquet/parquet_writer_builder.cpp:54`) never enables arrow's `store_decimal_as_integer`, which defaults to `false` (`cpp/src/parquet/properties.h:239`). So arrow writes **every** decimal as `FIXED_LEN_BYTE_ARRAY`, regardless of precision (`cpp/src/parquet/arrow/schema.cc:362`). Three consequences: 1. **Physical-layout divergence.** For the same table schema, a file written by Flink/Spark Paimon and a file written by paimon-cpp differ in physical type for every decimal column with precision <= 18. Both are readable by both engines, but the divergence is invisible in the table schema and surfaces only in the footer — awkward for tooling, file-level diffing and any future code that keys off physical type. 2. **Slower reads of paimon-cpp-written files.** An FLBA decimal goes through `TransferDecimal` in arrow's `cpp/src/parquet/arrow/reader_internal.cc`, which materializes a `BinaryArray` (offsets + data) and then converts per value via `Decimal128::FromBigEndian`. The INT32/INT64 path (`DecimalIntegerTransfer`, same file) reads a fixed-width column directly with no intermediate array. 3. **Larger and less compressible on disk.** An FLBA column stores `DecimalSize(precision)` bytes per value with no integer-friendly encoding available, whereas an INT32/INT64 column can use `RLE_DICTIONARY` or delta encodings on the raw integers. The read side of paimon-cpp already handles all three physical types, so this is a write-side-only gap: `ParquetStatsExtractor` extracts DECIMAL min/max for `INT32`, `INT64` and `FIXED_LEN_BYTE_ARRAY` / `BYTE_ARRAY` alike (`src/paimon/format/parquet/parquet_stats_extractor.cpp:174-201`), and value decoding is arrow's job either way. ### Solution 1. Enable `enable_store_decimal_as_integer()` on the `::parquet::WriterProperties::Builder` in `ParquetWriterBuilder::PrepareWriterProperties`. Arrow applies it only for `1 <= precision <= 18` and picks `INT32` for `precision <= 9`, `INT64` otherwise (`cpp/src/parquet/arrow/schema.cc:362`) — which is exactly Java Paimon's `is32BitDecimal` / `is64BitDecimal` split, so no additional mapping logic is needed on our side. 2. Decide the default. Aligning with Java Paimon argues for defaulting it **on**; that changes the physical layout of all newly written files, so it is worth an explicit decision rather than a silent flip. A `parquet.write.store-decimal-as-integer` option can cover the escape hatch either way. Files already written stay readable — arrow's reader handles all three physical types, as does parquet-mr. 3. Tests: - round-trip `DECIMAL(9, 2)` / `DECIMAL(18, 4)` / `DECIMAL(38, 10)` and assert the physical type in the footer for each; - stats extraction over the new layout (the `INT32` / `INT64` branches at `parquet_stats_extractor.cpp:180-191` currently only ever run against Java-written files, so this also gives them first-party coverage); - predicate pushdown on decimal columns still prunes row groups; - cross-engine check: read a paimon-cpp-written file from Java Paimon and vice versa. ### Anything else? This came out of evaluating [velox#17994](https://github.com/facebookincubator/velox/issues/17994) against paimon-cpp. [velox#17992](https://github.com/facebookincubator/velox/pull/17992) adds a SIMD bulk path for `PLAIN` `INT64` short decimals — the equivalent in arrow is `DecimalIntegerTransfer` (`cpp/src/parquet/arrow/reader_internal.cc:645`), which converts one value at a time (`Decimal128 decimal(value); decimal.ToBytes(out_ptr);`). Vectorizing it would require an addition to `cmake_modules/arrow.diff`. That is deliberately **not** proposed here: the loop is already cheap, the gain is unquantified, and every entry in `arrow.diff` is maintenance debt at the next arrow upgrade. It should only be revisited if the format-level benchmarks (#228) show the decimal transfer is actually material. Switching the writer to INT32/INT64 is the change that matters, and it is a few lines in paimon-cpp with no arrow patch at all. ### Are you willing to submit a PR? - [x] I'm willing to submit a PR! -- 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]
