Skip to content

Add IN list sqllogictest test (and integer type coverage)#23305

Open
alamb wants to merge 1 commit into
apache:mainfrom
alamb:codex/in-list-slt-integers
Open

Add IN list sqllogictest test (and integer type coverage)#23305
alamb wants to merge 1 commit into
apache:mainfrom
alamb:codex/in-list-slt-integers

Conversation

@alamb

@alamb alamb commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

We (mostly @geoffreyclaude ) are in the process of optimizing IN lists with specialized implementations for various different data types.

@kosiew suggested in #23299 (comment) that we add SQL level coverage for the IN list optimizations to both cover the dispatch logic as well as ensure everything works end to end.

One small follow-up idea: it could be useful to add a public-path regression through InListExpr::try_new_from_array, or a SQL-level test, for Int8 and Int16 signed values.

What changes are included in this PR?

  1. Add in_list.slt file to specifically target the IN lists
  2. Only add test coverage for integer types

note I think we should have significiantly more SLT coverage (list on #23307) but to keep the review small / understandable I plan to do it with several PRs

Are these changes tested?

Only tests

Are there any user-facing changes?

No

@github-actions github-actions Bot added the sqllogictest SQL Logic Tests (.slt) label Jul 3, 2026

@geoffreyclaude geoffreyclaude left a comment

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.

Very clear and hits quite a few edge cases. Could you just extend the tests to also include longer in lists (eg, 12 elements)?

@alamb alamb force-pushed the codex/in-list-slt-integers branch from 71a568f to b801c50 Compare July 3, 2026 14:12
@alamb

alamb commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Very clear and hits quite a few edge cases. Could you just extend the tests to also include longer in lists (eg, 12 elements)?

Done

@alamb

alamb commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

I also added a note to point out that the IN list is only executed for larger lists (over 4)

@alamb alamb marked this pull request as ready for review July 3, 2026 14:14
@geoffreyclaude

Copy link
Copy Markdown
Contributor

I also added a note to point out that the IN list is only executed for larger lists (over 4)

We'll have to review that optimization rule once the full IN LIST optims land!

@alamb

alamb commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

I also added a note to point out that the IN list is only executed for larger lists (over 4)

We'll have to review that optimization rule once the full IN LIST optims land!

Indeed

At some point stuff like PruningPredicate didn't know how to handle IN lists either -- but I believe now that IN LIST is handled quite well at this point across the system

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

Labels

sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants