Skip to content

Commit d123bf7

Browse files
committed
perf(parquet): decode every dictionary when the passthrough is vetoed
Addresses the review on #257. `FlattenUnresolvableDictionaries()` decided what to decode from whether a dictionary survives the Arrow C data interface, but what decides it is whether the writer receiving the batch recovers an encoding from the layout at all, and only ParquetFormatWriter does. A reader can hand over a dictionary for reasons the Parquet passthrough option does not govern - a format's own lazy-decoding setting - and those stay on when the rewrite vetoes the passthrough. A `dictionary(int32, utf8)` column could therefore reach a writer that imports it against the logical type, where a dictionary child fails on the buffer count. The built-in ORC reader widens to `dictionary(int64, large_utf8)`, which was already decoded for its shape, so reaching this needs a reader outside this repository. CompactRewrite now derives `preserve_layout_recoverable_dictionaries` from the veto: under a veto every dictionary is decoded before the export, whatever its shape. This also stops a dictionary reaching a Parquet writer whose table sets `parquet.enable-dictionary` to false. Rename ArrowUtils::ResolveParquetDictionaryStructType() to ResolveDictionaryStructTypeFromLayout(), and the file-local IsResolvableDictionary() to IsDictionaryLayoutRecoverable(). Neither is Parquet-specific: they read `ArrowArray::dictionary` and the caller's declared type, and DataFileWriterBase::AddFileIndexBatch runs the former for every format. Moving it into the Parquet layer instead would make core depend on Parquet symbols.
1 parent dc2c759 commit d123bf7

8 files changed

Lines changed: 131 additions & 54 deletions

File tree

docs/source/user_guide/compaction.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,11 @@ the values again, or when variant/map shredding is configured because those
126126
writers reshape each batch against a fixed physical schema. Setting the option
127127
therefore never makes a rewrite fail; at worst it has no effect.
128128

129+
When the option is vetoed, the rewrite also enforces the veto on every input
130+
batch. A format reader can independently hand over dictionary-encoded columns
131+
because of its own lazy-decoding setting, so the rewrite decodes those columns
132+
before handing them to a writer that cannot accept dictionary arrays.
133+
129134
If a file index is configured on a forwarded column, that column alone is
130135
materialized so the index still sees its values; the other columns stay encoded.
131136

