Skip to content

simplify projection expression for SELECT * in duckdb#7885

Open
myrrc wants to merge 1 commit into
developfrom
myrrc/duckdb-star
Open

simplify projection expression for SELECT * in duckdb#7885
myrrc wants to merge 1 commit into
developfrom
myrrc/duckdb-star

Conversation

@myrrc
Copy link
Copy Markdown
Contributor

@myrrc myrrc commented May 11, 2026

In case we have a SELECT * with possible virtual columns, use
root() and not select(names, root()) in duckdb

@myrrc myrrc added changelog/feature A new feature ext/duckdb Relates to the DuckDB integration labels May 11, 2026
@myrrc myrrc requested a review from joseph-isaacs May 11, 2026 16:12
@myrrc myrrc enabled auto-merge (squash) May 11, 2026 16:14
Signed-off-by: Mikhail Kot <to@myrrc.dev>
@myrrc myrrc force-pushed the myrrc/duckdb-star branch from d2a4563 to 3937337 Compare May 11, 2026 16:19
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 11, 2026

Merging this PR will degrade performance by 24.75%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

❌ 6 regressed benchmarks
✅ 1204 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation new_bp_prim_test_between[i16, 32768] 120.6 µs 134.6 µs -10.36%
Simulation new_bp_prim_test_between[i32, 16384] 95 µs 109.4 µs -13.12%
Simulation new_bp_prim_test_between[i32, 32768] 141.4 µs 170.2 µs -16.96%
Simulation new_bp_prim_test_between[i64, 32768] 178.5 µs 237.2 µs -24.75%
Simulation new_bp_prim_test_between[i64, 16384] 115.6 µs 144.9 µs -20.23%
Simulation new_alp_prim_test_between[f64, 16384] 127.2 µs 149.1 µs -14.66%

Comparing myrrc/duckdb-star (3937337) with develop (93d0b4e)

Open in CodSpeed

if column_id == FILE_ROW_NUMBER_COLUMN_IDX {
file_row_number_column_pos = Some(column_pos);
}
is_star &= column_id == real_column_count;
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.

explain this.

let mut real_column_count = 0;

for (column_pos, &column_id) in ids.iter().enumerate() {
#[expect(clippy::cast_possible_truncation)]
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.

not this please


pub fn projection_ids(&self) -> Option<&[u64]> {
if self.input.projection_ids.is_null() {
if self.input.projection_ids.is_null() || self.input.projection_ids_count == 0 {
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.

this is unexpected

Copy link
Copy Markdown
Contributor

@joseph-isaacs joseph-isaacs left a comment

Choose a reason for hiding this comment

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

no clippy allow please

Copy link
Copy Markdown
Contributor

@joseph-isaacs joseph-isaacs left a comment

Choose a reason for hiding this comment

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

can you also please explain this logic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature ext/duckdb Relates to the DuckDB integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants