Skip to content

Ship constant NULL-carrying IN lists via native IN in truth context #321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
JoshDreamland wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Ship constant NULL-carrying IN lists via native IN in truth context #321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 73 additions & 4 deletions src/deparse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);

Expand All @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

/*
Expand Down Expand Up @@ -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;
}

/*
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)");
}

Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}

Expand All @@ -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
Expand Down
60 changes: 48 additions & 12 deletions test/expected/in_null_semantics.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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
-- ============================================================
Expand Down Expand Up @@ -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;
Expand Down
25 changes: 19 additions & 6 deletions test/sql/in_null_semantics.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
-- ============================================================
Expand Down
Loading