Skip to content

minor refactor of get limit - #3

Open
peterxcli wants to merge 1 commit into
feat/limited-distinctfrom
feat/limited-distinct-cleanup-getlimit
Open

minor refactor of get limit#3
peterxcli wants to merge 1 commit into
feat/limited-distinctfrom
feat/limited-distinct-cleanup-getlimit

Conversation

@peterxcli

Copy link
Copy Markdown
Owner

No description provided.

Signed-off-by: peterxcli <peterxcli@gmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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%)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant