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 -- ============================================================