Skip to content

Commit ba3f2cd

Browse files
committed
perf(parquet): make dictionary passthrough opt-in and restrict it to STRING
Addresses the review on #257. The compaction rewrite no longer turns `parquet.read.enable-dictionary-passthrough` on. It is a table option, off by default, and the rewrite only vetoes it - when the output is not Parquet, when `parquet.enable-dictionary` is false, or when a shredding writer is active - logging the reason at DEBUG for a table that did ask for it. Forwarding trades compaction CPU for output size, and a Parquet column chunk carries one dictionary, so a multi-file rewrite can keep the first input file's and write the rest of the row group plain; whether that trade is worth taking depends on the data, so it is left to the table. Restrict the reader to STRING. Parquet stores BINARY in the same BYTE_ARRAY leaf and dictionary-encodes it the same way, but no value accessor here can read a `dictionary(int32, binary)`: ColumnarUtils::GetView() asserts on it and returns an empty view in a release build, and LiteralConverter rejects it. Since the option applies to every read of the table, the gate gets that restriction, not the layout predicate - ArrowUtils::IsDictionaryLayoutRecoverableValueType() still accepts `utf8|binary`, which is what the writer can recover, so the writer keeps the capability for a future producer. Also from the review: use CastingUtils::Cast() instead of arrow::compute::Cast(); hold the Arrow pool adaptor in DataFileIndexWriter as a member declared before the index writers, so it outlives every buffer allocated through it; drop the batch schema cache in ParquetFormatWriter::ResolveBatchSchema(); read the two Parquet option names in core as local constants rather than through the format layer's headers. The changing-dictionary benchmark now rotates one alphabet and shifts the indices back by the same amount, so it writes the same logical column as its pair and the delta is the fallback rather than a difference in the data.
1 parent 450eed2 commit ba3f2cd

19 files changed

Lines changed: 809 additions & 246 deletions

benchmark/parquet_format_benchmark.cpp

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,14 +275,38 @@ Result<std::shared_ptr<arrow::Array>> MakeDictionaryColumn(
275275
return array;
276276
}
277277

278-
// STRING values: binary-like, so arrow can write the indices directly.
278+
// STRING values: binary-like, so arrow can write the indices directly. Every batch is built over
279+
// the same alphabet, the shape a single input file produces.
279280
Result<std::shared_ptr<arrow::Array>> MakeDictionaryStringColumn(int64_t num_rows, int64_t offset,
280281
int64_t cardinality) {
281282
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<arrow::Array> values,
282283
MakeStringColumn(cardinality, /*offset=*/0, cardinality));
283284
return MakeDictionaryColumn(values, num_rows, offset);
284285
}
285286

