Skip to content

Backport VARIANT extract pushdown and optimize unshredded paths - #2

Merged
utay merged 3 commits into
altertable-1.5.5from
altertable-1.5.5-variant-path-extract
Aug 6, 2026
Merged

Backport VARIANT extract pushdown and optimize unshredded paths#2
utay merged 3 commits into
altertable-1.5.5from
altertable-1.5.5-variant-path-extract

Conversation

@utay

@utay utay commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

https://altertable.slack.com/archives/C08L2BJR5UY/p1785938202234059

Backported from DuckDB main

Parquet variant_extract pushdown from duckdb/duckdb#22478, adapted to the 1.5.5 VARIANT decoder.

New

Decode only the requested path from unshredded binary VARIANT values, avoiding full sibling materialization.
Add a reader-side fallback for incorrectly flagged unsorted metadata dictionaries. This complements the writer fix in duckdb/duckdb#23591.

Add correctness tests and a microbenchmark.

Benchmark

Workload Before (full decode) After (DecodePath) Speedup JSON
Filter id = 12345 ~276 ms ~69 ms ~4.0× ~80 ms
Filter nested.x = 42 ~276 ms ~99 ms ~2.8× ~84 ms
Full scan sum(id) ~313 ms ~106 ms ~3.0× ~69 ms

