Repository navigation
Conversation
Signed-off-by: peterxcli <peterxcli@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors plan_limit.cpp by moving the GetLimit helper function and updating UseBatchLimit to utilize it, reducing code duplication. Additionally, it introduces const qualifiers to the BoundLimitNode parameters in both functions for better const-correctness. I have no feedback to provide.
peterxcli
pushed a commit
that referenced
this pull request
Apr 27, 2026
### Remove copied extensions from release/relassert artifact The tarball for `relassert` contains duplicate extensions: ``` relassert artifact includes: duckdb extension/core_functions/core_functions.duckdb_extension extension/parquet/parquet.duckdb_extension repository/36a968b1bf/linux_amd64/core_functions.duckdb_extension repository/36a968b1bf/linux_amd64/parquet.duckdb_extension src/libduckdb.so test/extension/loadable_extension_demo.duckdb_extension test/extension/loadable_extension_optimizer_demo.duckdb_extension test/unittest relassert artifact size: 470M build/relassert-artifact/relassert/extension/parquet 530M build/relassert-artifact/relassert/extension/core_functions 592M build/relassert-artifact/relassert/src 870M build/relassert-artifact/relassert/test/extension 1000M build/relassert-artifact/relassert/extension 1000M build/relassert-artifact/relassert/repository 1000M build/relassert-artifact/relassert/repository/36a968b1bf 1000M build/relassert-artifact/relassert/repository/36a968b1bf/linux_amd64 1.5G build/relassert-artifact/relassert/test 4.6G build/relassert-artifact/relassert ``` because these two paths: ``` extension/*/*.duckdb_extension repository/36a968b1bf/linux_amd64/*.duckdb_extension ``` appears to contain binary file copies. ### Show symbols for stacktrace in `linux-debug` Before, on `linux-debug`, we would [see](https://github.com/duckdb/duckdb/actions/runs/24786615493/job/72534047134) a traceback with no symbols: ``` build/relassert/test/unittest(+0x4447240) [0x560387001240] build/relassert/test/unittest(+0x7b63367) [0x56038a71d367] build/relassert/test/unittest(+0x7b66aec) [0x56038a720aec] build/relassert/test/unittest(+0x7b7447c) [0x56038a72e47c] build/relassert/test/unittest(+0x7b79e61) [0x56038a733e61] build/relassert/test/unittest(+0x7b722c3) [0x56038a72c2c3] build/relassert/test/unittest(+0x7b75a79) [0x56038a72fa79] build/relassert/test/unittest(+0x7b7c248) [0x56038a736248] build/relassert/test/unittest(+0x7b9b018) [0x56038a755018] build/relassert/test/unittest(+0x301e5d4) [0x560385bd85d4] build/relassert/test/unittest(+0x3ba5b9f) [0x56038675fb9f] build/relassert/test/unittest(+0x3b9e56e) [0x56038675856e] build/relassert/test/unittest(+0x3b9b716) [0x560386755716] build/relassert/test/unittest(+0x3bafee7) [0x560386769ee7] build/relassert/test/unittest(+0x3bad196) [0x560386767196] build/relassert/test/unittest(+0x3bf2c57) [0x5603867acc57] /lib/x86_64-linux-gnu/libc.so.6(+0x2a1ca) [0x7fc4a01b61ca] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x8b) [0x7fc4a01b628b] build/relassert/test/unittest(+0x2ee2065) [0x560385a9c065] ``` After, we get a stacktrace with symbols. Use `-rdynamic` as linker flag to fix unused CLI flag warning. ### `-rdynamic` caused ODR violations With `EXPORT_DYNAMIC_SYMBOLS: 1` for `linux-debug`, address sanitizer [finds](https://github.com/duckdb/duckdb/actions/runs/24837199395/job/72703372246?pr=22246#step:9:22) One Definition Rule violations: ``` ==437==ERROR: AddressSanitizer: odr-violation (0x7f85b6a36b60): [1] size=256 'LOOKUP_TABLE' /home/runner/work/duckdb/duckdb/src/function/cast/nested_to_varchar_cast.cpp:5:12 in /home/runner/work/duckdb/duckdb/build/relassert/src/libduckdb.so [2] size=256 'LOOKUP_TABLE' /home/runner/work/duckdb/duckdb/src/function/cast/nested_to_varchar_cast.cpp:5:12 in /home/runner/work/duckdb/duckdb/build/relassert/test/unittest These globals were registered at these points: [1]: #0 0x7f85d158906f in __asan_register_globals ../../../../src/libsanitizer/asan/asan_globals.cpp:350 #1 0x7f85d1c2071e in call_init elf/dl-init.c:74 #2 0x7f85d1c20823 in call_init elf/dl-init.c:120 #3 0x7f85d1c20823 in _dl_init elf/dl-init.c:121 #4 0x7f85d1c1c5b1 in __GI__dl_catch_exception elf/dl-catch.c:211 duckdb#5 0x7f85d1c27d7b in dl_open_worker elf/dl-open.c:829 duckdb#6 0x7f85d1c27d7b in dl_open_worker elf/dl-open.c:792 duckdb#7 0x7f85d1c1c51b in __GI__dl_catch_exception elf/dl-catch.c:237 duckdb#8 0x7f85d1c28163 in _dl_open elf/dl-open.c:905 duckdb#9 0x7f85d0a0f1a3 in dlopen_doit dlfcn/dlopen.c:56 duckdb#10 0x7f85d1c1c51b in __GI__dl_catch_exception elf/dl-catch.c:237 duckdb#11 0x7f85d1c1c668 in _dl_catch_error elf/dl-catch.c:256 duckdb#12 0x7f85d0a0ec82 in _dlerror_run dlfcn/dlerror.c:138 duckdb#13 0x7f85d0a0f25e in dlopen_implementation dlfcn/dlopen.c:71 duckdb#14 0x7f85d0a0f25e in ___dlopen dlfcn/dlopen.c:81 duckdb#15 0x7f85d15b2f08 in dlopen ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:6341 duckdb#16 0x55ad3185a30f in AdbcLoadDriver (/home/runner/work/duckdb/duckdb/build/relassert/test/unittest+0xcf3630f) (BuildId: fbeec3ab975633c9b9b6c6233c1545043737a86a) [2]: #0 0x7f85d158906f in __asan_register_globals ../../../../src/libsanitizer/asan/asan_globals.cpp:350 #1 0x7f85d09a1303 in call_init ../csu/libc-start.c:145 #2 0x7f85d09a1303 in __libc_start_main_impl ../csu/libc-start.c:347 #3 0x55ad31808ea4 in _start (/home/runner/work/duckdb/duckdb/build/relassert/test/unittest+0xcee4ea4) (BuildId: fbeec3ab975633c9b9b6c6233c1545043737a86a) ``` The errors are caused by duplicated symbols. The symbols are present in both the unittest binary and the loaded shared object `libduckdb.so`. We can solve the errors by separating unittest object files and linking the `unittest` binary to `libduckdb.so`. ### Differences in compressed (gzip -4) artifacts build type | before | after | reduction -- | -- | -- | -- linux-relassert-build | 1250 MB | 873 MB | 377 MB (30.2%) linux-release-build | 258 MB | 170 MB | 88 MB (34.1%)
peterxcli
pushed a commit
that referenced
this pull request
Sep 21, 2026
<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)
duckdb#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)
duckdb#6 duckdb::RowOperations::FinalizeStates(duckdb::RowOperationsState&, duckdb::TupleDataLayout&, duckdb::Vector&, duckdb::DataChunk&, unsigned long long) r ow_aggregate.cpp:182 (libduckdb.dylib:arm64+0x166f848)
duckdb#7 duckdb::RadixHTLocalSourceState::Scan(duckdb::RadixHTGlobalSinkState&, duckdb::RadixHTGlobalSourceState&, duckdb::DataChunk&) <null> (libduckdb.dylib:a rm64+0x2458dac)
duckdb#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)
duckdb#5 duckdb::RowOperations::FinalizeStates(duckdb::RowOperationsState&, duckdb::TupleDataLayout&, duckdb::Vector&, duckdb::DataChunk&, unsigned long long) r ow_aggregate.cpp:182 (libduckdb.dylib:arm64+0x166f848)
duckdb#6 duckdb::RadixHTLocalSourceState::Scan(duckdb::RadixHTGlobalSinkState&, duckdb::RadixHTGlobalSourceState&, duckdb::DataChunk&) <null> (libduckdb.dylib:a rm64+0x2458dac)
duckdb#7 duckdb::RadixHTLocalSourceState::ExecuteTask(duckdb::RadixHTGlobalSinkState&, duckdb::RadixHTGlobalSourceState&, duckdb::DataChunk&) <null> (libduckdb. dylib:arm64+0x2457ea0)
duckdb#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)
duckdb#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)
duckdb#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)
duckdb#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)
duckdb#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.
peterxcli
pushed a commit
that referenced
this pull request
Sep 21, 2026
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.