From b104bcb408fac73b93dc5174914c906581f714df Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 20 Oct 2026 14:29:02 -0400 Subject: [PATCH] In deparsing, parenthesize IS DOCUMENT where it is an operand. isSimpleNode() and related code treated every XmlExpr as self-delimiting. Most of them are, because they look like function calls with some extra decoration. But "x IS DOCUMENT" is not: it is a postfix test, like IS NULL. Printed as the operand of another operator, or as the operand of a postfix test, it was therefore not parenthesized and could not be parsed back, or was parsed as something unintended. Under-parenthesization occurred even in non-pretty-print cases, so that this could break dump/restore workflows. To fix, treat IS DOCUMENT as neither simple nor function-like, and give it the same treatment as other postfix tests. Also pass the single argument of IS DOCUMENT, not the argument list, to get_rule_expr_paren(). This avoids unnecessary parenthesization of the argument. Author: Henson Choi Reviewed-by: Prashant Chinnam Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAAAe_zDuPSkc8ihCctV67Lxbqock+S4wqCHHo8xxZ1t4teXo0g@mail.gmail.com Backpatch-through: 14 --- src/backend/utils/adt/ruleutils.c | 33 +++++++++++++++++++---- src/test/regress/expected/create_view.out | 21 ++++++++++++++- src/test/regress/sql/create_view.sql | 10 +++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 8a9beed7854..a52e8235dc5 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -8392,7 +8392,6 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags) case T_CoalesceExpr: case T_MinMaxExpr: case T_SQLValueFunction: - case T_XmlExpr: case T_NextValueExpr: case T_NullIfExpr: case T_Aggref: @@ -8488,6 +8487,17 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags) } /* FALLTHROUGH */ + case T_XmlExpr: + + /* + * Function-like (name(..)), except "x IS DOCUMENT", which is a + * postfix test like IS NULL. (The IsA check is needed because + * T_OpExpr falls through to here.) + */ + if (IsA(node, XmlExpr) && ((XmlExpr *) node)->op != IS_DOCUMENT) + return true; + /* FALLTHROUGH */ + case T_SubLink: case T_NullTest: case T_BooleanTest: @@ -8511,13 +8521,14 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags) case T_RowExpr: /* other separators */ case T_CoalesceExpr: /* own parentheses */ case T_MinMaxExpr: /* own parentheses */ - case T_XmlExpr: /* own parentheses */ case T_NullIfExpr: /* other separators */ case T_Aggref: /* own parentheses */ case T_GroupingFunc: /* own parentheses */ case T_WindowFunc: /* own parentheses */ case T_CaseExpr: /* other separators */ return true; + case T_XmlExpr: /* own parentheses, except IS DOCUMENT */ + return ((XmlExpr *) parentNode)->op != IS_DOCUMENT; default: return false; } @@ -8563,13 +8574,14 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags) case T_RowExpr: /* other separators */ case T_CoalesceExpr: /* own parentheses */ case T_MinMaxExpr: /* own parentheses */ - case T_XmlExpr: /* own parentheses */ case T_NullIfExpr: /* other separators */ case T_Aggref: /* own parentheses */ case T_GroupingFunc: /* own parentheses */ case T_WindowFunc: /* own parentheses */ case T_CaseExpr: /* other separators */ return true; + case T_XmlExpr: /* own parentheses, except IS DOCUMENT */ + return ((XmlExpr *) parentNode)->op != IS_DOCUMENT; default: return false; } @@ -9466,6 +9478,9 @@ get_rule_expr(Node *node, deparse_context *context, appendStringInfoString(buf, "XMLSERIALIZE("); break; case IS_DOCUMENT: + /* postfix test, like IS NULL: not self-delimiting */ + if (!PRETTY_PAREN(context)) + appendStringInfoChar(buf, '('); break; } if (xexpr->op == IS_XMLPARSE || xexpr->op == IS_XMLSERIALIZE) @@ -9573,7 +9588,9 @@ get_rule_expr(Node *node, deparse_context *context, } break; case IS_DOCUMENT: - get_rule_expr_paren((Node *) xexpr->args, context, false, node); + Assert(list_length(xexpr->args) == 1); + get_rule_expr_paren((Node *) linitial(xexpr->args), + context, false, node); break; } } @@ -9582,7 +9599,11 @@ get_rule_expr(Node *node, deparse_context *context, format_type_with_typemod(xexpr->type, xexpr->typmod)); if (xexpr->op == IS_DOCUMENT) + { appendStringInfoString(buf, " IS DOCUMENT"); + if (!PRETTY_PAREN(context)) + appendStringInfoChar(buf, ')'); + } else appendStringInfoChar(buf, ')'); } @@ -9960,9 +9981,11 @@ looks_like_function(Node *node) case T_CoalesceExpr: case T_MinMaxExpr: case T_SQLValueFunction: - case T_XmlExpr: /* these are all accepted by func_expr_common_subexpr */ return true; + case T_XmlExpr: + /* all but IS DOCUMENT, which is a postfix test */ + return ((XmlExpr *) node)->op != IS_DOCUMENT; default: break; } diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out index 8ec3230c582..6d216019350 100644 --- a/src/test/regress/expected/create_view.out +++ b/src/test/regress/expected/create_view.out @@ -2151,6 +2151,23 @@ CREATE RULE "_RETURN" AS ON SELECT TO tt28 DO INSTEAD SELECT * FROM tt26; CREATE RULE "_RETURN" AS ON SELECT TO tt28 DO INSTEAD SELECT * FROM tt26; ERROR: "tt28" is already a view ROLLBACK; +-- xml IS DOCUMENT needs to be treated specially +create operator || + (function = xmlconcat2, leftarg=xml, rightarg=xml); +create view tt261v as +select x is document as c1, + (x || x) is document as c2, + f = (x is document) as c3 +from (values(false,null::xml)) v(f,x); +select pg_get_viewdef('tt261v', true); + pg_get_viewdef +---------------------------------------------- + SELECT v.x IS DOCUMENT AS c1, + + (v.x || v.x) IS DOCUMENT AS c2, + + v.f = (v.x IS DOCUMENT) AS c3 + + FROM ( VALUES (false,NULL::xml)) v(f, x); +(1 row) + -- test restriction on non-system view expansion. create table tt27v_tbl (a int); create view tt27v as select a from tt27v_tbl; @@ -2197,7 +2214,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 80 other objects +NOTICE: drop cascades to 82 other objects DETAIL: drop cascades to table t1 drop cascades to view temporal1 drop cascades to view temporal2 @@ -2276,5 +2293,7 @@ drop cascades to view tt24v drop cascades to view tt25v drop cascades to view tt26v drop cascades to table tt26 +drop cascades to operator ||(xml,xml) +drop cascades to view tt261v drop cascades to table tt27v_tbl drop cascades to view tt27v diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql index e8bba27e2c1..5f941b10e79 100644 --- a/src/test/regress/sql/create_view.sql +++ b/src/test/regress/sql/create_view.sql @@ -806,6 +806,16 @@ CREATE RULE "_RETURN" AS ON SELECT TO tt28 DO INSTEAD SELECT * FROM tt26; CREATE RULE "_RETURN" AS ON SELECT TO tt28 DO INSTEAD SELECT * FROM tt26; ROLLBACK; +-- xml IS DOCUMENT needs to be treated specially +create operator || + (function = xmlconcat2, leftarg=xml, rightarg=xml); +create view tt261v as +select x is document as c1, + (x || x) is document as c2, + f = (x is document) as c3 +from (values(false,null::xml)) v(f,x); +select pg_get_viewdef('tt261v', true); + -- test restriction on non-system view expansion. create table tt27v_tbl (a int); create view tt27v as select a from tt27v_tbl;