utay and others added 3 commits August 5, 2026 21:02
@utay @cursoragent
Backport selective Parquet VARIANT path decode for unshredded blobs.
e95f048 
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 <cursoragent@cursor.com>
@utay @cursoragent
Fix VARIANT path lookup when metadata sorted flag is wrong.
46a3f52 
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 <cursoragent@cursor.com>
@utay @cursoragent
Apply clang-format fixes for CodeQuality CI.
1ab3491 
Co-authored-by: Cursor <cursoragent@cursor.com>
@utay utay self-assigned this Aug 5, 2026
@utay
utay merged commit 9f815a3 into altertable-1.5.5 Aug 6, 2026
119 of 132 checks passed
redox pushed a commit that referenced this pull request Sep 29, 2026
@Mytherin
Fix tdigest data race (duckdb#25460)
6d04e4c 
<details>
<summary>WARNING: ThreadSanitizer: data race; Read of size 8</summary>

```c++
  Filters: test/sql/show_select/summarize_subquery.test

  [0/1] (0%): test/sql/show_select/summarize_subquery.test==================
  WARNING: ThreadSanitizer: data race (pid=80064)
    Read of size 8 at 0x00010708e758 by thread T10:
      #0 duckdb::ArenaAllocator::AlignNext() <null> (libduckdb.dylib:arm64+0x2a37af0)
      #1 std::__1::vector<double, duckdb::arena_stl_allocator<double>>::reserve(unsigned long) vector.h:1100 (libduckdb.dylib:arm64+0x3184418)
      #2 duckdb_tdigest::TDigest::updateCumulative() t_digest.hpp:538 (libduckdb.dylib:arm64+0x31819cc)
      #3 duckdb_tdigest::TDigest::process() t_digest.hpp:583 (libduckdb.dylib:arm64+0x31816d8)
      #4 void duckdb::(anonymous namespace)::ApproxQuantileScalarOperation::Finalize<long long, duckdb::(anonymous namespace)::ApproxQuantileState>(duckdb::(ano  nymous namespace)::ApproxQuantileState&, long long&, duckdb::AggregateFinalizeData&) approximate_quantile.cpp:162 (libduckdb.dylib:arm64+0x318d540)
      #5 void duckdb::AggregateFunction::StateFinalize<duckdb::(anonymous namespace)::ApproxQuantileState, long long, duckdb::(anonymous namespace)::ApproxQuant  ileScalarOperation>(duckdb::Vector&, duckdb::AggregateFinalizeInputData&, duckdb::Vector&, unsigned long long, unsigned long long) aggregate_function.hpp:822   (libduckdb.dylib:arm64+0x318ca64)
      #6 duckdb::RowOperations::FinalizeStates(duckdb::RowOperationsState&, duckdb::TupleDataLayout&, duckdb::Vector&, duckdb::DataChunk&, unsigned long long) r  ow_aggregate.cpp:182 (libduckdb.dylib:arm64+0x166f848)
      #7 duckdb::RadixHTLocalSourceState::Scan(duckdb::RadixHTGlobalSinkState&, duckdb::RadixHTGlobalSourceState&, duckdb::DataChunk&) <null> (libduckdb.dylib:a  rm64+0x2458dac)
      #8 duckdb::RadixHTLocalSourceState::ExecuteTask(duckdb::RadixHTGlobalSinkState&, duckdb::RadixHTGlobalSourceState&, duckdb::DataChunk&) <null> (libduckdb.  dylib:arm64+0x2457ea0)
      duckdb#9 duckdb::RadixPartitionedHashTable::GetData(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::GlobalSinkState&, duckdb::OperatorSourceInput&) const   <null> (libduckdb.dylib:arm64+0x2459624)
      duckdb#10 duckdb::PhysicalHashAggregate::GetDataInternal(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSourceInput&) const physical_hash_aggreg  ate.cpp:978 (libduckdb.dylib:arm64+0x21305cc)
      duckdb#11 duckdb::PhysicalOperator::GetData(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSourceInput&) const <null> (libduckdb.dylib:arm64+0x2  448dac)
...

    Previous write of size 8 at 0x00010708e758 by thread T7:
      #0 std::__1::vector<double, duckdb::arena_stl_allocator<double>>::reserve(unsigned long) vector.h:1100 (libduckdb.dylib:arm64+0x31844a0)
      #1 duckdb_tdigest::TDigest::updateCumulative() t_digest.hpp:538 (libduckdb.dylib:arm64+0x31819cc)
      #2 duckdb_tdigest::TDigest::process() t_digest.hpp:583 (libduckdb.dylib:arm64+0x31816d8)
      #3 void duckdb::(anonymous namespace)::ApproxQuantileScalarOperation::Finalize<long long, duckdb::(anonymous namespace)::ApproxQuantileState>(duckdb::(ano  nymous namespace)::ApproxQuantileState&, long long&, duckdb::AggregateFinalizeData&) approximate_quantile.cpp:162 (libduckdb.dylib:arm64+0x318d540)
      #4 void duckdb::AggregateFunction::StateFinalize<duckdb::(anonymous namespace)::ApproxQuantileState, long long, duckdb::(anonymous namespace)::ApproxQuant  ileScalarOperation>(duckdb::Vector&, duckdb::AggregateFinalizeInputData&, duckdb::Vector&, unsigned long long, unsigned long long) aggregate_function.hpp:822   (libduckdb.dylib:arm64+0x318ca64)
      #5 duckdb::RowOperations::FinalizeStates(duckdb::RowOperationsState&, duckdb::TupleDataLayout&, duckdb::Vector&, duckdb::DataChunk&, unsigned long long) r  ow_aggregate.cpp:182 (libduckdb.dylib:arm64+0x166f848)
      #6 duckdb::RadixHTLocalSourceState::Scan(duckdb::RadixHTGlobalSinkState&, duckdb::RadixHTGlobalSourceState&, duckdb::DataChunk&) <null> (libduckdb.dylib:a  rm64+0x2458dac)
      #7 duckdb::RadixHTLocalSourceState::ExecuteTask(duckdb::RadixHTGlobalSinkState&, duckdb::RadixHTGlobalSourceState&, duckdb::DataChunk&) <null> (libduckdb.  dylib:arm64+0x2457ea0)
      #8 duckdb::RadixPartitionedHashTable::GetData(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::GlobalSinkState&, duckdb::OperatorSourceInput&) const   <null> (libduckdb.dylib:arm64+0x2459624)
      duckdb#9 duckdb::PhysicalHashAggregate::GetDataInternal(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSourceInput&) const physical_hash_aggrega  te.cpp:978 (libduckdb.dylib:arm64+0x21305cc)
      duckdb#10 duckdb::PhysicalOperator::GetData(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSourceInput&) const <null> (libduckdb.dylib:arm64+0x2  448dac)
...

    Location is heap block of size 56 at 0x00010708e740 allocated by main thread:
      #0 operator new(unsigned long) <null> (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x91650)
      #1 duckdb::ArenaAllocator::AllocateNewBlock(unsigned long long) <null> (libduckdb.dylib:arm64+0x2a37830)
      #2 std::__1::vector<duckdb_tdigest::Centroid, duckdb::arena_stl_allocator<duckdb_tdigest::Centroid>>::reserve(unsigned long) vector.h:1100 (libduckdb.dyli  b:arm64+0x3180da0)
      #3 void duckdb::(anonymous namespace)::ApproxQuantileOperation::Operation<long long, duckdb::(anonymous namespace)::ApproxQuantileState, duckdb::(anonymou  s namespace)::ApproxQuantileScalarOperation>(duckdb::(anonymous namespace)::ApproxQuantileState&, long long const&, duckdb::AggregateUnaryInput&) approximate_  quantile.cpp:122 (libduckdb.dylib:arm64+0x318ccd4)
      #4 void duckdb::AggregateFunction::UnaryScatterUpdate<duckdb::(anonymous namespace)::ApproxQuantileState, long long, duckdb::(anonymous namespace)::Approx  QuantileScalarOperation>(duckdb::Vector*, duckdb::AggregateInputData&, unsigned long long, duckdb::Vector&, unsigned long long) aggregate_function.hpp:782 (li  bduckdb.dylib:arm64+0x318c454)
      #5 duckdb::RowOperations::UpdateStates(duckdb::RowOperationsState&, duckdb::AggregateObject&, duckdb::Vector&, duckdb::DataChunk&, unsigned long long, duc  kdb::optional_ptr<duckdb::ClusteredAggr const, true>) aggregate_function.hpp (libduckdb.dylib:arm64+0x166ed24)
      #6 duckdb::GroupedAggregateHashTable::UpdateAggregates(duckdb::DataChunk&, duckdb::vector<unsigned long long, false, std::__1::allocator<unsigned long lon  g>> const&, unsigned long long, bool) <null> (libduckdb.dylib:arm64+0x240d234)
      #7 duckdb::GroupedAggregateHashTable::AddChunk(duckdb::DataChunk&, duckdb::Vector&, duckdb::DataChunk&, duckdb::vector<unsigned long long, false, std::__1  ::allocator<unsigned long long>> const&) <null> (libduckdb.dylib:arm64+0x240f000)
      #8 duckdb::GroupedAggregateHashTable::AddChunk(duckdb::DataChunk&, duckdb::DataChunk&, duckdb::vector<unsigned long long, false, std::__1::allocator<unsig  ned long long>> const&) <null> (libduckdb.dylib:arm64+0x240cb30)
      duckdb#9 duckdb::RadixPartitionedHashTable::Sink(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSinkInput&, duckdb::DataChunk&, duckdb::vector<u  nsigned long long, false, std::__1::allocator<unsigned long long>> const&) const <null> (libduckdb.dylib:arm64+0x2455190)
      duckdb#10 duckdb::PhysicalHashAggregate::Sink(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSinkInput&) const physical_hash_aggregate.cpp:466 (  libduckdb.dylib:arm64+0x212bafc)

  SUMMARY: ThreadSanitizer: data race (libduckdb.dylib:arm64+0x2a37af0) in duckdb::ArenaAllocator::AlignNext()+0x2c
  ==================
```
</details>

`SUMMARIZE` calculates three `approx_quantile` aggregates (`q25`, `q50`,
`q75`). Each aggregate owns a TDigest whose vectors use the hash table’s
`ArenaAllocator`.

Two hash-aggregate worker threads concurrently finalized separate
TDigest states:

- Both called `TDigest::process()` → `updateCumulative()` →
`vector::reserve()`.
- The states shared the same non-thread-safe `ArenaAllocator`.
- One thread read allocator position metadata in
`ArenaAllocator::AlignNext()` while another wrote it.
- The shared arena had originally been allocated during
`approx_quantile` aggregation on the main thread.

Result: a race in arena allocation during parallel `approx_quantile`
finalization, reported at `ArenaAllocator::AlignNext()`.

It was intermittent because `reserve()` only occurs when vector capacity
must grow, and the worker calls had to overlap closely enough.

The fix reserves the cumulative buffer and sufficient centroid merge
capacity when the TDigest is constructed and its allocator is still used
by only one thread. Parallel finalization then operates entirely within
already allocated buffers and no longer mutates the shared arena.
redox pushed a commit that referenced this pull request Sep 29, 2026
@Mytherin
Bound the unselected prefix range key lookup by the filter count (duc…
673b4d9 
…kdb#25378)

`PrefixRangeBitmap::LookupKeys` (no-selection overload) ignored its
`count`
parameter and iterated `keys.ValidValues<T>()`, which spans the vector's
buffer capacity rather than the rows being filtered. The caller sizes
`result_sel` to `approved_tuple_count` via `PrepareCapacity`, so a
vector
holding more valid values than that count overflows the selection
buffer.

The write is unconditional per iteration (the cursor only advances on a
match), so it overflows even when nothing matches.

Fixed by bounding the loop by `count`, matching the selection overload
directly below it.

Found via AddressSanitizer debugging a flaky ducklake test:

```
WRITE of size 4 at 0x7767409efc78 thread T73
  #0 SelectionVector::set_index                          selection_vector.hpp:126
  #1 PrefixRangeBitmap<uint64_t>::LookupKeys<int64_t,..> table_filter_prefix_range_function.cpp:148
  #2 NumericPrefixRangeFilter<int64_t>::LookupKeys       :322
  #3 PrefixRangeFilterExecutor::FilterSelection          table_filter_state.cpp:386
0x7767409efc78 is located 0 bytes after 88-byte region
```

88 bytes = 22 `sel_t` entries; the write lands at index 22.

No test: existing prefix-range tests do not trip ASAN, and I could not
construct a deterministic DuckDB-level reproducer. Observed via a
DuckLake
concurrency test on a musl build, where the corrupted chunk aborts in
`free()`; glibc tolerates it silently.

Made with AI help
redox pushed a commit that referenced this pull request Sep 29, 2026
@Mytherin
Fix positional column references in macro arguments (duckdb#25763)
2be21ff 
Passing positional column references (#1, #2) to a macro can bind them
by the input column names instead of their argument positions. If those
names match the macro parameters in a different order, the query
silently returns the wrong result. If the names do not match, the call
can fail with a Binder Error.
Skip inferred aliases for positional references inside function
expressions, matching the existing handling of ordinary column
references from duckdb#9990. Explicit argument names and output aliases
outside function calls are preserved.

Fixes duckdb#25758 duckdb#19516
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@redox redox redox approved these changes
@leonkenneth leonkenneth Awaiting requested review from leonkenneth
@leo-altertable leo-altertable Awaiting requested review from leo-altertable

Assignees

@utay utay

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@utay @redox