SteNicholas opened a new pull request, #257:
URL: https://github.com/apache/paimon-cpp/pull/257

   ### Purpose
   
   Linked issue: close #230
   
   An append-only compaction rewrite copies rows into the new file without 
inspecting any value, so expanding a dictionary-encoded Parquet column on read 
and hashing it again on write is work neither side needs. This forwards the 
encoding instead.
   
   **Reader.** New option `parquet.read.enable-dictionary-passthrough`, **off 
by default**, makes `ParquetFileBatchReader` request `set_read_dictionary` for 
non-nested `STRING`/`BINARY` columns whose every data page, in every row group 
of the file, is dictionary-encoded. A dictionary page alone cannot be the 
signal — a column that outgrew its page limit still carries the page it had 
already emitted — so the gate reads the encoding statistics. The file schema 
keeps reporting the logical type; only the emitted batches carry the dictionary.
   
   **Writer.** `ParquetFormatWriter` recovers each batch's encoding from its 
layout, because exporting through the Arrow C data interface drops the type. A 
layout pins down neither the index nor the offset width, so only 
`dictionary(int32, utf8|binary)` — exactly what Arrow's Parquet reader emits — 
is recoverable. `CompactRewrite` therefore decodes anything else **per column, 
while the type is still known**: the ORC reader's `dictionary(int64, 
large_utf8)` under lazy decoding, dictionaries below the top level, non-binary 
value types. The rest stay encoded, so one awkward column does not cost the 
others their encoding. A dictionary holding nulls in its values is flattened at 
the writer — the one shape `parquet::arrow` rejects outright.
   
   **Gating.** Compaction opts in only when the output is Parquet, 
`parquet.enable-dictionary` is on, and no shredding plan is active; anything 
else forces the read option off regardless of the table setting. A file index 
configured on a forwarded column materializes that column alone.
   
   **Known trade-off.** A Parquet column chunk carries one dictionary, so when 
the input files supply different ones the output keeps the first and falls back 
to plain for the rest of the row group. The rewritten data is unchanged, but 
the output file may be larger than one written from materialized values. This 
is documented in `compaction.rst` and pinned by 
`TestWriteDictionaryChangingAcrossBatches`. Whether it is worth mitigating (for 
example by starting a new row group at each dictionary boundary) should be 
decided from the benchmark numbers below.
   
   Design follows Velox's `perf(parquet): Dictionary passthrough and selective 
