Skip to content

Use SQL null semantics for DuckDB IN and NOT IN pushdown - #10054

Draft
robert3005 wants to merge 3 commits into
rk/list-contains-bindingsfrom
rk/duckdb-list-contains
Draft

robert3005 wants to merge 3 commits into
rk/list-contains-bindingsfrom
rk/duckdb-list-contains

Conversation

@robert3005

@robert3005 robert3005 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

DuckDB membership pushdown used Vortex's default null rules, which can make a nonmatch in NOT IN (1, NULL) true. Use SQL null semantics so that result remains unknown and the row is excluded.

Stacked on #10053; uses SQL null semantics from #10057 and membership optimizations from #10052.

Changes

  • Convert both bound IN/NOT IN expressions and IN table filters through in_list.
  • Leave membership lists containing nonconstant expressions in DuckDB instead of advertising support and then failing conversion.
  • Add result and EXPLAIN regressions for null elements in either position, nullable needles, negation, scans through views, and a 1,000-element set. Add a nonconstant-list fallback regression.

Validation: cargo clippy -p vortex-duckdb --all-targets --all-features -- -D warnings passed on this branch. Runtime tests were not executed.

@codspeed

codspeed Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ 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.

✅ 2070 untouched benchmarks
⏩ 525 skipped benchmarks1


Comparing rk/duckdb-list-contains (e4b6598) with rk/list-contains-bindings (2a51612)

Open in CodSpeed

Footnotes

  1. 525 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. ↩

@robert3005
robert3005 removed this pull request from stack #10055 September 25, 2026 14:05
@robert3005
robert3005 force-pushed the rk/duckdb-list-contains branch from 7f7f687 to 29fa2b0 Compare September 25, 2026 14:06
@robert3005
robert3005 added this pull request to stack #10058 September 25, 2026 14:06
@robert3005
robert3005 removed this pull request from stack #10058 September 25, 2026 14:09
@robert3005
robert3005 added this pull request to stack #10061 September 25, 2026 14:09
@robert3005
robert3005 force-pushed the rk/duckdb-list-contains branch from 29fa2b0 to adc15ee Compare September 25, 2026 14:09
@robert3005
robert3005 force-pushed the rk/duckdb-list-contains branch from adc15ee to 2535299 Compare September 25, 2026 15:53
@robert3005
robert3005 force-pushed the rk/duckdb-list-contains branch from 2535299 to cafd7d2 Compare September 25, 2026 16:28
@robert3005
robert3005 force-pushed the rk/duckdb-list-contains branch 2 times, most recently from 04bd414 to 459162a Compare September 25, 2026 17:26
@robert3005
robert3005 force-pushed the rk/duckdb-list-contains branch 2 times, most recently from c0e6065 to 843d57c Compare September 28, 2026 20:43
) {
let mut children = op.children();
return children.next().is_some_and(can_push_expression)
&& children.all(|child| matches!(child.as_class(), Some(BoundConstant(_))));

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.

this seems stricter than before

TableFilterClass::InFilter(values) => {
// TODO(ngates): I'm pretty sure we actually need this as ScalarValue with the
// scope dtype
let scalars: Vec<_> = values.iter().map(Scalar::try_from).try_collect()?;

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.

we should remove this

Convert both IN table filters and bound membership expressions through
in_list, so a null set element leaves nonmatches unknown. Decline
nonconstant lists instead of promising conversion and failing later.

Add scan-result and EXPLAIN regressions for nullable sets, views, large
constant lists, and fallback for nonconstant list elements.

Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005
robert3005 force-pushed the rk/duckdb-list-contains branch from 843d57c to e4b6598 Compare September 29, 2026 19:13

This branch has not been deployed

No deployments
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