src/paimon/common/utils/arrow/arrow_utils.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ namespace {
4444
// Whether `type` is a dictionary this can carry across the C data interface unchanged. The index
4545
// width is part of the test because nothing in a layout reveals it; see
4646
// ArrowUtils::IsDictionaryLayoutRecoverableValueType().
47-
bool IsResolvableDictionary(const arrow::DataType& type) {
47+
bool IsDictionaryLayoutRecoverable(const arrow::DataType& type) {
4848
if (type.id() != arrow::Type::DICTIONARY) {
4949
return false;
5050
}
@@ -558,7 +558,7 @@ Result<arrow::Compression::type> ArrowUtils::GetCompressionType(const std::strin
558558
// `int32` indices over `int32` offsets would silently reinterpret both buffers instead of failing.
559559
//
560560
// This narrows what may be carried; it cannot verify what was. See
561-
// ResolveParquetDictionaryStructType() for where the index width becomes a caller contract.
561+
// ResolveDictionaryStructTypeFromLayout() for where the index width becomes a caller contract.
562562
bool ArrowUtils::IsDictionaryLayoutRecoverableValueType(const arrow::DataType& type) {
563563
return arrow::is_binary_like(type.id());
564564
}
@@ -574,7 +574,7 @@ bool ArrowUtils::IsDictionaryLayoutRecoverableValueType(const arrow::DataType& t
574574
// batch whose dictionaries the schema does not declare, and it honours the contract by running
575575
// FlattenUnresolvableDictionaries() first. Closing the hole instead of narrowing it needs the real
576576
// `ArrowSchema` to reach the writer, which `FormatWriter::AddBatch(ArrowArray*)` drops.
577-
Result<std::shared_ptr<arrow::DataType>> ArrowUtils::ResolveParquetDictionaryStructType(
577+
Result<std::shared_ptr<arrow::DataType>> ArrowUtils::ResolveDictionaryStructTypeFromLayout(
578578
const std::shared_ptr<arrow::DataType>& logical_type, const ::ArrowArray* batch) {
579579
if (batch == nullptr || logical_type->id() != arrow::Type::STRUCT ||
580580
batch->n_children != logical_type->num_fields()) {
@@ -619,7 +619,8 @@ Result<std::shared_ptr<arrow::DataType>> ArrowUtils::ResolveParquetDictionaryStr
619619

620620
Result<std::shared_ptr<arrow::StructArray>> ArrowUtils::FlattenUnresolvableDictionaries(
621621
const std::shared_ptr<arrow::StructArray>& batch,
622-
const std::shared_ptr<arrow::DataType>& logical_type, arrow::MemoryPool* pool) {
622+
const std::shared_ptr<arrow::DataType>& logical_type, arrow::MemoryPool* pool,
623+
bool preserve_layout_recoverable_dictionaries) {
623624
const std::shared_ptr<arrow::DataType>& batch_type = batch->type();
624625
if (logical_type->id() != arrow::Type::STRUCT || !HasDictionary(*batch_type)) {
625626
return batch;
@@ -629,7 +630,13 @@ Result<std::shared_ptr<arrow::StructArray>> ArrowUtils::FlattenUnresolvableDicti
629630
arrow::FieldVector fields = batch_type->fields();
630631
for (int32_t i = 0; i < batch_type->num_fields(); ++i) {
631632
std::shared_ptr<arrow::Field> field = fields[i];
632-
if (IsResolvableDictionary(*field->type()) || !HasDictionary(*field->type())) {
633+
if (!HasDictionary(*field->type())) {
634+
continue;
635+
}
636+
// Surviving the export is not enough when the destination imports against the logical
637+
// type: an undeclared dictionary child of any shape then fails on the buffer count.
638+
if (preserve_layout_recoverable_dictionaries &&
639+
IsDictionaryLayoutRecoverable(*field->type())) {
633640
continue;
634641
}
635642
std::shared_ptr<arrow::Field> logical_field =

src/paimon/common/utils/arrow/arrow_utils.h

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,14 @@ class PAIMON_EXPORT ArrowUtils {
8686
/// @return True when `dictionary(int32(), type)` round-trips through an `ArrowArray`.
8787
static bool IsDictionaryLayoutRecoverableValueType(const arrow::DataType& type);
8888

89-
/// Recovers the struct type of a batch that Arrow's Parquet reader produced with
90-
/// `set_read_dictionary` enabled: `logical_type` with every top-level field whose matching
91-
/// child in `batch` carries a dictionary replaced by `dictionary(int32(), field type)`, or
92-
/// `logical_type` itself when no child is dictionary-encoded.
89+
/// Recovers dictionary fields omitted from a batch's declared logical type by inspecting its
90+
/// layout: `logical_type` with every top-level field whose matching child in `batch` carries a
91+
/// dictionary replaced by `dictionary(int32(), field type)`, or `logical_type` itself when no
92+
/// child is dictionary-encoded.
9393
///
9494
/// The `int32` index width is assumed rather than inferred, so this is a contract on whoever
95-
/// produces the batch, not a check the callers can rely on: the producer must either hand on a
96-
/// batch that came straight from Arrow's Parquet reader, or run
95+
/// produces the batch, not a check the callers can rely on: the producer must either provide a
96+
/// batch whose dictionaries all have `int32` indices, or run
9797
/// FlattenUnresolvableDictionaries() while the type is still known. The definition spells out
9898
/// what that buys and what it does not.
9999
///
@@ -106,27 +106,33 @@ class PAIMON_EXPORT ArrowUtils {
106106
/// @param batch Only its structure is inspected, never its data, and it is not consumed.
107107
/// @return `logical_type` or a copy of it carrying the recovered dictionary fields, or
108108
/// NotImplemented for a dictionary this cannot describe.
109-
static Result<std::shared_ptr<arrow::DataType>> ResolveParquetDictionaryStructType(
109+
static Result<std::shared_ptr<arrow::DataType>> ResolveDictionaryStructTypeFromLayout(
110110
const std::shared_ptr<arrow::DataType>& logical_type, const ::ArrowArray* batch);
111111

112-
/// Returns `batch` with every top-level column that ResolveParquetDictionaryStructType() could
113-
/// not resolve decoded to the type its field carries in `logical_type`. A column it can
114-
/// resolve stays dictionary-encoded, so one column that has to be decoded does not cost the
115-
/// others their encoding, and a batch that needs no decoding is returned unchanged.
112+
/// Returns a copy of `batch` in which every top-level column that cannot be preserved for the
113+
/// destination has been decoded to the type its field carries in `logical_type`. A layout-
114+
/// recoverable column may stay dictionary-encoded, so one column that has to be decoded does
115+
/// not cost the others their encoding, and a batch that needs no decoding is returned
116+
/// unchanged.
116117
///
117-
/// The counterpart of the restriction above: an encoding that does not survive the export has
118-
/// to be decoded while the type is still known.
118+
/// The counterpart of the restriction above: an encoding the destination cannot take has to be
119+
/// decoded while the type is still known.
119120
///
120121
/// @param batch The batch to decode, matched to `logical_type` by field name; a column with no
121122
/// matching field is left alone.
122123
/// @param logical_type The struct type the decoded columns are cast to. `batch` is returned
123124
/// unchanged when it is not a struct.
124125
/// @param pool Allocates the decoded columns. Only used when a column is actually decoded.
126+
/// @param preserve_layout_recoverable_dictionaries Whether dictionaries recoverable through
127+
/// ResolveDictionaryStructTypeFromLayout() may remain encoded. Pass false
128+
/// to decode every dictionary not already declared by `logical_type`,
129+
/// whatever its shape.
125130
/// @return `batch` itself when nothing had to be decoded, otherwise a copy of it with the
126131
/// offset, length and validity of the original and the decoded columns swapped in.
127132
static Result<std::shared_ptr<arrow::StructArray>> FlattenUnresolvableDictionaries(
128133
const std::shared_ptr<arrow::StructArray>& batch,
129-
const std::shared_ptr<arrow::DataType>& logical_type, arrow::MemoryPool* pool);
134+
const std::shared_ptr<arrow::DataType>& logical_type, arrow::MemoryPool* pool,
135+
bool preserve_layout_recoverable_dictionaries);
130136

131137
private:
132138
static Status InnerCheckNullabilityMatch(const std::shared_ptr<arrow::Field>& field,

0 commit comments

Comments
 (0)