flattening in Parquet writer` (facebookincubator/velox#17986): per-column 
selective flatten, only VARCHAR/VARBINARY passed through, dictionaries with 
null values flattened, import schema reconciled per batch. It differs in where 
the flatten happens — Velox still holds a typed `Vector` inside its writer, 
whereas `FormatWriter::AddBatch(ArrowArray*)` here receives an untyped array, 
so the flatten has to run one layer up, before the type is dropped.
   
   ### Tests
   
   Unit:
   
   - `ArrowUtilsTest.TestResolveParquetDictionaryStructType` — layout-derived 
resolution; rejects non-`STRING`/`BINARY` value types, `large_utf8`, and 
dictionaries below the top level; preserves a caller-declared dictionary type.
   - `ArrowUtilsTest.TestFlattenUnresolvableDictionaries` — selective flatten: 
`dictionary(int64, large_utf8)` and `dictionary(int64, utf8)` decoded while the 
`int32` neighbour stays encoded; nested dictionary decoded; unchanged batch 
returned by identity; sliced batch keeps its offset.
   - `ReaderUtilsTest.TestApplyBitmapToReadBatchKeepsDictionaryEncoding` — a 
deletion vector filters batches by slice + `arrow::Concatenate`; pins that the 
encoding survives it.
   - `DataFileIndexWriterTest.TestDictionaryEncodedIndexedColumnRoundTrip` — 
bitmap index built from a forwarded column.
   - `ParquetFileBatchReaderTest.TestDictionaryPassthrough` — on / explicitly 
off / option absent / file without dictionary pages.
   - `ParquetFileBatchReaderTest.TestDictionaryPassthroughSkipsFallbackToPlain` 
— a column that falls back to plain inside one chunk is declined.
   - 
`ParquetFileBatchReaderTest.TestDictionaryPassthroughRequiresEveryRowGroup` — 
first row group fully dictionary-encoded, second falls back; the whole column 
is declined.
   - `ParquetFormatWriterTest.TestWriteDictionaryEncodedColumn` / 
`ChangingAcrossBatches` / `WithNullsInDictionary` / `WithNullRows` / 
`WithDuplicateValues` / `EmptyBatch` / `OfUnsupportedTypeIsRejected`.
   - `ParquetFormatWriterTest.TestGetEstimateLengthWithDictionaryBatches` — 
`GetEstimateLength()` and `ReachTargetSize()` still drive file rolling when 
batches arrive encoded.
   
   Integration:
   
   - `AppendCompactionInteTest.TestAppendTableCompactionDictionaryPassthrough` 
(Parquet + ORC) — asserts the input read types (`id` INT32, `s`/`b` 
`dictionary(int32, utf8)` on Parquet, `s` `dictionary(int64, large_utf8)` on 
ORC), then the full rewrite, then a predicate read through the bitmap index.
   - 
`AppendCompactionInteTest.TestAppendTableCompactionDictionaryPassthroughDisabled`
 — the kill switch produces the same table.
   
   Benchmarks (`benchmark/parquet_format_benchmark.cpp`):
   
   - `BM_ParquetWrite_DictionaryStringIntoStringSchema` — the shape a rewrite 
produces (plain `STRING` write schema, dictionary-encoded batch), on the same 
`10 / 1000 / kRowsPerFile` axis as its `BM_ParquetWrite_String` baseline.
   - `BM_ParquetRead_DictionaryPassthrough` — the same axis with the option on 
and off at each cardinality; the high-cardinality point is where the gate 
declines and the two runs should measure the same work.
   
   > [!IMPORTANT]
   > **No build, test, benchmark or `pre-commit` run was performed for this 
patch.** Only source review and static `git diff --check` were done, at the 
author's request. The tests and benchmarks above have never been executed. In 
particular these fixtures rest on Arrow behaviour that was read from the Arrow 
17.0.0 sources but not observed: `max_row_group_length` / `write_batch_size` 
producing the intended row-group and fallback boundaries, `NewBufferedRowGroup` 
flushing so the size estimate moves, `orc.dictionary-key-size-threshold` making 
the ORC reader emit a dictionary under lazy decoding, and `arrow::Concatenate` 
taking its same-dictionary fast path. Reviewers should treat CI as the first 
execution, and the performance claim as unmeasured — the benchmarks exist but 
produced no numbers.
   
   ### API and Format
   
   No public API under `include/` changed. `ArrowUtils` (internal, 
`PAIMON_EXPORT`) gains `IsParquetDictionaryValueType`, 
`ResolveParquetDictionaryStructType` and `FlattenUnresolvableDictionaries`; two 
private `AppendOnlyFileStoreWrite` helpers changed signature.
   
   No storage format or protocol change. Output files remain standard Parquet — 
a forwarded dictionary is written through Arrow's ordinary dictionary path, and 
a file written with the option on is readable by any reader.
   
   One new table option, `parquet.read.enable-dictionary-passthrough`, default 
`false`. The append compaction rewrite turns it on for itself when eligible and 
forces it off when not; setting it to `false` on the table disables the 
optimization.
   
   ### Documentation
   
   Yes — `docs/source/user_guide/compaction.rst` gains a "Dictionary 
Passthrough" section covering the scope (append-only only), per-file 
eligibility, the three gating conditions, file-index behaviour, the 
plain-fallback trade-off and the kill switch.
   
   ### Generative AI tooling
   
   Generated-by: Claude Code (Claude Opus 5)
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to