From 147098a4c9cbfc92137ea505a62a8db1b442b21b Mon Sep 17 00:00:00 2001 From: Josh Ventura Date: Wed, 22 Jul 2026 10:40:45 -0400 Subject: [PATCH] Ship constant NULL-carrying IN lists via native IN in truth context 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. --- src/deparse.c | 77 +++++++++++++++++++++++++++-- test/expected/in_null_semantics.out | 60 +++++++++++++++++----- test/sql/in_null_semantics.sql | 25 +++++++--- 3 files changed, 140 insertions(+), 22 deletions(-) diff --git a/src/deparse.c b/src/deparse.c index 66d50d0b..5c813ebc 100644 --- a/src/deparse.c +++ b/src/deparse.c @@ -121,6 +121,17 @@ typedef struct deparse_expr_cxt { bool array_as_tuple; /* determines array output format */ bool no_sort_parens; /* determines sort group clause format */ + /* + * True while the expression being deparsed only qualifies rows (as in a + * filter). For our purposes, denotes NULL and FALSE are interchangeable. + * This is the deparse-time analog of the walker's `EXPR_CTX_TRUTH`. + * Set to true where the walker sets `EXPR_CTX_TRUTH` (appendConditions, + * SubPlan quals, aggregate FILTERs, CASE WHEN conditions), restored to + * false by `deparseExpr` anywhere the walker assigns either other value. + * Only used to reduce the number of guards emitted by queries. + */ + bool truth_ctx; + /* * True when the statement being deparsed inlines at least one SubPlan. * Computed once up front (a contain_subplans scan over the tlist/quals) @@ -2352,6 +2363,9 @@ appendConditions(List* exprs, deparse_expr_cxt* context) { bool is_first = true; StringInfo buf = context->buf; + /* These conditions only qualify rows (walker: EXPR_CTX_TRUTH) */ + context->truth_ctx = true; + foreach (lc, exprs) { Expr* expr = (Expr*)lfirst(lc); @@ -2371,6 +2385,8 @@ appendConditions(List* exprs, deparse_expr_cxt* context) { is_first = false; } + + context->truth_ctx = false; } /* Output join name for given join type */ @@ -2898,10 +2914,34 @@ deparseStringLiteral(StringInfo buf, const char* val) { */ static void deparseExpr(Expr* node, deparse_expr_cxt* context) { + bool truth_ctx; + if (node == NULL) { return; } + /* + * This block should mirror foreign_expr_walker's `ExprTruthCtx` transitions + * (which exist as scattered logic within the walker that cannot be easily + * extracted for reuse here). An `EXPR_CTX_TRUTH` context survives only + * into the nodes below: deparseRelabelType passes it through untouched (a + * cast has no operands of its own to distinguish), while deparseBoolExpr + * (under NOT), deparseCaseExpr, and deparseScalarArrayOpExpr unset it + * internally for whichever of their operands the walker also treats as + * exact. Every other node uses its children's exact values. + */ + truth_ctx = context->truth_ctx; + switch (nodeTag(node)) { + case T_BoolExpr: + case T_CaseExpr: + case T_RelabelType: + case T_ScalarArrayOpExpr: + break; + default: + context->truth_ctx = false; + break; + } + switch (nodeTag(node)) { case T_Var: deparseVar((Var*)node, context); @@ -2976,6 +3016,9 @@ deparseExpr(Expr* node, deparse_expr_cxt* context) { elog(ERROR, "unsupported expression type for deparse: %d", (int)nodeTag(node)); break; } + + /* Restore for siblings: one child's degrade must not leak sideways. */ + context->truth_ctx = truth_ctx; } /* @@ -3651,11 +3694,14 @@ deparseSubPlan(SubPlan* node, deparse_expr_cxt* context) { */ static void deparseSubPlanQuals(Node* quals, deparse_expr_cxt* context) { + /* Interior quals only qualify the subquery's own rows: truth context */ + context->truth_ctx = true; if (IsA(quals, List)) { appendConditions((List*)quals, context); } else { deparseExpr((Expr*)quals, context); } + context->truth_ctx = false; } /* @@ -5364,10 +5410,14 @@ deparseScalarArrayOpExpr(ScalarArrayOpExpr* node, deparse_expr_cxt* context) { Expr* arg1 = linitial(node->args); Expr* arg2 = lsecond(node->args); CHEqualOp optype = chfdw_is_equal_op(node->opno); + bool truth_ctx = context->truth_ctx; foreign_glob_cxt glob_cxt; SaopArrayNulls nulls; bool cheap_ok; + /* Both arguments' values are observable inputs (walker: EXPR_CTX_EXACT) */ + context->truth_ctx = false; + if (optype == CH_OP_NONE) { ereport( ERROR, @@ -5390,9 +5440,13 @@ deparseScalarArrayOpExpr(ScalarArrayOpExpr* node, deparse_expr_cxt* context) { * Very narrow case for IN()/NOT IN(): the native form is exact once the * array is a nice, non-empty, provably NULL-free constant, regardless * of the probe (ClickHouse's native IN propagates a NULL probe - * correctly; see the block comment above) + * correctly; see the block comment above). A list that may carry a NULL + * diverges only toward FALSE for `IN` (which does not matter in a TRUTH + * context) and toward TRUE for `NOT IN` (which breaks any context). */ - if (nulls == SAOP_ARR_NULL_FREE && is_ok_in_expr(arg2) && + if ((nulls == SAOP_ARR_NULL_FREE || + (truth_ctx && node->useOr && optype == CH_OP_EQ)) && + is_ok_in_expr(arg2) && ((node->useOr && optype == CH_OP_EQ) || (!node->useOr && optype == CH_OP_NE))) { appendStringInfoChar(buf, '('); deparseAsIn(node, context, optype); @@ -5469,6 +5523,9 @@ deparseBoolExpr(BoolExpr* node, deparse_expr_cxt* context) { case NOT_EXPR: { Node* arg = (Node*)linitial(node->args); + /* FALSE-for-NULL below NOT surfaces as TRUE-for-NULL above: exact */ + context->truth_ctx = false; + /* * NOT over an ANY SubPlan is Postgres's x NOT IN (SELECT ..). When a * NULL could reach the comparison the native complement is wrong; @@ -5740,9 +5797,11 @@ deparsePartialStatArray(Aggref* node, AggPartialKind kind, deparse_expr_cxt* con /* Capture FILTER condition as each component's -If argument. */ if (node->aggfilter) { - cf = psprintf( + context->truth_ctx = true; /* FILTER only qualifies rows */ + cf = psprintf( ", (%s) > 0", deparseExprToString((Expr*)node->aggfilter, context) ); + context->truth_ctx = false; } if (kind == AGG_PARTIAL_AVG_INT) { @@ -6005,7 +6064,9 @@ deparseAggref(Aggref* node, deparse_expr_cxt* context) { if (node->aggfilter) { appendStringInfoString(buf, "(("); + context->truth_ctx = true; /* FILTER only qualifies rows */ deparseExpr((Expr*)node->aggfilter, context); + context->truth_ctx = false; appendStringInfoString(buf, ") > 0)"); } @@ -6030,7 +6091,9 @@ deparseAggref(Aggref* node, deparse_expr_cxt* context) { appendStringInfoString(buf, fpinfo->ch_table_sign_field); appendStringInfoChar(buf, ','); if (node->aggfilter) { + context->truth_ctx = true; /* FILTER only qualifies rows */ deparseExpr((Expr*)node->aggfilter, context); + context->truth_ctx = false; appendStringInfoString(buf, " AND "); } deparseExpr((Expr*)((TargetEntry*)linitial(node->args))->expr, context); @@ -6215,7 +6278,8 @@ deparseCaseExpr(CaseExpr* node, deparse_expr_cxt* context) { StringInfo buf = context->buf; ListCell* lc; - char* conv = NULL; + char* conv = NULL; + bool truth_ctx = context->truth_ctx; if (node->casetype == INT2OID || node->casetype == INT4OID || node->casetype == INT8OID) { @@ -6226,6 +6290,7 @@ deparseCaseExpr(CaseExpr* node, deparse_expr_cxt* context) { appendStringInfoString(buf, "CASE"); if (node->arg) { appendStringInfoChar(buf, ' '); + context->truth_ctx = false; /* the comparison value is observable */ deparseExpr(node->arg, context); } @@ -6234,7 +6299,11 @@ deparseCaseExpr(CaseExpr* node, deparse_expr_cxt* context) { Assert(IsA(arg, CaseWhen)); appendStringInfoString(buf, " WHEN "); + /* A WHEN condition takes the same branch for NULL and FALSE... */ + context->truth_ctx = true; deparseExpr(arg->expr, context); + /* ...while a result is only truth-tolerant if the CASE itself is */ + context->truth_ctx = truth_ctx; /* * in simple cases like WHEN val THEN we should extend the condition diff --git a/test/expected/in_null_semantics.out b/test/expected/in_null_semantics.out index 356b254f..769f762f 100644 --- a/test/expected/in_null_semantics.out +++ b/test/expected/in_null_semantics.out @@ -6,10 +6,12 @@ -- The has()/countEqual() functions also match a NULL probe against a NULL -- element as if it were an ordinary value. Every shape below ships regardless: -- deparse.c uses the cheap native form when it can prove neither side can be --- NULL, and a guarded CASE otherwise that checks at runtime instead, computing --- Postgres's exact three-valued answer (NULL, not FALSE, or worse, TRUE) --- in every context. These tests ensure both forms deparse correctly and that --- the rows returned match local (Postgres) semantics either way. +-- NULL (or when the only possible divergence is FALSE-for-NULL and the result +-- merely qualifies rows), and a guarded CASE otherwise that checks at runtime +-- instead, computing Postgres's exact three-valued answer (NULL, not FALSE, +-- or worse, TRUE) in every context. These tests ensure both forms deparse +-- correctly and that the rows returned match local (Postgres) semantics +-- either way. CREATE SERVER in_null_svr FOREIGN DATA WRAPPER clickhouse_fdw OPTIONS(dbname 'in_null_test', driver 'binary'); CREATE USER MAPPING FOR CURRENT_USER SERVER in_null_svr; @@ -79,15 +81,15 @@ SELECT id FROM tnull WHERE xn IN (1, 500) ORDER BY id; 1 (1 row) --- A NULL in the list can't be proven absent, so this ships via a guarded --- CASE instead of the native IN, computing the real answer at runtime +-- A NULL in the list only pulls results toward FALSE, which a WHERE treats +-- like the NULL Postgres computes: still the native IN EXPLAIN (VERBOSE, COSTS OFF) SELECT id FROM tnull WHERE xn IN (1, NULL) ORDER BY id; - QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ + QUERY PLAN +----------------------------------------------------------------------------------------------------- Foreign Scan on in_null_test.tnull Output: id - Remote SQL: SELECT id FROM in_null_test.tnull WHERE ((CASE WHEN xn IS NULL AND notEmpty([1,NULL]) THEN NULL WHEN countEqual([1,NULL], xn) > 0 THEN true WHEN countEqual([1,NULL], NULL) > 0 THEN NULL ELSE false END)) ORDER BY id ASC NULLS LAST + Remote SQL: SELECT id FROM in_null_test.tnull WHERE ((xn IN (1,NULL))) ORDER BY id ASC NULLS LAST (3 rows) SELECT id FROM tnull WHERE xn IN (1, NULL) ORDER BY id; @@ -96,6 +98,40 @@ SELECT id FROM tnull WHERE xn IN (1, NULL) ORDER BY id; 1 (1 row) +-- OR preserves the truth context, so the native IN still ships +EXPLAIN (VERBOSE, COSTS OFF) +SELECT id FROM tnull WHERE xp > 500 OR xn IN (1, NULL) ORDER BY id; + QUERY PLAN +--------------------------------------------------------------------------------------------------------------------- + Foreign Scan on in_null_test.tnull + Output: id + Remote SQL: SELECT id FROM in_null_test.tnull WHERE (((xp > 500) OR (xn IN (1,NULL)))) ORDER BY id ASC NULLS LAST +(3 rows) + +SELECT id FROM tnull WHERE xp > 500 OR xn IN (1, NULL) ORDER BY id; + id +---- + 1 +(1 row) + +-- An IS NULL test observes the exact value even inside a WHERE clause: the +-- guarded CASE ships instead, keeping id 2's NULL distinct from a FALSE +EXPLAIN (VERBOSE, COSTS OFF) +SELECT id FROM tnull WHERE (xn IN (1, NULL)) IS NULL ORDER BY id; + QUERY PLAN +--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + Foreign Scan on in_null_test.tnull + Output: id + Remote SQL: SELECT id FROM in_null_test.tnull WHERE (((CASE WHEN xn IS NULL AND notEmpty([1,NULL]) THEN NULL WHEN countEqual([1,NULL], xn) > 0 THEN true WHEN countEqual([1,NULL], NULL) > 0 THEN NULL ELSE false END) IS NULL)) ORDER BY id ASC NULLS LAST +(3 rows) + +SELECT id FROM tnull WHERE (xn IN (1, NULL)) IS NULL ORDER BY id; + id +---- + 2 + 3 +(2 rows) + -- ============================================================ -- 2. NOT IN over a constant list: always pushable, native when NULL-free -- ============================================================ @@ -493,12 +529,12 @@ SELECT xn <> ANY(ARRAY[xn, 500]) AS flag, count(*) FROM tnull GROUP BY flag ORDE -- An aggregate FILTER only qualifies rows, so it keeps truth-context rules EXPLAIN (VERBOSE, COSTS OFF) SELECT count(*) FILTER (WHERE xn IN (1, NULL)) FROM tnull; - QUERY PLAN --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + QUERY PLAN +-------------------------------------------------------------------------------- Foreign Scan Output: (count(*) FILTER (WHERE (xn = ANY ('{1,NULL}'::integer[])))) Relations: Aggregate on (tnull) - Remote SQL: SELECT countIf((((CASE WHEN xn IS NULL AND notEmpty([1,NULL]) THEN NULL WHEN countEqual([1,NULL], xn) > 0 THEN true WHEN countEqual([1,NULL], NULL) > 0 THEN NULL ELSE false END)) > 0)) FROM in_null_test.tnull + Remote SQL: SELECT countIf((((xn IN (1,NULL))) > 0)) FROM in_null_test.tnull (4 rows) SELECT count(*) FILTER (WHERE xn IN (1, NULL)) FROM tnull; diff --git a/test/sql/in_null_semantics.sql b/test/sql/in_null_semantics.sql index ece252da..48c2024d 100644 --- a/test/sql/in_null_semantics.sql +++ b/test/sql/in_null_semantics.sql @@ -6,10 +6,12 @@ -- The has()/countEqual() functions also match a NULL probe against a NULL -- element as if it were an ordinary value. Every shape below ships regardless: -- deparse.c uses the cheap native form when it can prove neither side can be --- NULL, and a guarded CASE otherwise that checks at runtime instead, computing --- Postgres's exact three-valued answer (NULL, not FALSE, or worse, TRUE) --- in every context. These tests ensure both forms deparse correctly and that --- the rows returned match local (Postgres) semantics either way. +-- NULL (or when the only possible divergence is FALSE-for-NULL and the result +-- merely qualifies rows), and a guarded CASE otherwise that checks at runtime +-- instead, computing Postgres's exact three-valued answer (NULL, not FALSE, +-- or worse, TRUE) in every context. These tests ensure both forms deparse +-- correctly and that the rows returned match local (Postgres) semantics +-- either way. CREATE SERVER in_null_svr FOREIGN DATA WRAPPER clickhouse_fdw OPTIONS(dbname 'in_null_test', driver 'binary'); CREATE USER MAPPING FOR CURRENT_USER SERVER in_null_svr; @@ -43,12 +45,23 @@ EXPLAIN (VERBOSE, COSTS OFF) SELECT id FROM tnull WHERE xn IN (1, 500) ORDER BY id; SELECT id FROM tnull WHERE xn IN (1, 500) ORDER BY id; --- A NULL in the list can't be proven absent, so this ships via a guarded --- CASE instead of the native IN, computing the real answer at runtime +-- A NULL in the list only pulls results toward FALSE, which a WHERE treats +-- like the NULL Postgres computes: still the native IN EXPLAIN (VERBOSE, COSTS OFF) SELECT id FROM tnull WHERE xn IN (1, NULL) ORDER BY id; SELECT id FROM tnull WHERE xn IN (1, NULL) ORDER BY id; +-- OR preserves the truth context, so the native IN still ships +EXPLAIN (VERBOSE, COSTS OFF) +SELECT id FROM tnull WHERE xp > 500 OR xn IN (1, NULL) ORDER BY id; +SELECT id FROM tnull WHERE xp > 500 OR xn IN (1, NULL) ORDER BY id; + +-- An IS NULL test observes the exact value even inside a WHERE clause: the +-- guarded CASE ships instead, keeping id 2's NULL distinct from a FALSE +EXPLAIN (VERBOSE, COSTS OFF) +SELECT id FROM tnull WHERE (xn IN (1, NULL)) IS NULL ORDER BY id; +SELECT id FROM tnull WHERE (xn IN (1, NULL)) IS NULL ORDER BY id; + -- ============================================================ -- 2. NOT IN over a constant list: always pushable, native when NULL-free -- ============================================================