Push down the array IN family unconditionally - #317
Merged
Conversation
theory
added
pushdown
This was referenced Jul 21, 2026
JoshDreamland
force-pushed
the
notin-any-array
branch
from
July 22, 2026 15:35
7471763 to
5c7cd83
Compare
JoshDreamland
marked this pull request as ready for review
July 22, 2026 15:35
JoshDreamland
force-pushed
the
notin-any-array
branch
2 times, most recently
from
July 22, 2026 16:03
e945fe2 to
8986019
Compare
JoshDreamland
added a commit
that referenced
this pull request
Jul 22, 2026
deparseScalarArrayOpExpr's cheap native IN(...)/NOT IN(...) form requires the array to be provably NULL-free. A constant list with a literal NULL (x IN (1, 2, NULL)) always fails that and falls through to the guarded CASE, even in a WHERE clause, where the only actual divergence (a non-NULL, non-matching probe against a NULL-containing list resolving to FALSE natively where Postgres computes NULL) is invisible: NULL and FALSE are interchangeable there. This restores the cheap form for that specific case when the surrounding context tolerates it. Deliberately IN-only, not NOT IN: with transform_null_in=0, ClickHouse's native IN resolves a non-matching probe against a NULL-containing list to FALSE (tolerable), but native NOT IN resolves the same case to TRUE, which diverges from Postgres's NULL in every context, not just value-observable ones -- confirmed empirically against a live server. Mechanism: a new deparse_expr_cxt.truth_ctx bool mirrors the walker's ExprTruthCtx (foreign_expr_walker, EXPR_CTX_TRUTH specifically -- the third state, EXPR_CTX_NEGATED, can't reach a plain ScalarArrayOpExpr, since the planner folds NOT over one into its dual before we ever see it; verified via EXPLAIN). Granted at deparse's mirrors of the walker's EXPR_CTX_TRUTH grants (appendConditions, SubPlan quals, aggregate FILTER, CASE WHEN conditions) and degraded to false everywhere else via a nodeTag switch in deparseExpr. Open concern raised in review, not yet resolved: this switch is a second, hand-maintained copy of "which node types matter here" with nothing forcing it to stay in sync with deparseExpr's real dispatch switch as new node types are added, and the new context field is one more thing to carry on deparse_expr_cxt for a single consumer. Filed as its own PR against notin-any-array (#317) rather than folded in, both because the net line count wasn't a savings once test coverage for the new mechanism is included, and because the duplication concern is worth resolving before this lands -- likely by having deparse consult the walker's own already-computed verdict for a node instead of re-deriving it a second time, rather than tracking a parallel opinion.
theory
approved these changes
Jul 22, 2026
theory
left a comment
theory
left a comment
Collaborator
There was a problem hiding this comment.
This looks great, mostly just nits and desiring a bit more clarity of narrative text.
Comment on lines
+32
to
+41
| Like with `NOT IN`, a more elegant spelling is used when non-nullability can | ||
| be proven, and in other cases, an uglier shape is used which can handle NULL | ||
| values correctly (matching Postgres behavior). | ||
| * Extended that guarded shape to the rest of the array `IN` family | ||
| (`IN`/`NOT IN`, `= ANY`, `= ALL`, `<> ALL`), so all of them now push down | ||
| unconditionally, in every context (`SELECT` list and `GROUP BY` included), | ||
| instead of falling back to local evaluation whenever non-nullability | ||
| couldn't be proven. The non-nullability proof that selects the cheaper | ||
| unguarded form now also follows basic arithmetic (`+`, `-`, `*`, unary | ||
| `-`) over non-NULL constants and `NOT NULL` columns ([#317]). |
Collaborator
There was a problem hiding this comment.
This might all be more detail than necessary TBH. Just say what changed and the impact.
Though I'll likely edit it all again during release prep.
Contributor
Author
There was a problem hiding this comment.
Simplified. How's that?
Collaborator
There was a problem hiding this comment.
Much better; the IN change should be roughly the same, yes?
JoshDreamland
force-pushed
the
notin-any-array
branch
2 times, most recently
from
July 23, 2026 01:07
825aabd to
1a06fe3
Compare
The array IN family (`IN`, `NOT IN`, `= ANY`, `= ALL`, `<> ANY`, `<> ALL`) now ships unconditionally, in every context, instead of falling back to local evaluation whenever non-nullability couldn't be proven. `saop_null_semantics_ok` collapses to true (past the pre-existing empty-array and NULL-array-constant checks); deparseScalarArrayOpExpr picks the cheap native/`has()`/`countEqual()` form when it can prove neither side is NULL, and a unified `deparseGuardedScalarArrayOp` helper otherwise; see doc comment thereabove for details about guard mechanics. The non-nullability proof also now follows basic arithmetic (`+`, `-`, `*`, unary `-`) over non-NULL constants and NOT NULL columns, via a hand-picked list of operator functions: being strict alone doesn't prove a function can't return NULL from non-NULL inputs (->> on a missing JSON key is a counterexample), so each entry is individually checked. The list is sorted once at load time (a constructor-attribute function, since these OIDs aren't guaranteed identical across every supported PG version) so the check is a bsearch instead of a linear scan. A trade-off (mitigated in #321): a constant list with a possible NULL previously shipped via native `IN(...)` specifically in a truth context, where FALSE-for-NULL substitution from ClickHouse makes no difference. The deparse function has no way to see context as of this commit, so it now always guards that case instead, which sucks, but buys us pushdown for the rest of the array IN family. Also, finally patches `chfdw_is_equal_op` to return a `CHEqualOp` enum (NONE/EQ/NE) instead of magic 0/1/2 ints.
JoshDreamland
force-pushed
the
notin-any-array
branch
from
July 23, 2026 14:26
1a06fe3 to
183794d
Compare
JoshDreamland
added a commit
that referenced
this pull request
Jul 23, 2026
deparseScalarArrayOpExpr's cheap native IN(...)/NOT IN(...) form requires the array to be provably NULL-free. A constant list with a literal NULL (x IN (1, 2, NULL)) always fails that and falls through to the guarded CASE, even in a WHERE clause, where the only actual divergence (a non-NULL, non-matching probe against a NULL-containing list resolving to FALSE natively where Postgres computes NULL) is invisible: NULL and FALSE are interchangeable there. This restores the cheap form for that specific case when the surrounding context tolerates it. Deliberately IN-only, not NOT IN: with transform_null_in=0, ClickHouse's native IN resolves a non-matching probe against a NULL-containing list to FALSE (tolerable), but native NOT IN resolves the same case to TRUE, which diverges from Postgres's NULL in every context, not just value-observable ones -- confirmed empirically against a live server. Mechanism: a new deparse_expr_cxt.truth_ctx bool mirrors the walker's ExprTruthCtx (foreign_expr_walker, EXPR_CTX_TRUTH specifically -- the third state, EXPR_CTX_NEGATED, can't reach a plain ScalarArrayOpExpr, since the planner folds NOT over one into its dual before we ever see it; verified via EXPLAIN). Granted at deparse's mirrors of the walker's EXPR_CTX_TRUTH grants (appendConditions, SubPlan quals, aggregate FILTER, CASE WHEN conditions) and degraded to false everywhere else via a nodeTag switch in deparseExpr. Open concern raised in review, not yet resolved: this switch is a second, hand-maintained copy of "which node types matter here" with nothing forcing it to stay in sync with deparseExpr's real dispatch switch as new node types are added, and the new context field is one more thing to carry on deparse_expr_cxt for a single consumer. Filed as its own PR against notin-any-array (#317) rather than folded in, both because the net line count wasn't a savings once test coverage for the new mechanism is included, and because the duplication concern is worth resolving before this lands -- likely by having deparse consult the walker's own already-computed verdict for a node instead of re-deriving it a second time, rather than tracking a parallel opinion.
JoshDreamland
added a commit
that referenced
this pull request
Jul 29, 2026
deparseScalarArrayOpExpr's cheap native IN(...)/NOT IN(...) form requires the array to be provably NULL-free. A constant list with a literal NULL (x IN (1, 2, NULL)) always fails that and falls through to the guarded CASE, even in a WHERE clause, where the only actual divergence (a non-NULL, non-matching probe against a NULL-containing list resolving to FALSE natively where Postgres computes NULL) is invisible: NULL and FALSE are interchangeable there. This restores the cheap form for that specific case when the surrounding context tolerates it. Deliberately IN-only, not NOT IN: with transform_null_in=0, ClickHouse's native IN resolves a non-matching probe against a NULL-containing list to FALSE (tolerable), but native NOT IN resolves the same case to TRUE, which diverges from Postgres's NULL in every context, not just value-observable ones -- confirmed empirically against a live server. Mechanism: a new deparse_expr_cxt.truth_ctx bool mirrors the walker's ExprTruthCtx (foreign_expr_walker, EXPR_CTX_TRUTH specifically -- the third state, EXPR_CTX_NEGATED, can't reach a plain ScalarArrayOpExpr, since the planner folds NOT over one into its dual before we ever see it; verified via EXPLAIN). Granted at deparse's mirrors of the walker's EXPR_CTX_TRUTH grants (appendConditions, SubPlan quals, aggregate FILTER, CASE WHEN conditions) and degraded to false everywhere else via a nodeTag switch in deparseExpr. Open concern raised in review, not yet resolved: this switch is a second, hand-maintained copy of "which node types matter here" with nothing forcing it to stay in sync with deparseExpr's real dispatch switch as new node types are added, and the new context field is one more thing to carry on deparse_expr_cxt for a single consumer. Filed as its own PR against notin-any-array (#317) rather than folded in, both because the net line count wasn't a savings once test coverage for the new mechanism is included, and because the duplication concern is worth resolving before this lands -- likely by having deparse consult the walker's own already-computed verdict for a node instead of re-deriving it a second time, rather than tracking a parallel opinion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The array IN family (
IN,NOT IN,= ANY,= ALL,<> ANY,<> ALL) now ships unconditionally, in every context, instead of falling back to local evaluation whenever non-nullability couldn't be proven.saop_null_semantics_okcollapses to true (past the pre-existing empty-array and NULL-array-constant checks); deparseScalarArrayOpExpr picks the cheap native/has()/countEqual()form when it can prove neither side is NULL, and a unifieddeparseGuardedScalarArrayOphelper otherwise; see doc comment thereabove for details about guard mechanics.The non-nullability proof also now follows basic arithmetic (
+,-,*, unary-) over non-NULL constants and NOT NULL columns, via a hand-picked list of operator functions: being strict alone doesn't prove a function can't return NULL from non-NULL inputs (->> on a missing JSON key is a counterexample), so each entry is individuallychecked. The list is sorted once at load time (a constructor-attribute function, since these OIDs aren't guaranteed identical across every supported PG version) so the check is a bsearch instead of a linear scan.
A trade-off (mitigated in #321): a constant list with a possible NULL previously shipped via native
IN(...)specifically in a truth context, where FALSE-for-NULL substitution from ClickHouse makes no difference. The deparse function has no way to see context as of this commit, so it now always guards that case instead, which sucks, but buys us pushdown for the rest of the array IN family.Also, finally patches
chfdw_is_equal_opto return aChfdwEqualOpenum (NONE/EQ/NE) instead of magic 0/1/2 ints.