From 00165798ea90aad84c0f059fdd6f8f112346b271 Mon Sep 17 00:00:00 2001 From: Vik Fearing Date: Sat, 3 Oct 2026 17:18:22 +0200 Subject: [PATCH v3 3/3] Keep IMPLIES in stored expressions Until now a IMPLIES b was expanded to (NOT a OR b) during parse analysis, so views, check constraints, index expressions and predicates, and policies were stored, deparsed, and dumped in terms of the expansion rather than as written. Make IMPLIES a fourth BoolExprType so that it survives into the stored tree and ruleutils.c can print it back. In pretty mode, NOT, AND, and OR beneath IMPLIES need no parentheses, while an IMPLIES beneath any boolean operator, IMPLIES included, always gets them, since IMPLIES has the lowest precedence and is not associative. The expansion moves to eval_const_expressions, so the rest of the planner, including qual canonicalization, predicate proving for partial indexes, and outer join reduction, never sees IMPLIES and needs no changes. EXPLAIN therefore shows the expanded and simplified form, much as it does for BETWEEN. The executor and postgres_fdw's deparser should not see an unexpanded IMPLIES either, but each handles one anyway, as (NOT a OR b), rather than fail. The executor needs no new opcodes for that, so JIT is unaffected, and the deparser sends the expansion because the remote server might not know IMPLIES. --- contrib/postgres_fdw/deparse.c | 12 ++++ src/backend/executor/execExpr.c | 20 +++++- src/backend/nodes/outfuncs.c | 3 + src/backend/nodes/readfuncs.c | 2 + src/backend/optimizer/util/clauses.c | 23 ++++++- src/backend/parser/parse_expr.c | 18 ++--- src/backend/utils/adt/ruleutils.c | 22 +++++- src/include/nodes/primnodes.h | 11 ++- src/test/regress/expected/boolean.out | 97 +++++++++++++++++++++++++-- src/test/regress/sql/boolean.sql | 39 ++++++++++- 10 files changed, 220 insertions(+), 27 deletions(-) diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index ff9fe0f87e4..69a8529f583 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -3742,20 +3742,32 @@ deparseBoolExpr(BoolExpr *node, deparse_expr_cxt *context) op = "AND"; break; case OR_EXPR: op = "OR"; break; case NOT_EXPR: appendStringInfoString(buf, "(NOT "); deparseExpr(linitial(node->args), context); appendStringInfoChar(buf, ')'); return; + case IMPLIES_EXPR: + + /* + * eval_const_expressions should already have expanded this, but + * if not, send the expansion, which any remote server accepts. + */ + appendStringInfoString(buf, "((NOT "); + deparseExpr(linitial(node->args), context); + appendStringInfoString(buf, ") OR "); + deparseExpr(lsecond(node->args), context); + appendStringInfoChar(buf, ')'); + return; } appendStringInfoChar(buf, '('); first = true; foreach(lc, node->args) { if (!first) appendStringInfo(buf, " %s ", op); deparseExpr((Expr *) lfirst(lc), context); first = false; diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 82e846a1f4f..6c069e56b50 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1379,21 +1379,21 @@ ExecInitExprRec(Expr *node, ExprState *state, } case T_BoolExpr: { BoolExpr *boolexpr = (BoolExpr *) node; int nargs = list_length(boolexpr->args); List *adjust_jumps = NIL; int off; ListCell *lc; - /* allocate scratch memory used by all steps of AND/OR */ + /* allocate scratch memory used by all steps of AND/OR/IMPLIES */ if (boolexpr->boolop != NOT_EXPR) scratch.d.boolexpr.anynull = palloc_object(bool); /* * For each argument evaluate the argument itself, then * perform the bool operation's appropriate handling. * * We can evaluate each argument into our result area, since * the short-circuiting logic means we only need to remember * previous NULL values. @@ -1432,20 +1432,38 @@ ExecInitExprRec(Expr *node, ExprState *state, else if (off + 1 == nargs) scratch.opcode = EEOP_BOOL_OR_STEP_LAST; else scratch.opcode = EEOP_BOOL_OR_STEP; break; case NOT_EXPR: Assert(nargs == 1); scratch.opcode = EEOP_BOOL_NOT_STEP; break; + case IMPLIES_EXPR: + + /* + * The planner normally expands this, but in case + * it didn't, evaluate it as NOT a OR b. The NOT + * step doesn't jump, so it needs no adjusting. + */ + Assert(nargs == 2); + + if (off == 0) + { + scratch.opcode = EEOP_BOOL_NOT_STEP; + ExprEvalPushStep(state, &scratch); + scratch.opcode = EEOP_BOOL_OR_STEP_FIRST; + } + else + scratch.opcode = EEOP_BOOL_OR_STEP_LAST; + break; default: elog(ERROR, "unrecognized boolop: %d", (int) boolexpr->boolop); break; } scratch.d.boolexpr.jumpdone = -1; ExprEvalPushStep(state, &scratch); adjust_jumps = lappend_int(adjust_jumps, state->steps_len - 1); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 9a35ff53de0..75b7856725b 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -415,20 +415,23 @@ _outBoolExpr(StringInfo str, const BoolExpr *node) { case AND_EXPR: opstr = "and"; break; case OR_EXPR: opstr = "or"; break; case NOT_EXPR: opstr = "not"; break; + case IMPLIES_EXPR: + opstr = "implies"; + break; } appendStringInfoString(str, " :boolop "); outToken(str, opstr); WRITE_NODE_FIELD(args); WRITE_LOCATION_FIELD(location); } static void _outForeignKeyOptInfo(StringInfo str, const ForeignKeyOptInfo *node) diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 4cc019a012b..d06051e9851 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -288,20 +288,22 @@ _readBoolExpr(ReadNodeContext *ctx) /* do-it-yourself enum representation */ token = pg_strtok(ctx, &length); /* skip :boolop */ token = pg_strtok(ctx, &length); /* get field value */ if (length == 3 && strncmp(token, "and", 3) == 0) local_node->boolop = AND_EXPR; else if (length == 2 && strncmp(token, "or", 2) == 0) local_node->boolop = OR_EXPR; else if (length == 3 && strncmp(token, "not", 3) == 0) local_node->boolop = NOT_EXPR; + else if (length == 7 && strncmp(token, "implies", 7) == 0) + local_node->boolop = IMPLIES_EXPR; else elog(ERROR, "unrecognized boolop \"%.*s\"", length, token); READ_NODE_FIELD(args); READ_LOCATION_FIELD(location); READ_DONE(); } static A_Const * diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 3e1f210652d..f012d5b9db0 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -1135,21 +1135,22 @@ contain_nonstrict_functions_walker(Node *node, void *context) /* else fall through to check args */ } else if (IsA(node, BoolExpr)) { BoolExpr *expr = (BoolExpr *) node; switch (expr->boolop) { case AND_EXPR: case OR_EXPR: - /* AND, OR are inherently non-strict */ + case IMPLIES_EXPR: + /* AND, OR, IMPLIES are inherently non-strict */ return true; default: break; } } else if (IsA(node, SubLink)) { /* In some cases a sublink might be strict, but in general not */ return true; } @@ -3371,20 +3372,40 @@ eval_const_expressions_mutator(Node *node, Assert(list_length(expr->args) == 1); arg = eval_const_expressions_mutator(linitial(expr->args), context); /* * Use negate_clause() to see if we can simplify * away the NOT. */ return negate_clause(arg); } + case IMPLIES_EXPR: + { + Node *newexpr; + + /* + * Expand a IMPLIES b into NOT a OR b and simplify + * that, so that nothing downstream need know + * about IMPLIES. + */ + Assert(list_length(expr->args) == 2); + newexpr = (Node *) + makeBoolExpr(OR_EXPR, + list_make2(makeBoolExpr(NOT_EXPR, + list_make1(linitial(expr->args)), + expr->location), + lsecond(expr->args)), + expr->location); + return eval_const_expressions_mutator(newexpr, + context); + } default: elog(ERROR, "unrecognized boolop: %d", (int) expr->boolop); break; } break; } case T_JsonValueExpr: { JsonValueExpr *jve = (JsonValueExpr *) node; diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 3f9dc86fa00..3a83bce2691 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -1444,47 +1444,39 @@ transformBoolExpr(ParseState *pstate, BoolExpr *a) arg = transformExprRecurse(pstate, arg); arg = coerce_to_boolean(pstate, arg, opname); args = lappend(args, arg); } return (Node *) makeBoolExpr(a->boolop, args, a->location); } /* - * Transform "a IMPLIES b" into the equivalent "NOT a OR b". + * Transform "a IMPLIES b". * - * We expand this here rather than in gram.y so that a non-boolean operand is - * complained of in terms of IMPLIES, rather than in terms of the NOT or OR - * that the construct happens to be built from. + * This means the same as "NOT a OR b", but we keep it as its own BoolExpr so + * that stored expressions deparse as written. eval_const_expressions does the + * expansion, so the planner proper never sees IMPLIES. */ static Node * transformAExprImplies(ParseState *pstate, A_Expr *a) { Node *lexpr; Node *rexpr; lexpr = transformExprRecurse(pstate, a->lexpr); rexpr = transformExprRecurse(pstate, a->rexpr); lexpr = coerce_to_boolean(pstate, lexpr, "IMPLIES"); rexpr = coerce_to_boolean(pstate, rexpr, "IMPLIES"); - /* - * Each operand appears exactly once in the expansion, so unlike BETWEEN - * this does not risk evaluating anything twice. - */ - return (Node *) makeBoolExpr(OR_EXPR, - list_make2(makeBoolExpr(NOT_EXPR, - list_make1(lexpr), - exprLocation(lexpr)), - rexpr), + return (Node *) makeBoolExpr(IMPLIES_EXPR, list_make2(lexpr, rexpr), a->location); } static Node * transformFuncCall(ParseState *pstate, FuncCall *fn) { Node *last_srf = pstate->p_last_srf; List *targs; ListCell *args; diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 5e8e1683db0..b33e4cff332 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -9048,27 +9048,33 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags) { BoolExprType type; BoolExprType parentType; type = ((BoolExpr *) node)->boolop; parentType = ((BoolExpr *) parentNode)->boolop; switch (type) { case NOT_EXPR: case AND_EXPR: - if (parentType == AND_EXPR || parentType == OR_EXPR) + if (parentType == AND_EXPR || + parentType == OR_EXPR || + parentType == IMPLIES_EXPR) return true; break; case OR_EXPR: - if (parentType == OR_EXPR) + if (parentType == OR_EXPR || + parentType == IMPLIES_EXPR) return true; break; + case IMPLIES_EXPR: + /* lowest precedence, and not associative */ + break; } } return false; case T_FuncExpr: { /* special handling for casts and COERCE_SQL_SYNTAX */ CoercionForm type = ((FuncExpr *) parentNode)->funcformat; if (type == COERCE_EXPLICIT_CAST || type == COERCE_IMPLICIT_CAST || @@ -9529,20 +9535,32 @@ get_rule_expr(Node *node, deparse_context *context, case NOT_EXPR: if (!PRETTY_PAREN(context)) appendStringInfoChar(buf, '('); appendStringInfoString(buf, "NOT "); get_rule_expr_paren(first_arg, context, false, node); if (!PRETTY_PAREN(context)) appendStringInfoChar(buf, ')'); break; + case IMPLIES_EXPR: + if (!PRETTY_PAREN(context)) + appendStringInfoChar(buf, '('); + get_rule_expr_paren(first_arg, context, + false, node); + appendStringInfoString(buf, " IMPLIES "); + get_rule_expr_paren(lsecond(expr->args), context, + false, node); + if (!PRETTY_PAREN(context)) + appendStringInfoChar(buf, ')'); + break; + default: elog(ERROR, "unrecognized boolop: %d", (int) expr->boolop); } } break; case T_SubLink: get_sublink_expr((SubLink *) node, context); break; diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 2a832a27f49..f206e7ddddc 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -931,29 +931,34 @@ typedef struct ScalarArrayOpExpr Oid inputcollid pg_node_attr(query_jumble_ignore); /* the scalar and array operands */ List *args; /* token location, or -1 if unknown */ ParseLoc location; } ScalarArrayOpExpr; /* - * BoolExpr - expression node for the basic Boolean operators AND, OR, NOT + * BoolExpr - expression node for the basic Boolean operators AND, OR, NOT, + * and IMPLIES * * Notice the arguments are given as a List. For NOT, of course the list * must always have exactly one element. For AND and OR, there can be two - * or more arguments. + * or more arguments. IMPLIES is not associative and always has exactly two. + * + * IMPLIES is kept only so that stored expressions can be deparsed as the user + * wrote them; eval_const_expressions expands it to NOT a OR b, so the rest of + * the planner never sees it. */ typedef enum BoolExprType { - AND_EXPR, OR_EXPR, NOT_EXPR + AND_EXPR, OR_EXPR, NOT_EXPR, IMPLIES_EXPR } BoolExprType; typedef struct BoolExpr { pg_node_attr(custom_read_write) Expr xpr; BoolExprType boolop; List *args; /* arguments to this expression */ ParseLoc location; /* token location, or -1 if unknown */ diff --git a/src/test/regress/expected/boolean.out b/src/test/regress/expected/boolean.out index 5c092b09a5e..70c31e38244 100644 --- a/src/test/regress/expected/boolean.out +++ b/src/test/regress/expected/boolean.out @@ -674,30 +674,117 @@ SELECT isfalse IMPLIES isnul IS NULL FROM booltbl4; -- non-boolean operands are reported in terms of IMPLIES, not of its expansion SELECT 1 IMPLIES true; -- error ERROR: argument of IMPLIES must be type boolean, not type integer LINE 1: SELECT 1 IMPLIES true; ^ SELECT true IMPLIES 1; -- error ERROR: argument of IMPLIES must be type boolean, not type integer LINE 1: SELECT true IMPLIES 1; ^ --- the construct is expanded during parse analysis, so this is what is stored -CREATE VIEW boolview AS SELECT istrue IMPLIES isnul AS i FROM booltbl4; +-- stored expressions keep IMPLIES, and deparse with only the parentheses +-- that are needed +CREATE VIEW boolview AS + SELECT istrue IMPLIES isnul AS i, + NOT istrue IMPLIES isnul AS not_antecedent, + istrue OR isfalse IMPLIES isnul AND istrue AS or_and, + (istrue IMPLIES isfalse) IMPLIES isnul AS grouped_left, + istrue IMPLIES (isfalse IMPLIES isnul) AS grouped_right, + (istrue IMPLIES isfalse) OR isnul AS under_or, + NOT (istrue IMPLIES isfalse) AS under_not, + (istrue IMPLIES isfalse) IS TRUE AS under_is + FROM booltbl4; SELECT pg_get_viewdef('boolview', true); - pg_get_viewdef ----------------------------------- - SELECT NOT istrue OR isnul AS i+ + pg_get_viewdef +-------------------------------------------------------------- + SELECT istrue IMPLIES isnul AS i, + + NOT istrue IMPLIES isnul AS not_antecedent, + + istrue OR isfalse IMPLIES isnul AND istrue AS or_and, + + (istrue IMPLIES isfalse) IMPLIES isnul AS grouped_left, + + istrue IMPLIES (isfalse IMPLIES isnul) AS grouped_right,+ + (istrue IMPLIES isfalse) OR isnul AS under_or, + + NOT (istrue IMPLIES isfalse) AS under_not, + + (istrue IMPLIES isfalse) IS TRUE AS under_is + + FROM booltbl4; +(1 row) + +SELECT pg_get_viewdef('boolview', false); + pg_get_viewdef +----------------------------------------------------------------- + SELECT (istrue IMPLIES isnul) AS i, + + ((NOT istrue) IMPLIES isnul) AS not_antecedent, + + ((istrue OR isfalse) IMPLIES (isnul AND istrue)) AS or_and,+ + ((istrue IMPLIES isfalse) IMPLIES isnul) AS grouped_left, + + (istrue IMPLIES (isfalse IMPLIES isnul)) AS grouped_right, + + ((istrue IMPLIES isfalse) OR isnul) AS under_or, + + (NOT (istrue IMPLIES isfalse)) AS under_not, + + ((istrue IMPLIES isfalse) IS TRUE) AS under_is + FROM booltbl4; (1 row) +SELECT * FROM boolview; + i | not_antecedent | or_and | grouped_left | grouped_right | under_or | under_not | under_is +--------+----------------+--------+--------------+---------------+----------+-----------+---------- + (null) | t | (null) | t | t | (null) | t | f +(1 row) + DROP VIEW boolview; +CREATE TABLE implies_check (a int, b int, + CHECK (a > 0 IMPLIES b > 0)); +SELECT pg_get_constraintdef(oid) FROM pg_constraint + WHERE conrelid = 'implies_check'::regclass; + pg_get_constraintdef +----------------------------------- + CHECK (((a > 0) IMPLIES (b > 0))) +(1 row) + +INSERT INTO implies_check VALUES (1, 1), (0, 0), (NULL, 0), (1, NULL); +INSERT INTO implies_check VALUES (1, 0); -- error +ERROR: new row for relation "implies_check" violates check constraint "implies_check_check" +DETAIL: Failing row contains (1, 0). +-- the planner expands it, so EXPLAIN shows NOT a OR b, simplified +EXPLAIN (COSTS OFF, VERBOSE) +SELECT * FROM implies_check WHERE a > 0 IMPLIES b > 0; + QUERY PLAN +------------------------------------------------------------- + Seq Scan on public.implies_check + Output: a, b + Filter: ((implies_check.a <= 0) OR (implies_check.b > 0)) +(3 rows) + +EXPLAIN (COSTS OFF) +SELECT * FROM implies_check WHERE a IS NULL IMPLIES false; + QUERY PLAN +--------------------------- + Seq Scan on implies_check + Filter: (a IS NOT NULL) +(2 rows) + +-- and a partial index on IMPLIES is usable for the expanded form +CREATE INDEX implies_check_idx ON implies_check (b) + WHERE a IS NULL IMPLIES b > 0; +SELECT pg_get_indexdef('implies_check_idx'::regclass); + pg_get_indexdef +------------------------------------------------------------------------------------------------------------ + CREATE INDEX implies_check_idx ON public.implies_check USING btree (b) WHERE ((a IS NULL) IMPLIES (b > 0)) +(1 row) + +SET enable_seqscan = off; +EXPLAIN (COSTS OFF) +SELECT b FROM implies_check WHERE a IS NOT NULL OR b > 0; + QUERY PLAN +---------------------------------------------------------- + Index Only Scan using implies_check_idx on implies_check +(1 row) + +RESET enable_seqscan; +DROP TABLE implies_check; -- IMPLIES is unreserved, so it remains usable as an identifier CREATE TABLE implies (implies bool); INSERT INTO implies VALUES (false); SELECT implies IMPLIES implies FROM implies; ?column? ---------- t (1 row) DROP TABLE implies; diff --git a/src/test/regress/sql/boolean.sql b/src/test/regress/sql/boolean.sql index dfe96cb8ae5..f977cabc987 100644 --- a/src/test/regress/sql/boolean.sql +++ b/src/test/regress/sql/boolean.sql @@ -290,25 +290,60 @@ SELECT a, b, c SELECT istrue OR isfalse IMPLIES isfalse FROM booltbl4; SELECT isfalse IMPLIES isfalse AND isfalse FROM booltbl4; SELECT NOT istrue IMPLIES istrue FROM booltbl4; SELECT 1 = 1 IMPLIES 2 = 3; SELECT isfalse IMPLIES isnul IS NULL FROM booltbl4; -- non-boolean operands are reported in terms of IMPLIES, not of its expansion SELECT 1 IMPLIES true; -- error SELECT true IMPLIES 1; -- error --- the construct is expanded during parse analysis, so this is what is stored -CREATE VIEW boolview AS SELECT istrue IMPLIES isnul AS i FROM booltbl4; +-- stored expressions keep IMPLIES, and deparse with only the parentheses +-- that are needed +CREATE VIEW boolview AS + SELECT istrue IMPLIES isnul AS i, + NOT istrue IMPLIES isnul AS not_antecedent, + istrue OR isfalse IMPLIES isnul AND istrue AS or_and, + (istrue IMPLIES isfalse) IMPLIES isnul AS grouped_left, + istrue IMPLIES (isfalse IMPLIES isnul) AS grouped_right, + (istrue IMPLIES isfalse) OR isnul AS under_or, + NOT (istrue IMPLIES isfalse) AS under_not, + (istrue IMPLIES isfalse) IS TRUE AS under_is + FROM booltbl4; SELECT pg_get_viewdef('boolview', true); +SELECT pg_get_viewdef('boolview', false); +SELECT * FROM boolview; DROP VIEW boolview; +CREATE TABLE implies_check (a int, b int, + CHECK (a > 0 IMPLIES b > 0)); +SELECT pg_get_constraintdef(oid) FROM pg_constraint + WHERE conrelid = 'implies_check'::regclass; +INSERT INTO implies_check VALUES (1, 1), (0, 0), (NULL, 0), (1, NULL); +INSERT INTO implies_check VALUES (1, 0); -- error + +-- the planner expands it, so EXPLAIN shows NOT a OR b, simplified +EXPLAIN (COSTS OFF, VERBOSE) +SELECT * FROM implies_check WHERE a > 0 IMPLIES b > 0; +EXPLAIN (COSTS OFF) +SELECT * FROM implies_check WHERE a IS NULL IMPLIES false; + +-- and a partial index on IMPLIES is usable for the expanded form +CREATE INDEX implies_check_idx ON implies_check (b) + WHERE a IS NULL IMPLIES b > 0; +SELECT pg_get_indexdef('implies_check_idx'::regclass); +SET enable_seqscan = off; +EXPLAIN (COSTS OFF) +SELECT b FROM implies_check WHERE a IS NOT NULL OR b > 0; +RESET enable_seqscan; +DROP TABLE implies_check; + -- IMPLIES is unreserved, so it remains usable as an identifier CREATE TABLE implies (implies bool); INSERT INTO implies VALUES (false); SELECT implies IMPLIES implies FROM implies; DROP TABLE implies; SELECT 1 AS implies; SELECT 1 implies; -- Casts SELECT 0::boolean; -- 2.56.0