From e95f0483bd5ecf8b7576a5936ef5b5887c758eeb Mon Sep 17 00:00:00 2001 From: Yannick Utard Date: Wed, 5 Aug 2026 20:53:42 +0200 Subject: [PATCH 1/3] Backport selective Parquet VARIANT path decode for unshredded blobs. When extract projection hints a single key path, DecodePath walks the binary object encoding and materializes only the terminal subtree (wrapped sparsely so post-scan variant_extract still resolves), instead of decoding every sibling field. Co-authored-by: Cursor --- .../variant_unshredded_path_extract.benchmark | 35 +++++ .../reader/variant/variant_binary_decoder.hpp | 6 + .../variant/variant_shredded_conversion.hpp | 3 +- .../include/reader/variant_column_reader.hpp | 5 +- extension/parquet/parquet_reader.cpp | 24 ++- .../reader/variant/variant_binary_decoder.cpp | 112 ++++++++++++++ .../variant/variant_shredded_conversion.cpp | 26 +++- .../parquet/reader/variant_column_reader.cpp | 10 +- .../variant_extract_unshredded_direct.test | 145 ++++++++++++++++++ 9 files changed, 353 insertions(+), 13 deletions(-) create mode 100644 benchmark/micro/variant/variant_unshredded_path_extract.benchmark create mode 100644 test/parquet/variant/pushdown/variant_extract_unshredded_direct.test diff --git a/benchmark/micro/variant/variant_unshredded_path_extract.benchmark b/benchmark/micro/variant/variant_unshredded_path_extract.benchmark new file mode 100644 index 000000000000..a44c298069b6 --- /dev/null +++ b/benchmark/micro/variant/variant_unshredded_path_extract.benchmark @@ -0,0 +1,35 @@ +# name: benchmark/micro/variant/variant_unshredded_path_extract.benchmark +# description: Filter on a small top-level key whose values live in the unshredded Parquet VARIANT value blob (forced via mismatched shredding types so typed_value cannot hold them). Isolates CPU/allocation cost of pushed path extraction versus converting the full root blob; TOTAL_BYTES_READ is expected to stay similar because the root value page must still be read. +# group: [variant] + +name variant_unshredded_path_extract +group variant +subgroup parquet +storage persistent v2.0.0 + +cache variant_unshredded_path_extract.duckdb + +load +COPY ( + SELECT { + 'id': i::INTEGER, + 'flag': (i % 2 = 0), + 'payload_a': md5(i::VARCHAR) || md5((i + 1)::VARCHAR) || md5((i + 2)::VARCHAR) || md5((i + 3)::VARCHAR), + 'payload_b': md5((i + 10)::VARCHAR) || md5((i + 11)::VARCHAR) || md5((i + 12)::VARCHAR) || md5((i + 13)::VARCHAR), + 'payload_c': md5((i + 20)::VARCHAR) || md5((i + 21)::VARCHAR) || md5((i + 22)::VARCHAR) || md5((i + 23)::VARCHAR), + 'nested': { + 'x': (i % 100)::INTEGER, + 'y': md5((i + 30)::VARCHAR) || md5((i + 31)::VARCHAR) + } + }::VARIANT AS v + FROM range(200000) t(i) +) TO 'variant_unshredded_path_extract.parquet' ( + FORMAT PARQUET, + SHREDDING {'v': 'VARCHAR'} +); + +run +SELECT count(*) FROM 'variant_unshredded_path_extract.parquet' WHERE try(v.id::INTEGER) = 12345; + +result I +1 diff --git a/extension/parquet/include/reader/variant/variant_binary_decoder.hpp b/extension/parquet/include/reader/variant/variant_binary_decoder.hpp index d0d92c0fc8bf..bfb6d60d0f54 100644 --- a/extension/parquet/include/reader/variant/variant_binary_decoder.hpp +++ b/extension/parquet/include/reader/variant/variant_binary_decoder.hpp @@ -138,6 +138,12 @@ class VariantBinaryDecoder { public: static VariantValue Decode(const VariantMetadata &metadata, const_data_ptr_t data, idx_t data_offset, idx_t data_size); + //! Walk a constant key path without decoding sibling fields. Returns MISSING for absent keys / + //! non-object intermediates. Terminal objects/arrays still decode their complete requested subtree. + static VariantValue DecodePath(const VariantMetadata &metadata, const_data_ptr_t data, idx_t data_offset, + idx_t data_size, const vector &path); + //! Rebuild a sparse object tree from 'path' so post-scan variant_extract still resolves the same keys. + static VariantValue WrapSparsePath(VariantValue &&leaf, const vector &path); public: static VariantValue PrimitiveTypeDecode(const VariantValueMetadata &value_metadata, const_data_ptr_t data, diff --git a/extension/parquet/include/reader/variant/variant_shredded_conversion.hpp b/extension/parquet/include/reader/variant/variant_shredded_conversion.hpp index 5ef709ccc78f..2c42ea7dc642 100644 --- a/extension/parquet/include/reader/variant/variant_shredded_conversion.hpp +++ b/extension/parquet/include/reader/variant/variant_shredded_conversion.hpp @@ -11,7 +11,8 @@ class VariantShreddedConversion { VariantShreddedConversion() = delete; public: - static vector Convert(Vector &metadata, Vector &group, idx_t offset, idx_t length, idx_t total_size); + static vector Convert(Vector &metadata, Vector &group, idx_t offset, idx_t length, idx_t total_size, + const vector *extract_path = nullptr); static void ConvertBinaryToVariant(Vector &metadata_and_value, idx_t offset, idx_t length, idx_t total_size, Vector &result); static vector ConvertShreddedLeaf(Vector &metadata, Vector &value, Vector &typed_value, idx_t offset, diff --git a/extension/parquet/include/reader/variant_column_reader.hpp b/extension/parquet/include/reader/variant_column_reader.hpp index fe2fb4f2f88d..5ef3ffd616a2 100644 --- a/extension/parquet/include/reader/variant_column_reader.hpp +++ b/extension/parquet/include/reader/variant_column_reader.hpp @@ -19,10 +19,13 @@ class VariantColumnReader : public ColumnReader { public: VariantColumnReader(ClientContext &context, const ParquetReader &reader, const ParquetColumnSchema &schema, - vector> child_readers_p, bool projected_mode_p = false); + vector> child_readers_p, bool projected_mode_p = false, + vector extract_path_p = {}); ClientContext &context; vector> child_readers; + //! Optional constant key path from extract projection hints (unshredded binary fast path) + vector extract_path; public: ColumnReader &GetChildReader(idx_t child_idx); diff --git a/extension/parquet/parquet_reader.cpp b/extension/parquet/parquet_reader.cpp index 083c3e220db8..d9ee6bd45c75 100644 --- a/extension/parquet/parquet_reader.cpp +++ b/extension/parquet/parquet_reader.cpp @@ -592,10 +592,32 @@ unique_ptr ParquetReader::CreateReaderRecursive(ClientContext &con return make_uniq(context, *this, schema, std::move(children), true); } + //! Unshredded / non-projectable path: retain a single constant key path for selective binary decode + vector extract_path; + if (indexes.size() == 1 && !indexes[0].HasPrimaryIndex()) { + reference current(indexes[0]); + while (true) { + if (current.get().HasPrimaryIndex()) { + extract_path.clear(); + break; + } + extract_path.push_back(current.get().GetFieldName()); + if (!current.get().HasChildren()) { + break; + } + if (current.get().ChildIndexCount() != 1) { + extract_path.clear(); + break; + } + current = current.get().GetChildIndex(0); + } + } + for (idx_t child_index = 0; child_index < schema.children.size(); child_index++) { children[child_index] = CreateReaderRecursive(context, vector(), schema.children[child_index]); } - return make_uniq(context, *this, schema, std::move(children), false); + return make_uniq(context, *this, schema, std::move(children), false, + std::move(extract_path)); } default: throw InternalException("Unsupported ParquetColumnSchemaType"); diff --git a/extension/parquet/reader/variant/variant_binary_decoder.cpp b/extension/parquet/reader/variant/variant_binary_decoder.cpp index b9bd88a66de2..a882da1e46b7 100644 --- a/extension/parquet/reader/variant/variant_binary_decoder.cpp +++ b/extension/parquet/reader/variant/variant_binary_decoder.cpp @@ -1,5 +1,7 @@ #include "reader/variant/variant_binary_decoder.hpp" #include "duckdb/common/printer.hpp" +#include "duckdb/common/optional_idx.hpp" +#include "duckdb/common/numeric_utils.hpp" #include "utf8proc_wrapper.hpp" #include "reader/uuid_column_reader.hpp" @@ -10,6 +12,8 @@ #include "duckdb/common/types/time.hpp" #include "duckdb/common/types/date.hpp" +#include + static constexpr uint8_t VERSION_MASK = 0xF; static constexpr uint8_t SORTED_STRINGS_MASK = 0x1; static constexpr uint8_t SORTED_STRINGS_SHIFT = 4; @@ -462,4 +466,112 @@ VariantValue VariantBinaryDecoder::Decode(const VariantMetadata &variant_metadat } } +namespace { + +optional_idx FindMetadataStringId(const VariantMetadata &metadata, const string &key) { + auto &strings = metadata.strings; + if (metadata.header.sorted_strings) { + auto it = std::lower_bound(strings.begin(), strings.end(), key); + if (it != strings.end() && *it == key) { + return optional_idx(NumericCast(it - strings.begin())); + } + return optional_idx(); + } + for (idx_t i = 0; i < strings.size(); i++) { + if (strings[i] == key) { + return optional_idx(i); + } + } + return optional_idx(); +} + +//! Locate 'key' in a binary OBJECT and return the byte offset of the matching child value (header byte), +//! or invalid when the key is absent. +optional_idx FindObjectChildOffset(const VariantMetadata &metadata, const VariantValueMetadata &value_metadata, + const_data_ptr_t data, idx_t data_offset, idx_t data_size, const string &key) { + auto field_id = FindMetadataStringId(metadata, key); + if (!field_id.IsValid()) { + return optional_idx(); + } + + auto field_offset_size = value_metadata.field_offset_size; + auto field_id_size = value_metadata.field_id_size; + auto is_large = value_metadata.is_large; + + idx_t num_elements; + idx_t cursor = data_offset; + if (is_large) { + if (cursor + sizeof(uint32_t) > data_size) { + throw IOException("Corrupted VARIANT 'value' buffer"); + } + num_elements = Load(data + cursor); + cursor += sizeof(uint32_t); + } else { + if (cursor + sizeof(uint8_t) > data_size) { + throw IOException("Corrupted VARIANT 'value' buffer"); + } + num_elements = Load(data + cursor); + cursor += sizeof(uint8_t); + } + + auto field_ids_offset = cursor; + auto field_offsets_offset = cursor + (num_elements * field_id_size); + auto values_offset = field_offsets_offset + ((num_elements + 1) * field_offset_size); + + idx_t ids_cursor = field_ids_offset; + idx_t offsets_cursor = field_offsets_offset; + idx_t last_offset = ReadVariableLengthLittleEndian(field_offset_size, data, offsets_cursor, data_size); + for (idx_t i = 0; i < num_elements; i++) { + auto id = ReadVariableLengthLittleEndian(field_id_size, data, ids_cursor, data_size); + auto next_offset = ReadVariableLengthLittleEndian(field_offset_size, data, offsets_cursor, data_size); + if (id == field_id.GetIndex()) { + auto child_offset = values_offset + last_offset; + if (child_offset >= data_size) { + throw IOException("Corrupted VARIANT 'value' buffer"); + } + return optional_idx(child_offset); + } + last_offset = next_offset; + } + return optional_idx(); +} + +} // namespace + +VariantValue VariantBinaryDecoder::DecodePath(const VariantMetadata &metadata, const_data_ptr_t data, + idx_t data_offset, idx_t data_size, const vector &path) { + D_ASSERT(!path.empty()); + idx_t offset = data_offset; + for (idx_t path_idx = 0; path_idx < path.size(); path_idx++) { + if (offset + 1 > data_size) { + throw IOException("Corrupted VARIANT 'value' buffer"); + } + auto value_metadata = VariantValueMetadata::FromHeaderByte(data[offset]); + if (value_metadata.basic_type != VariantBasicType::OBJECT) { + return VariantValue(); + } + auto child_offset = + FindObjectChildOffset(metadata, value_metadata, data, offset + 1, data_size, path[path_idx]); + if (!child_offset.IsValid()) { + return VariantValue(); + } + offset = child_offset.GetIndex(); + } + //! Decode only the terminal subtree + return Decode(metadata, data, offset, data_size); +} + +VariantValue VariantBinaryDecoder::WrapSparsePath(VariantValue &&leaf, const vector &path) { + if (leaf.IsMissing() || path.empty()) { + return std::move(leaf); + } + VariantValue current = std::move(leaf); + for (idx_t i = path.size(); i > 0; i--) { + VariantValue parent(VariantValueType::OBJECT); + parent.AddChild(path[i - 1], std::move(current)); + current = std::move(parent); + } + return current; +} + } // namespace duckdb diff --git a/extension/parquet/reader/variant/variant_shredded_conversion.cpp b/extension/parquet/reader/variant/variant_shredded_conversion.cpp index 193df51b2bb7..e13ac6a1384f 100644 --- a/extension/parquet/reader/variant/variant_shredded_conversion.cpp +++ b/extension/parquet/reader/variant/variant_shredded_conversion.cpp @@ -324,7 +324,8 @@ struct ShreddedVariantField { } // namespace static vector ConvertBinaryEncoding(Vector &metadata, Vector &value, idx_t offset, idx_t length, - idx_t total_size, bool add_metadata_offset = false) { + idx_t total_size, bool add_metadata_offset = false, + const vector *extract_path = nullptr) { UnifiedVectorFormat value_format; value.ToUnifiedFormat(total_size, value_format); auto value_data = value_format.GetData(value_format); @@ -334,6 +335,7 @@ static vector ConvertBinaryEncoding(Vector &metadata, Vector &valu metadata.ToUnifiedFormat(length, metadata_format); auto metadata_data = metadata_format.GetData(metadata_format); auto metadata_validity = metadata_format.validity; + (void)metadata_validity; //! Fills every row with MISSING, turned into NULL later if this is not in an OBJECT field vector ret(length); @@ -356,8 +358,15 @@ static vector ConvertBinaryEncoding(Vector &metadata, Vector &valu //! The value bytes start directly after the metadata bytes value_offset += variant_metadata.total_size; } - ret[i] = VariantBinaryDecoder::Decode(variant_metadata, const_data_ptr_cast(binary_value), value_offset, - value_buffer.GetSize()); + if (extract_path && !extract_path->empty()) { + //! Selective path decode: skip sibling fields, then wrap so post-scan extract still works + auto leaf = VariantBinaryDecoder::DecodePath(variant_metadata, const_data_ptr_cast(binary_value), + value_offset, value_buffer.GetSize(), *extract_path); + ret[i] = VariantBinaryDecoder::WrapSparsePath(std::move(leaf), *extract_path); + } else { + ret[i] = VariantBinaryDecoder::Decode(variant_metadata, const_data_ptr_cast(binary_value), value_offset, + value_buffer.GetSize()); + } } return ret; } @@ -532,7 +541,7 @@ vector VariantShreddedConversion::ConvertShreddedArray(Vector &met } vector VariantShreddedConversion::Convert(Vector &metadata, Vector &group, idx_t offset, idx_t length, - idx_t total_size) { + idx_t total_size, const vector *extract_path) { D_ASSERT(group.GetType().id() == LogicalTypeId::STRUCT); auto &group_entries = StructVector::GetEntries(group); @@ -560,7 +569,12 @@ vector VariantShreddedConversion::Convert(Vector &metadata, Vector if (typed_value) { auto &type = typed_value->GetType(); - vector ret; + //! When the root is shredded as a leaf (e.g. SHREDDING VARCHAR) the object payload lives in + //! binary 'value'. Prefer the selective path decode over shredding the unusable typed leaf. + if (extract_path && !extract_path->empty() && type.id() != LogicalTypeId::STRUCT && + type.id() != LogicalTypeId::LIST) { + return ConvertBinaryEncoding(metadata, *value, offset, length, total_size, false, extract_path); + } if (type.id() == LogicalTypeId::STRUCT) { return ConvertShreddedObject(metadata, *value, *typed_value, offset, length, total_size); } else if (type.id() == LogicalTypeId::LIST) { @@ -569,7 +583,7 @@ vector VariantShreddedConversion::Convert(Vector &metadata, Vector return ConvertShreddedLeaf(metadata, *value, *typed_value, offset, length, total_size); } } else { - return ConvertBinaryEncoding(metadata, *value, offset, length, total_size); + return ConvertBinaryEncoding(metadata, *value, offset, length, total_size, false, extract_path); } } diff --git a/extension/parquet/reader/variant_column_reader.cpp b/extension/parquet/reader/variant_column_reader.cpp index bed955e19c17..93381c8e6185 100644 --- a/extension/parquet/reader/variant_column_reader.cpp +++ b/extension/parquet/reader/variant_column_reader.cpp @@ -9,9 +9,10 @@ namespace duckdb { //===--------------------------------------------------------------------===// VariantColumnReader::VariantColumnReader(ClientContext &context, const ParquetReader &reader, const ParquetColumnSchema &schema, - vector> child_readers_p, bool projected_mode_p) + vector> child_readers_p, bool projected_mode_p, + vector extract_path_p) : ColumnReader(reader, schema), context(context), child_readers(std::move(child_readers_p)), - projected_mode(projected_mode_p) { + extract_path(std::move(extract_path_p)), projected_mode(projected_mode_p) { D_ASSERT(Type().InternalType() == PhysicalType::STRUCT); optional_idx metadata_idx; @@ -111,8 +112,9 @@ idx_t VariantColumnReader::Read(uint64_t num_values, data_ptr_t define_out, data "The shredded Variant column did not contain the same amount of values for 'typed_value' and 'value'"); } } - auto intermediate = - VariantShreddedConversion::Convert(metadata_intermediate, intermediate_group, 0, num_values, num_values); + const vector *path_ptr = extract_path.empty() ? nullptr : &extract_path; + auto intermediate = VariantShreddedConversion::Convert(metadata_intermediate, intermediate_group, 0, num_values, + num_values, path_ptr); VariantValue::ToVARIANT(intermediate, result); return value_values; diff --git a/test/parquet/variant/pushdown/variant_extract_unshredded_direct.test b/test/parquet/variant/pushdown/variant_extract_unshredded_direct.test new file mode 100644 index 000000000000..fd3d92c793f7 --- /dev/null +++ b/test/parquet/variant/pushdown/variant_extract_unshredded_direct.test @@ -0,0 +1,145 @@ +# name: test/parquet/variant/pushdown/variant_extract_unshredded_direct.test +# description: Direct path extraction from unshredded / partially shredded Parquet VARIANT blobs +# group: [pushdown] + +require parquet + +# Force values into the binary 'value' blob: shred the root as VARCHAR so object fields cannot +# use the fully-shredded typed_value shortcut. Avoid length-64 strings (writer edge case). +statement ok +COPY ( + FROM (VALUES + ({'id': 1::INTEGER, 'payload': repeat('a', 80), 'nested': {'x': 10::INTEGER, 'y': 'ten'}, 'arr': [1::INTEGER, 2]}::VARIANT), + ({'id': 2::INTEGER, 'payload': repeat('b', 80), 'nested': {'x': 20::INTEGER}, 'arr': [3::INTEGER]}::VARIANT), + ({'id': 3::INTEGER, 'payload': repeat('c', 80), 'nested': NULL, 'arr': []}::VARIANT), + ({'payload': repeat('d', 80), 'nested': {'x': 40::INTEGER}}::VARIANT), + (NULL::VARIANT), + ({'id': NULL, 'payload': 'e', 'nested': {'x': NULL}}::VARIANT) + ) t(v) +) TO '{TEST_DIR}/variant_unshredded_direct.parquet' (SHREDDING {'v': 'VARCHAR'}); + +# Top-level key extract + filter +query II nosort +SELECT v.id::INTEGER, v.payload::VARCHAR +FROM '{TEST_DIR}/variant_unshredded_direct.parquet' +WHERE v.id = 2 +---- +2 bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + +# Nested unshredded key +query II +SELECT v.id::INTEGER, v.nested.x::INTEGER +FROM '{TEST_DIR}/variant_unshredded_direct.parquet' +WHERE v.nested.x = 20 +---- +2 20 + +# Absent / case-distinct keys (key lookup is case-sensitive) +query II +SELECT + count(*) FILTER (WHERE v.missing_key IS NULL), + count(*) FILTER (WHERE v.Id IS NULL) +FROM '{TEST_DIR}/variant_unshredded_direct.parquet' +---- +6 6 + +# SQL NULL root, VARIANT_NULL values, missing nested intermediates +query IIII +SELECT + count(*) FILTER (WHERE v.id IS NULL), + count(*) FILTER (WHERE v.nested IS NULL), + count(*) FILTER (WHERE v.nested.x IS NULL), + count(*) FILTER (WHERE v.id = 1) +FROM '{TEST_DIR}/variant_unshredded_direct.parquet' +---- +3 2 3 1 + +# Non-object intermediate (scalar id) yields NULL for deeper path +query I +SELECT count(*) FILTER (WHERE v.id.x IS NULL) +FROM '{TEST_DIR}/variant_unshredded_direct.parquet' +---- +6 + +# Terminal object / array still emit their complete requested subtree +query TT nosort +SELECT variant_typeof(v.nested), v.nested::VARCHAR +FROM '{TEST_DIR}/variant_unshredded_direct.parquet' +WHERE v.id = 1 +---- +OBJECT(x, y) {'x': 10, 'y': ten} + +query TT +SELECT variant_typeof(v.arr), v.arr::VARCHAR +FROM '{TEST_DIR}/variant_unshredded_direct.parquet' +WHERE v.id = 1 +---- +ARRAY(2) [1, 2] + +# Casts / TRY on the unshredded extract path +query III +SELECT + count(*) FILTER (WHERE v.id::INTEGER = 2), + count(*) FILTER (WHERE TRY(v.id::INTEGER) = 2), + count(*) FILTER (WHERE v.nested.x::BIGINT = 20) +FROM '{TEST_DIR}/variant_unshredded_direct.parquet' +---- +1 1 1 + +# Path extract remains in the plan on this DuckDB pin (parquet has no pushdown-extract); +# selective DecodePath still materializes only the requested key into a sparse object. +query I +SELECT v.id::INTEGER +FROM '{TEST_DIR}/variant_unshredded_direct.parquet' +WHERE v.id = 2 +---- +2 + +# Partially shredded: typed field + binary overlay fallback +statement ok +COPY ( + FROM (VALUES + ({'typed': 1::INTEGER, 'overlay': 'one'}::VARIANT), + ({'typed': 2::INTEGER}::VARIANT), + ({'overlay': 'only_overlay'}::VARIANT), + ({'typed': NULL, 'overlay': 'null_typed'}::VARIANT) + ) t(v) +) TO '{TEST_DIR}/variant_partial_direct.parquet' ( + SHREDDING {'v': 'STRUCT(typed INTEGER)'} +); + +query ITT nosort +SELECT v.typed::INTEGER, v.overlay::VARCHAR, variant_typeof(v) +FROM '{TEST_DIR}/variant_partial_direct.parquet' +WHERE v.overlay = 'one' OR v.typed = 2 OR v.overlay = 'only_overlay' +ORDER BY coalesce(v.typed::INTEGER, -1), coalesce(v.overlay::VARCHAR, '') +---- +NULL only_overlay OBJECT(overlay) +1 one OBJECT(overlay, typed) +2 NULL OBJECT(typed) + +# Mixed files: unshredded + shredded layouts in one scan +statement ok +COPY ( + FROM (VALUES + ({'id': 100::INTEGER, 'tag': 'shredded'}::VARIANT) + ) t(v) +) TO '{TEST_DIR}/variant_mixed_direct_a.parquet' ( + SHREDDING {'v': 'STRUCT(id INTEGER, tag VARCHAR)'} +); + +statement ok +COPY ( + FROM (VALUES + ({'id': 200::INTEGER, 'tag': 'unshredded'}::VARIANT) + ) t(v) +) TO '{TEST_DIR}/variant_mixed_direct_b.parquet' (SHREDDING {'v': 'VARCHAR'}); + +query II nosort +SELECT v.id::INTEGER, v.tag::VARCHAR +FROM '{TEST_DIR}/variant_mixed_direct_*.parquet' +WHERE v.id IN (100, 200) +ORDER BY v.id::INTEGER +---- +100 shredded +200 unshredded From 46a3f526611b82bb2f3594e719b0abde5adfe9b4 Mon Sep 17 00:00:00 2001 From: Yannick Utard Date: Wed, 5 Aug 2026 20:58:12 +0200 Subject: [PATCH 2/3] Fix VARIANT path lookup when metadata sorted flag is wrong. Some writers set sorted_strings while keeping keys in insertion order; fall back to linear dictionary scan so DecodePath does not miss keys. Co-authored-by: Cursor --- .../reader/variant/variant_binary_decoder.cpp | 3 ++- .../variant_extract_unshredded_direct.test | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/extension/parquet/reader/variant/variant_binary_decoder.cpp b/extension/parquet/reader/variant/variant_binary_decoder.cpp index a882da1e46b7..466182fb098f 100644 --- a/extension/parquet/reader/variant/variant_binary_decoder.cpp +++ b/extension/parquet/reader/variant/variant_binary_decoder.cpp @@ -470,12 +470,13 @@ namespace { optional_idx FindMetadataStringId(const VariantMetadata &metadata, const string &key) { auto &strings = metadata.strings; + //! Prefer binary search when the dictionary claims to be sorted. Some writers set the sorted flag while + //! leaving keys in insertion order (e.g. ["id","b"]), so fall back to a linear scan on miss. if (metadata.header.sorted_strings) { auto it = std::lower_bound(strings.begin(), strings.end(), key); if (it != strings.end() && *it == key) { return optional_idx(NumericCast(it - strings.begin())); } - return optional_idx(); } for (idx_t i = 0; i < strings.size(); i++) { if (strings[i] == key) { diff --git a/test/parquet/variant/pushdown/variant_extract_unshredded_direct.test b/test/parquet/variant/pushdown/variant_extract_unshredded_direct.test index fd3d92c793f7..c9a92fedb97a 100644 --- a/test/parquet/variant/pushdown/variant_extract_unshredded_direct.test +++ b/test/parquet/variant/pushdown/variant_extract_unshredded_direct.test @@ -95,6 +95,23 @@ WHERE v.id = 2 ---- 2 +# Unsorted metadata dictionary: writer may set sorted_strings while keys stay in insertion +# order (here "id" before "flag"). Binary search alone would miss; linear fallback must work. +statement ok +COPY ( + SELECT {'id': i::INTEGER, 'flag': (i % 2 = 0)}::VARIANT AS v + FROM range(3) t(i) +) TO '{TEST_DIR}/variant_unsorted_dict_direct.parquet' (SHREDDING {'v': 'VARCHAR'}); + +query II +SELECT v.id::INTEGER, v.flag::BOOLEAN +FROM '{TEST_DIR}/variant_unsorted_dict_direct.parquet' +ORDER BY v.id::INTEGER +---- +0 true +1 false +2 true + # Partially shredded: typed field + binary overlay fallback statement ok COPY ( From 1ab3491b63113b46fb5f9e69b6dbdfd5aae0d261 Mon Sep 17 00:00:00 2001 From: Yannick Utard Date: Wed, 5 Aug 2026 21:23:01 +0200 Subject: [PATCH 3/3] Apply clang-format fixes for CodeQuality CI. Co-authored-by: Cursor --- .../parquet/reader/variant/variant_binary_decoder.cpp | 4 ++-- extension/parquet/variant_arrow_capi.cpp | 11 +++++------ src/common/arrow/arrow_appender.cpp | 3 ++- .../duckdb/common/multi_file/multi_file_function.hpp | 4 ++-- src/optimizer/sampling_pushdown.cpp | 5 ++--- 5 files changed, 13 insertions(+), 14 deletions(-) diff --git a/extension/parquet/reader/variant/variant_binary_decoder.cpp b/extension/parquet/reader/variant/variant_binary_decoder.cpp index 466182fb098f..6c152282191d 100644 --- a/extension/parquet/reader/variant/variant_binary_decoder.cpp +++ b/extension/parquet/reader/variant/variant_binary_decoder.cpp @@ -539,8 +539,8 @@ optional_idx FindObjectChildOffset(const VariantMetadata &metadata, const Varian } // namespace -VariantValue VariantBinaryDecoder::DecodePath(const VariantMetadata &metadata, const_data_ptr_t data, - idx_t data_offset, idx_t data_size, const vector &path) { +VariantValue VariantBinaryDecoder::DecodePath(const VariantMetadata &metadata, const_data_ptr_t data, idx_t data_offset, + idx_t data_size, const vector &path) { D_ASSERT(!path.empty()); idx_t offset = data_offset; for (idx_t path_idx = 0; path_idx < path.size(); path_idx++) { diff --git a/extension/parquet/variant_arrow_capi.cpp b/extension/parquet/variant_arrow_capi.cpp index 38ae3d0ee67f..d316a21fabbf 100644 --- a/extension/parquet/variant_arrow_capi.cpp +++ b/extension/parquet/variant_arrow_capi.cpp @@ -257,10 +257,10 @@ void RegisterParquetVariantArrowExtension(DatabaseInstance &db_instance) { if (config.HasArrowExtension(LogicalType::VARIANT())) { return; } - config.RegisterArrowExtension({ARROW_PARQUET_VARIANT, PopulateVariantArrowSchema, GetVariantArrowType, - make_shared_ptr(LogicalType::VARIANT(), - GetParquetVariantStructType(), - ArrowToVariant, VariantToArrow)}); + config.RegisterArrowExtension( + {ARROW_PARQUET_VARIANT, PopulateVariantArrowSchema, GetVariantArrowType, + make_shared_ptr(LogicalType::VARIANT(), GetParquetVariantStructType(), ArrowToVariant, + VariantToArrow)}); } duckdb_state RegisterParquetVariantArrow(duckdb_database database) { @@ -277,8 +277,7 @@ duckdb_state RegisterParquetVariantArrow(duckdb_database database) { extern "C" DUCKDB_EXTENSION_API duckdb_state duckdb_parquet_variant_bytes_to_json(duckdb_database database, const uint8_t *metadata, idx_t metadata_len, - const uint8_t *value, - idx_t value_len, + const uint8_t *value, idx_t value_len, char **out_json) { if (!database || !out_json) { return DuckDBError; diff --git a/src/common/arrow/arrow_appender.cpp b/src/common/arrow/arrow_appender.cpp index a7bc31905c3b..f556c50cb37a 100644 --- a/src/common/arrow/arrow_appender.cpp +++ b/src/common/arrow/arrow_appender.cpp @@ -115,7 +115,8 @@ ArrowArray ArrowAppender::Finalize() { for (idx_t i = 0; i < root_holder->child_data.size(); i++) { auto &child_data = root_holder->child_data[i]; - const LogicalType child_type = child_data->extension_data ? child_data->extension_data->GetInternalType() : types[i]; + const LogicalType child_type = + child_data->extension_data ? child_data->extension_data->GetInternalType() : types[i]; root_holder->child_arrays[i] = *ArrowAppender::FinalizeChild(child_type, std::move(root_holder->child_data[i])); } diff --git a/src/include/duckdb/common/multi_file/multi_file_function.hpp b/src/include/duckdb/common/multi_file/multi_file_function.hpp index fc2a6f6f7992..cd7208844d2a 100644 --- a/src/include/duckdb/common/multi_file/multi_file_function.hpp +++ b/src/include/duckdb/common/multi_file/multi_file_function.hpp @@ -492,8 +492,8 @@ class MultiFileFunction : public TableFunction { // optionally apply sampling pushdown after filter pruning, before any readers open if (input.sample_options) { auto &candidate_list = new_list ? *new_list : *bind_data.file_list; - auto sampled_list = bind_data.multi_file_reader->SamplePushdown(context, candidate_list, - bind_data.file_options, *input.sample_options); + auto sampled_list = bind_data.multi_file_reader->SamplePushdown( + context, candidate_list, bind_data.file_options, *input.sample_options); if (sampled_list) { new_list = std::move(sampled_list); } diff --git a/src/optimizer/sampling_pushdown.cpp b/src/optimizer/sampling_pushdown.cpp index 4d9ada4e30e3..f2018ff0bf02 100644 --- a/src/optimizer/sampling_pushdown.cpp +++ b/src/optimizer/sampling_pushdown.cpp @@ -24,9 +24,8 @@ unique_ptr SamplingPushdown::Optimize(unique_ptrchildren[0]->Cast(); const auto &sample_options = *sample_op.sample_options; const bool has_filters = !get.table_filters.filters.empty() || get.dynamic_filters; - const bool can_push_system_sample = - sample_options.method == SampleMethod::SYSTEM_SAMPLE && !has_filters && - CanPushSampling(context, get, sample_options); + const bool can_push_system_sample = sample_options.method == SampleMethod::SYSTEM_SAMPLE && !has_filters && + CanPushSampling(context, get, sample_options); if (can_push_system_sample) { const bool is_row_count_sampling = !sample_options.is_percentage;