287+
// The same, except every batch carries its own dictionary - the shape a compaction rewrite
288+
// produces, where each input file supplies one. A Parquet column chunk holds a single dictionary,
289+
// so arrow keeps the first and falls back to plain encoding for the rest of the row group
290+
// (column_writer.cc, `dictionary->Equals(*preserved_dictionary_)`).
291+
//
292+
// The decoded column is byte for byte what MakeDictionaryStringColumn produces: same values, same
293+
// order, same widths, same cardinality. Only the dictionary object differs, so the delta between
294+
// the two benchmarks is the cost of the fallback and nothing else. Generating a fresh alphabet per
295+
// batch instead would change the data as well - different strings, different widths, a different
296+
// global cardinality - and the comparison would measure all of that at once.
297+
Result<std::shared_ptr<arrow::Array>> MakeChangingDictionaryStringColumn(int64_t num_rows,
298+
int64_t offset,
299+
int64_t cardinality) {
300+
// The alphabet is rotated by one position per batch and the indices are shifted the other way,
301+
// which cancels: index `(offset + cardinality - rotation + i) % cardinality` into an alphabet
302+
// whose entry `j` is `value_<(j + rotation) % cardinality>` is `value_<(offset + i) %
303+
// cardinality>` either way. `+ cardinality` keeps the shifted offset non-negative.
304+
const int64_t rotation = num_rows > 0 ? (offset / num_rows) % cardinality : 0;
305+
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<arrow::Array> values,
306+
MakeStringColumn(cardinality, rotation, cardinality));
307+
return MakeDictionaryColumn(values, num_rows, offset + cardinality - rotation);
308+
}
309+
286310
// INT32 values: is_base_binary_like excludes them, so arrow densifies before writing. The
287311
// dictionary holds the same `i * 7` values MakeInt32Column emits inline, so the two are directly
288312
// comparable.
@@ -979,6 +1003,10 @@ void BM_ParquetWrite_DictionaryString(::benchmark::State& state) {
9791003
// logical schema - and the batch arrives dictionary-encoded anyway. The delta against
9801004
// BM_ParquetWrite_String at the same cardinality is what the passthrough buys on the write side,
9811005
// including the per-batch schema fixup that recovers the encoding from the batch layout.
1006+
//
1007+
// One alphabet for the whole file, so the writer keeps writing indices: the favourable half of the
1008+
// passthrough. BM_ParquetWrite_ChangingDictionaryStringIntoStringSchema is the other half, and a
1009+
// rewrite of several input files lands between the two.
9821010
void BM_ParquetWrite_DictionaryStringIntoStringSchema(::benchmark::State& state) {
9831011
const int64_t cardinality = state.range(0);
9841012
RunWriteBenchmark(state, StringSchema(),
@@ -988,6 +1016,24 @@ void BM_ParquetWrite_DictionaryStringIntoStringSchema(::benchmark::State& state)
9881016
kRowsPerBatch, /*options=*/{}, kDefaultCompression);
9891017
}
9901018

1019+
// arg: dictionary cardinality. The same write, except every batch brings its own dictionary, as
1020+
// the input files of a compaction do, which is the cost of forwarding an encoding the writer
1021+
// cannot reuse. All three of BM_ParquetWrite_String,
1022+
// BM_ParquetWrite_DictionaryStringIntoStringSchema and this one write the same logical column at
1023+
// the same cardinality, so the triple reads directly: the middle one against the first is what the
1024+
// passthrough buys, this one against the middle is what the fallback costs in time, and this one
1025+
// against the first is the output size a rewrite that materialized and rebuilt would have produced.
1026+
// Row groups here are cut the way they are in production - by size and by the writer's memory
1027+
// limit, not at batch boundaries.
1028+
void BM_ParquetWrite_ChangingDictionaryStringIntoStringSchema(::benchmark::State& state) {
1029+
const int64_t cardinality = state.range(0);
1030+
RunWriteBenchmark(state, StringSchema(),
1031+
SingleEncodedColumnBatch([cardinality](int64_t rows, int64_t offset) {
1032+
return MakeChangingDictionaryStringColumn(rows, offset, cardinality);
1033+
}),
1034+
kRowsPerBatch, /*options=*/{}, kDefaultCompression);
1035+
}
1036+
9911037
// The same axis on an INTEGER dictionary, which arrow cannot direct-write - is_base_binary_like
9921038
// excludes int32, so it densifies first. Its baseline is BM_ParquetWrite_FlatInt32 at the same
9931039
// cardinality, not the String case: only the flat INT32 control holds value, width and encoding
@@ -1356,6 +1402,14 @@ BENCHMARK(BM_ParquetWrite_DictionaryStringIntoStringSchema)
13561402
->Arg(kRowsPerFile)
13571403
->Unit(benchmark::kMillisecond)
13581404
->UseRealTime();
1405+
// Same axis again, so the three points can be read against the run above them.
1406+
BENCHMARK(BM_ParquetWrite_ChangingDictionaryStringIntoStringSchema)
1407+
->ArgName("cardinality")
1408+
->Arg(10)
1409+
->Arg(1000)
1410+
->Arg(kRowsPerFile)
1411+
->Unit(benchmark::kMillisecond)
1412+
->UseRealTime();
13591413
BENCHMARK(BM_ParquetWrite_DictionaryInt32)
13601414
->ArgName("cardinality")
13611415
->Arg(10)

0 commit comments

Comments
 (0)