Ship constant NULL-carrying IN lists via native IN in truth context - #321
Draft
JoshDreamland wants to merge 1 commit into
Draft
Ship constant NULL-carrying IN lists via native IN in truth context#321JoshDreamland wants to merge 1 commit into
JoshDreamland wants to merge 1 commit into
Conversation
JoshDreamland
added a commit
that referenced
this pull request
Jul 22, 2026
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 shared deparseGuardedScalarArrayOp helper otherwise, parameterized directly by useOr/optype: every comparison against a NULL probe is NULL on both systems for a non-empty array, so a probe-IS-NULL branch always opens the guard; from there, = ANY and <> ALL both turn on whether ANY element matches, and = ALL and <> ANY both turn on whether a non-NULL element DOESN'T -- the same shape either way, just which side is "resolved" and which is "fallback" flips with useOr. The non-nullability proof also now follows basic arithmetic (+, -, *, unary -) over non-NULL constants and NOT NULL columns, via a vouched-for 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 `ChfdwEqualOp` enum (NONE/EQ/NE) instead of magic 0/1/2 ints.
JoshDreamland
force-pushed
the
notin-any-array
branch
from
July 22, 2026 15:35
7471763 to
5c7cd83
Compare
JoshDreamland
added a commit
that referenced
this pull request
Jul 22, 2026
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 shared deparseGuardedScalarArrayOp helper otherwise, parameterized directly by useOr/optype: every comparison against a NULL probe is NULL on both systems for a non-empty array, so a probe-IS-NULL branch always opens the guard; from there, = ANY and <> ALL both turn on whether ANY element matches, and = ALL and <> ANY both turn on whether a non-NULL element DOESN'T -- the same shape either way, just which side is "resolved" and which is "fallback" flips with useOr. 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 `ChfdwEqualOp` enum (NONE/EQ/NE) instead of magic 0/1/2 ints.
JoshDreamland
force-pushed
the
notin-any-array
branch
from
July 22, 2026 15:43
5c7cd83 to
e945fe2
Compare
JoshDreamland
added a commit
that referenced
this pull request
Jul 22, 2026
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 `ChfdwEqualOp` enum (NONE/EQ/NE) instead of magic 0/1/2 ints.
JoshDreamland
force-pushed
the
notin-any-array
branch
from
July 22, 2026 16:03
e945fe2 to
8986019
Compare
JoshDreamland
force-pushed
the
truth-ctx-guarded-in
branch
from
July 22, 2026 16:08
0d10c73 to
0762ba5
Compare
JoshDreamland
added a commit
that referenced
this pull request
Jul 23, 2026
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 00:53
258f21c to
825aabd
Compare
JoshDreamland
added a commit
that referenced
this pull request
Jul 23, 2026
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 01:07
825aabd to
1a06fe3
Compare
JoshDreamland
added a commit
that referenced
this pull request
Jul 23, 2026
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
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
force-pushed
the
truth-ctx-guarded-in
branch
from
July 23, 2026 16:29
0762ba5 to
1c24ad0
Compare
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
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.
Stacked on #317 — split out of that review rather than folded in.
What this does
deparseScalarArrayOpExpr's cheap nativeIN(...)/NOT IN(...)form requires the array to be provably NULL-free. A constant list with a literalNULL(x IN (1, 2, NULL)) always fails that and falls through to the guardedCASE, even in aWHEREclause, where the only actual divergence is invisible: a non-NULL, non-matching probe against a NULL-containing list resolves toFALSEnatively where Postgres computesNULL, andNULL/FALSEare interchangeable in a truth context. This restores the cheap form for that case when the surrounding context tolerates it.Deliberately
IN-only, notNOT IN: withtransform_null_in=0, ClickHouse's nativeINresolves a non-matching probe against a NULL-containing list toFALSE(tolerable), but nativeNOT INresolves the same case toTRUE, which diverges from Postgres'sNULLin every context, not just value-observable ones — confirmed empirically against a live server:Mechanism: a new
deparse_expr_cxt.truth_ctxbool mirrors the walker'sExprTruthCtx(foreign_expr_walker,EXPR_CTX_TRUTHspecifically — the third state,EXPR_CTX_NEGATED, can't reach a plainScalarArrayOpExpr, since the planner foldsNOTover one into its dual before we ever see it; verified viaEXPLAIN). Granted at deparse's mirrors of the walker'sEXPR_CTX_TRUTHgrants (appendConditions,SubPlanquals, aggregateFILTER,CASE WHENconditions) and degraded tofalseeverywhere else via anodeTagswitch indeparseExpr.Why this is a draft, not ready to merge
Open concern from review, not yet resolved:
The
nodeTagswitch indeparseExprthat decides which node types preserve vs. degradetruth_ctxis a second, hand-maintained copy of "which node types matter here," with nothing forcing it to stay in sync withdeparseExpr's real dispatch switch as new node types are added. And the new context field is one more thing carried ondeparse_expr_cxtfor a single consumer.Also worth noting: the net line count here isn't a savings once test coverage for the new mechanism is included (two new cases:
ORpreserving truth context, and(x IN (1,NULL)) IS NULLcorrectly degrading even inside aWHEREclause) —test/expected/in_null_semantics.outgrows by ~22 lines net versus #317, even though the behavioral revert itself shrinks it.Likely direction for resolving the duplication concern: have deparse consult the walker's own already-computed verdict for a given node (e.g. via an annotation captured during the shippability walk) instead of re-deriving the same transition rules a second time, rather than tracking a parallel opinion.
Verified
Full targeted suite (
in_null_semantics subplan_pushdown subquery_pushdown where_sub deparse_checks param gucs subquery_eq re2_functions) passes.make formatis a no-op.