From 6649427e1f5208423cf555985957e5c102dd19b7 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Fri, 2 Oct 2026 22:32:54 +0900 Subject: [PATCH v1 2/2] Keep the parentheses of sublink test expressions in pretty mode get_sublink_expr() printed the test expression of a sublink, the left operand of IN, of an operator ANY/ALL or of a single-column row comparison, with get_rule_expr(), so an expression that needs parentheses there lost them in pretty mode. Use get_rule_expr_paren(), as for the operands of an ordinary operator. That alone would add parentheses around many test expressions that need none, since isSimpleNode() knows nothing about a sublink as the parent. Teach it the precedence that matters there, following gram.y: the test expression needs parentheses if it binds less tightly than IN or the combining operator, or as tightly where they do not associate. Postfix tests, casts and expressions that end in parentheses of their own never need them. Node types it does not know are left to the existing rules. Only pretty mode is affected; without it, every operator prints parentheses of its own. Backpatch-through: 15 --- src/backend/utils/adt/ruleutils.c | 168 +++++++++++++++++++++- src/test/regress/expected/create_view.out | 109 +++++++------- 2 files changed, 221 insertions(+), 56 deletions(-) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 5e8e1683db0..61ddd6f02ba 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -102,6 +102,18 @@ #define PRETTY_INDENT(context) ((context)->prettyFlags & PRETTYFLAG_INDENT) #define PRETTY_SCHEMA(context) ((context)->prettyFlags & PRETTYFLAG_SCHEMA) +/* + * Precedence levels for parenthesizing the test expression of a sublink, + * after the precedence declarations in gram.y, loosest first + */ +#define SUBLINK_PREC_CMP 1 /* < > = <= >= <> */ +#define SUBLINK_PREC_IN 2 /* IN */ +#define SUBLINK_PREC_OP 3 /* any other operator, and qualified ones */ +#define SUBLINK_PREC_ADD 4 /* additive operators: + - */ +#define SUBLINK_PREC_MUL 5 /* multiplicative operators: * / % */ +#define SUBLINK_PREC_EXP 6 /* ^ */ +#define SUBLINK_PREC_TOP 7 /* unary + -, tighter than any of the above */ + /* ---------- * Local data types @@ -8874,6 +8886,133 @@ get_simple_binary_op_name(OpExpr *expr) } +/* + * Binary operator names that have a precedence of their own in gram.y. Any + * other operator, including a schema-qualified one, has the precedence of Op. + */ +static const struct +{ + const char *name; + int prec; +} sublink_binop_precs[] = + +{ + {"<", SUBLINK_PREC_CMP}, + {">", SUBLINK_PREC_CMP}, + {"=", SUBLINK_PREC_CMP}, + {"<=", SUBLINK_PREC_CMP}, + {">=", SUBLINK_PREC_CMP}, + {"<>", SUBLINK_PREC_CMP}, + {"+", SUBLINK_PREC_ADD}, + {"-", SUBLINK_PREC_ADD}, + {"*", SUBLINK_PREC_MUL}, + {"/", SUBLINK_PREC_MUL}, + {"%", SUBLINK_PREC_MUL}, + {"^", SUBLINK_PREC_EXP}, +}; + +/* + * The precedence level of a binary operator, by the name that is printed + * for it. + */ +static int +sublink_binop_prec(const char *opname) +{ + for (int i = 0; i < lengthof(sublink_binop_precs); i++) + { + if (strcmp(opname, sublink_binop_precs[i].name) == 0) + return sublink_binop_precs[i].prec; + } + return SUBLINK_PREC_OP; +} + +/* + * Decide whether the test expression of a sublink can be printed without + * parentheses before IN or the combining operator. If this can tell, set + * *simple and return true; otherwise return false, leaving the decision to + * the general rules of isSimpleNode(). + */ +static bool +sublink_testexpr_is_simple(Node *node, SubLink *sublink, bool *simple) +{ + OpExpr *combop; + char *opname; + int combprec; + int prec; + + /* only get_sublink_expr() passes a sublink, for a single OpExpr */ + Assert(sublink->testexpr && IsA(sublink->testexpr, OpExpr)); + combop = (OpExpr *) sublink->testexpr; + opname = generate_operator_name(combop->opno, + exprType(linitial(combop->args)), + exprType(lsecond(combop->args))); + if (sublink->subLinkType == ANY_SUBLINK && strcmp(opname, "=") == 0) + combprec = SUBLINK_PREC_IN; /* printed as IN */ + else + combprec = sublink_binop_prec(opname); + + switch (nodeTag(node)) + { + case T_SubLink: /* own parentheses */ + case T_RowCompareExpr: /* own parentheses */ + case T_ScalarArrayOpExpr: /* x op ANY (array), ends in ")" */ + case T_BooleanTest: /* IS TRUE and the like, postfix */ + case T_JsonIsPredicate: /* IS JSON, postfix */ + case T_CollateExpr: /* COLLATE name, postfix */ + case T_RelabelType: /* ::type, implicit casts shown too */ + case T_CoerceViaIO: /* ::type */ + case T_ArrayCoerceExpr: /* ::type */ + case T_ConvertRowtypeExpr: /* ::type */ + case T_CoerceToDomain: /* ::type */ + *simple = true; + return true; + + case T_NullTest: + { + NullTest *ntest = (NullTest *) node; + + /* IS NULL is postfix, but IS DISTINCT FROM NULL is not */ + *simple = ntest->argisrow || + !type_is_rowtype(exprType((Node *) ntest->arg)); + return true; + } + + case T_OpExpr: + { + OpExpr *op = (OpExpr *) node; + + if (list_length(op->args) == 2) + prec = sublink_binop_prec(generate_operator_name(op->opno, + exprType(linitial(op->args)), + exprType(lsecond(op->args)))); + else + { + char *name; + + name = generate_operator_name(op->opno, InvalidOid, + exprType(linitial(op->args))); + /* unary + and - have their own precedence, others are Op */ + if (strcmp(name, "-") == 0 || strcmp(name, "+") == 0) + prec = SUBLINK_PREC_TOP; + else + prec = SUBLINK_PREC_OP; + } + } + break; + + default: + return false; + } + + /* + * The test expression is the left operand, so an equal level suffices for + * the left-associative levels; comparisons and IN do not associate. + */ + *simple = prec > combprec || + (prec == combprec && combprec >= SUBLINK_PREC_OP); + return true; +} + /* * isSimpleNode - check if given node is simple (doesn't need parenthesizing) * @@ -8886,6 +9025,27 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags) if (!node) return false; + /* some parent node types have rules of their own */ + if (parentNode) + { + switch (nodeTag(parentNode)) + { + case T_SubLink: + { + bool simple; + + /* the test expression of a sublink */ + if (sublink_testexpr_is_simple(node, (SubLink *) parentNode, + &simple)) + return simple; + } + break; + + default: + break; + } + } + switch (nodeTag(node)) { case T_Var: @@ -11942,7 +12102,13 @@ get_sublink_expr(SubLink *sublink, deparse_context *context) /* single combining operator */ OpExpr *opexpr = (OpExpr *) sublink->testexpr; - get_rule_expr(linitial(opexpr->args), context, true); + /* + * The test expression is the left operand of the combining + * operator, so it needs the same parenthesization as any other + * operator operand (we print IN / op ANY after it). + */ + get_rule_expr_paren(linitial(opexpr->args), context, true, + (Node *) sublink); opname = generate_operator_name(opexpr->opno, exprType(linitial(opexpr->args)), exprType(lsecond(opexpr->args))); diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out index dfc570255c0..cbe51937bfb 100644 --- a/src/test/regress/expected/create_view.out +++ b/src/test/regress/expected/create_view.out @@ -2312,11 +2312,11 @@ select pg_get_viewdef('tt28v'::regclass, true); b, + c, + d, + - (a OR b IN ( SELECT false)) AS r1, + - (a AND b IN ( SELECT false)) AS r2, + + ((a OR b) IN ( SELECT false)) AS r1, + + ((a AND b) IN ( SELECT false)) AS r2, + (c + d IN ( SELECT 3)) AS r3, + (( SELECT 1) IN ( SELECT 1)) AS r4, + - (NOT a IN ( SELECT true)) AS r5 + + ((NOT a) IN ( SELECT true)) AS r5 + FROM ( VALUES (true,false,1,2), (false,true,1,2), (false,false,3,3), (true,true,3,3)) t(a, b, c, d); (1 row) @@ -2325,10 +2325,10 @@ create view tt28v_re as :def select * from tt28v_re; a | b | c | d | r1 | r2 | r3 | r4 | r5 ---+---+---+---+----+----+----+----+---- - t | f | 1 | 2 | t | t | t | t | f - f | t | 1 | 2 | f | f | t | t | t - f | f | 3 | 3 | t | f | f | t | t - t | t | 3 | 3 | t | f | f | t | f + t | f | 1 | 2 | f | t | t | t | f + f | t | 1 | 2 | f | t | t | t | t + f | f | 3 | 3 | t | t | f | t | t + t | t | 3 | 3 | f | f | f | t | f (4 rows) create view tt28w as @@ -2350,30 +2350,30 @@ select * from tt28w; (2 rows) select pg_get_viewdef('tt28w'::regclass, true); - pg_get_viewdef ---------------------------------------------------------------- - SELECT c, + - d, + - (c < d IN ( SELECT true)) AS r1, + - (c < d <> ALL ( SELECT false)) AS r2, + - (c <= d IN ( SELECT true)) AS r3, + - (c >= d <= ALL ( SELECT true)) AS r4, + - (c + d <> ALL ( SELECT 0)) AS r5, + - (c << 1 < ALL ( SELECT 9)) AS r6, + - (c < d = ( SELECT true)) AS r7, + - (ROW(c, d) IS DISTINCT FROM NULL IN ( SELECT true)) AS r8+ + pg_get_viewdef +----------------------------------------------------------------- + SELECT c, + + d, + + ((c < d) IN ( SELECT true)) AS r1, + + ((c < d) <> ALL ( SELECT false)) AS r2, + + ((c <= d) IN ( SELECT true)) AS r3, + + ((c >= d) <= ALL ( SELECT true)) AS r4, + + (c + d <> ALL ( SELECT 0)) AS r5, + + (c << 1 < ALL ( SELECT 9)) AS r6, + + ((c < d) = ( SELECT true)) AS r7, + + ((ROW(c, d) IS DISTINCT FROM NULL) IN ( SELECT true)) AS r8+ FROM ( VALUES (1,2), (3,3)) t(c, d); (1 row) select pg_get_viewdef('tt28w'::regclass, true) as def \gset create view tt28w_re as :def -ERROR: syntax error at or near "<>" -LINE 4: (c < d <> ALL ( SELECT false)) AS r2, - ^ select * from tt28w_re; -ERROR: relation "tt28w_re" does not exist -LINE 1: select * from tt28w_re; - ^ + c | d | r1 | r2 | r3 | r4 | r5 | r6 | r7 | r8 +---+---+----+----+----+----+----+----+----+---- + 1 | 2 | t | t | t | t | t | t | t | t + 3 | 3 | f | f | t | t | t | t | f | t +(2 rows) + -- test expressions that need no parentheses: those that bind tighter than IN -- or the combining operator, left-associative ones at an equal level, postfix -- tests and casts @@ -2463,9 +2463,9 @@ select pg_get_viewdef('tt28y'::regclass, true); pg_get_viewdef ------------------------------------------------------------------------ SELECT c, + - (c + 1 * ANY ( SELECT '2'::text AS text)) AS r1, + + ((c + 1) * ANY ( SELECT '2'::text AS text)) AS r1, + (c * 2 * ANY ( SELECT '2'::text AS text)) AS r2, + - (c << 1 * ANY ( SELECT '2'::text AS text)) AS r3, + + ((c << 1) * ANY ( SELECT '2'::text AS text)) AS r3, + (- c * ANY ( SELECT '-1'::text AS text)) AS r4, + (c = ANY (ARRAY[1, 2]) * ANY ( SELECT 'true'::text AS text)) AS r5+ FROM ( VALUES (1), (3)) t(c); @@ -2473,15 +2473,13 @@ select pg_get_viewdef('tt28y'::regclass, true); select pg_get_viewdef('tt28y'::regclass, true) as def \gset create view tt28y_re as :def -ERROR: operator does not exist: integer + boolean -LINE 2: (c + 1 * ANY ( SELECT '2'::text AS text)) AS r1, - ^ -DETAIL: No operator of that name accepts the given argument types. -HINT: You might need to add explicit type casts. select * from tt28y_re; -ERROR: relation "tt28y_re" does not exist -LINE 1: select * from tt28y_re; - ^ + c | r1 | r2 | r3 | r4 | r5 +---+----+----+----+----+---- + 1 | t | t | t | t | t + 3 | f | f | f | f | f +(2 rows) + create function tt28g(float8, text) returns bool language sql immutable as 'select $1::text = $2'; create operator + (leftarg = float8, rightarg = text, function = tt28g); @@ -2506,31 +2504,29 @@ select * from tt28z; (2 rows) select pg_get_viewdef('tt28z'::regclass, true); - pg_get_viewdef ------------------------------------------------------------------------------------------- - SELECT c, + - (c::double precision * 2::double precision + ANY ( SELECT '2'::text AS text)) AS r1,+ - (c::double precision - 1::double precision + ANY ( SELECT '0'::text AS text)) AS r2,+ - (|/ c::double precision + ANY ( SELECT '1'::text AS text)) AS r3, + - (c::double precision ^ 2::double precision * ANY ( SELECT '1'::text AS text)) AS r4,+ - (|/ c::double precision * ANY ( SELECT '1'::text AS text)) AS r5, + - (c::double precision * 2::double precision ^ ANY ( SELECT '2'::text AS text)) AS r6,+ - (c::double precision ^ 2::double precision ^ ANY ( SELECT '1'::text AS text)) AS r7,+ - (- c::double precision ^ ANY ( SELECT '-1'::text AS text)) AS r8 + + pg_get_viewdef +-------------------------------------------------------------------------------------------- + SELECT c, + + (c::double precision * 2::double precision + ANY ( SELECT '2'::text AS text)) AS r1, + + (c::double precision - 1::double precision + ANY ( SELECT '0'::text AS text)) AS r2, + + ((|/ c::double precision) + ANY ( SELECT '1'::text AS text)) AS r3, + + (c::double precision ^ 2::double precision * ANY ( SELECT '1'::text AS text)) AS r4, + + ((|/ c::double precision) * ANY ( SELECT '1'::text AS text)) AS r5, + + ((c::double precision * 2::double precision) ^ ANY ( SELECT '2'::text AS text)) AS r6,+ + (c::double precision ^ 2::double precision ^ ANY ( SELECT '1'::text AS text)) AS r7, + + (- c::double precision ^ ANY ( SELECT '-1'::text AS text)) AS r8 + FROM ( VALUES (1), (3)) t(c); (1 row) select pg_get_viewdef('tt28z'::regclass, true) as def \gset create view tt28z_re as :def -ERROR: operator does not exist: |/ boolean -LINE 4: (|/ c::double precision + ANY ( SELECT '1'::text AS text... - ^ -DETAIL: No operator of that name accepts the given argument type. -HINT: You might need to add an explicit type cast. select * from tt28z_re; -ERROR: relation "tt28z_re" does not exist -LINE 1: select * from tt28z_re; - ^ + c | r1 | r2 | r3 | r4 | r5 | r6 | r7 | r8 +---+----+----+----+----+----+----+----+---- + 1 | t | t | t | t | t | t | t | t + 3 | f | f | f | f | f | f | f | f +(2 rows) + -- clean up all the random objects we made above DROP SCHEMA temp_view_test CASCADE; NOTICE: drop cascades to 27 other objects @@ -2562,7 +2558,7 @@ drop cascades to view aliased_view_2 drop cascades to view aliased_view_3 drop cascades to view aliased_view_4 DROP SCHEMA testviewschm2 CASCADE; -NOTICE: drop cascades to 95 other objects +NOTICE: drop cascades to 98 other objects DETAIL: drop cascades to table t1 drop cascades to view temporal1 drop cascades to view temporal2 @@ -2646,6 +2642,7 @@ drop cascades to view tt27v drop cascades to view tt28v drop cascades to view tt28v_re drop cascades to view tt28w +drop cascades to view tt28w_re drop cascades to view tt28x drop cascades to view tt28x_re drop cascades to function tt28f(integer,text) @@ -2653,8 +2650,10 @@ drop cascades to operator *(integer,text) drop cascades to function tt28h(boolean,text) drop cascades to operator *(boolean,text) drop cascades to view tt28y +drop cascades to view tt28y_re drop cascades to function tt28g(double precision,text) drop cascades to operator +(double precision,text) drop cascades to operator *(double precision,text) drop cascades to operator ^(double precision,text) drop cascades to view tt28z +drop cascades to view tt28z_re -- 2.54.0 (Apple Git-157)