Skip to content

duckdb: more const for FFI - #9173

Merged
myrrc merged 1 commit into
developfrom
myrrc/duckdb-more-const
Aug 4, 2026
Merged

duckdb: more const for FFI#9173
myrrc merged 1 commit into
developfrom
myrrc/duckdb-more-const

Conversation

@myrrc

@myrrc myrrc commented Aug 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Remove unused duckdb_vx_set_dictionary_vector_length.
  • Move comments about duckdb callbacks from C++ to Rust side.
  • Add const modifiers to some callbacks.
  • Add helper get_ffi_* functions for C++ side.
  • Move structs to header file

#8628

@myrrc
myrrc force-pushed the myrrc/duckdb-more-const branch from 94b6047 to 779d85a Compare August 4, 2026 12:07
@myrrc myrrc added changelog/chore A trivial change ext/duckdb Relates to the DuckDB integration labels Aug 4, 2026
@myrrc
myrrc enabled auto-merge (squash) August 4, 2026 12:07
@myrrc
myrrc force-pushed the myrrc/duckdb-more-const branch from 779d85a to d49132a Compare August 4, 2026 12:18
@myrrc
duckdb: more const for FFI
c85d89d 
Signed-off-by: Mikhail Kot <[email protected]>
@myrrc
myrrc force-pushed the myrrc/duckdb-more-const branch from d49132a to c85d89d Compare August 4, 2026 15:06
@codspeed

codspeed Bot commented Aug 4, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 14.93%

⚡ 3 improved benchmarks
✅ 1839 untouched benchmarks
⏩ 44 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Simulation new_raw_prim_test_between[i32, 2048] 89.7 µs 76.5 µs +17.27%
⚡ Simulation execute_scalar_struct_simple 1.2 ms 1.1 ms +14.35%
⚡ Simulation execute_scalar_struct_wide 5.7 ms 5 ms +13.21%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.

Comparing myrrc/duckdb-more-const (c85d89d) with develop (fd43ebd)2

Open in CodSpeed

Footnotes

  1. 44 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on develop (bc9aa10) during the generation of this report, so fd43ebd was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

joseph-isaacs
joseph-isaacs reviewed Aug 4, 2026
View reviewed changes
Comment thread vortex-duckdb/cpp/include/table_function.hpp
#include "duckdb/function/function.hpp"
#include "duckdb/function/table_function.hpp"

using namespace duckdb;

@joseph-isaacs joseph-isaacs Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

are you sure? does this not export when included? Also why in this PR

@myrrc myrrc Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, this doesn't have any impact on exported symbols.

@myrrc myrrc Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why in this PR: this is a general cleanup before duckdb 2.0

joseph-isaacs
joseph-isaacs reviewed Aug 4, 2026
View reviewed changes
Comment thread vortex-duckdb/src/duckdb/vector.rs Resolved
@myrrc
myrrc requested a review from joseph-isaacs August 4, 2026 16:25
@joseph-isaacs
joseph-isaacs disabled auto-merge August 4, 2026 16:43
@myrrc
myrrc merged commit efa73f3 into develop Aug 4, 2026
111 of 117 checks passed
@myrrc
myrrc deleted the myrrc/duckdb-more-const branch August 4, 2026 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joseph-isaacs joseph-isaacs joseph-isaacs approved these changes

Assignees

No one assigned

Labels

changelog/chore A trivial change ext/duckdb Relates to the DuckDB integration

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@myrrc @joseph-isaacs