Skip to content

Commit 26d8678

Browse files
tglsfdctglsfdc
committed
Fix dumping of FUNCTION RTEs that contain non-function-call expressions.
The grammar will only accept something syntactically similar to a function call in a function-in-FROM expression. However, there are various ways to input something that ruleutils.c won't deparse that way, potentially leading to a view or rule that fails dump/reload. Fix by inserting a dummy CAST around anything that isn't going to deparse as a function (which is one of the ways to get something like that in there in the first place). In HEAD, also make use of the infrastructure added by this to avoid emitting unnecessary parentheses in CREATE INDEX deparsing. I did not change that in back branches, thinking that people might find it to be unexpected/unnecessary behavioral change. In HEAD, also fix incorrect logic for when to add extra parens to partition key expressions. Somebody apparently thought they could get away with simpler logic than pg_get_indexdef_worker has, but they were wrong --- a counterexample is PARTITION BY LIST ((a[1])). Ignoring the prettyprint flag for partition expressions isn't exactly a nice solution anyway. This has been broken all along, so back-patch to all supported branches. Discussion: https://postgr.es/m/10477.1499970459@sss.pgh.pa.us
1 parent a751714 commit 26d8678

File tree

3 files changed

+96
-2
lines changed

3 files changed

+96
-2
lines changed

‎src/backend/utils/adt/ruleutils.c

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,9 @@ static void get_rule_expr(Node *node, deparse_context *context,
391391
bool showimplicit);
392392
static void get_rule_expr_toplevel(Node *node, deparse_context *context,
393393
bool showimplicit);
394+
static void get_rule_expr_funccall(Node *node, deparse_context *context,
395+
bool showimplicit);
396+
static bool looks_like_function(Node *node);
394397
static void get_oper_expr(OpExpr *expr, deparse_context *context);
395398
static void get_func_expr(FuncExpr *expr, deparse_context *context,
396399
bool showimplicit);
@@ -7706,6 +7709,63 @@ get_rule_expr_toplevel(Node *node, deparse_context *context,
77067709
get_rule_expr(node, context, showimplicit);
77077710
}
77087711

7712+
/*
7713+
* get_rule_expr_funccall - Parse back a function-call expression
7714+
*
7715+
* Same as get_rule_expr(), except that we guarantee that the output will
7716+
* look like a function call, or like one of the things the grammar treats as
7717+
* equivalent to a function call (see the func_expr_windowless production).
7718+
* This is needed in places where the grammar uses func_expr_windowless and
7719+
* you can't substitute a parenthesized a_expr. If what we have isn't going
7720+
* to look like a function call, wrap it in a dummy CAST() expression, which
7721+
* will satisfy the grammar --- and, indeed, is likely what the user wrote to
7722+
* produce such a thing.
7723+
*/
7724+
static void
7725+
get_rule_expr_funccall(Node *node, deparse_context *context,
7726+
bool showimplicit)
7727+
{
7728+
if (looks_like_function(node))
7729+
get_rule_expr(node, context, showimplicit);
7730+
else
7731+
{
7732+
StringInfo buf = context->buf;
7733+
7734+
appendStringInfoString(buf, "CAST(");
7735+
/* no point in showing any top-level implicit cast */
7736+
get_rule_expr(node, context, false);
7737+
appendStringInfo(buf, " AS %s)",
7738+
format_type_with_typemod(exprType(node),
7739+
exprTypmod(node)));
7740+
}
7741+
}
7742+
7743+
/*
7744+
* Helper function to identify node types that satisfy func_expr_windowless.
7745+
* If in doubt, "false" is always a safe answer.
7746+
*/
7747+
static bool
7748+
looks_like_function(Node *node)
7749+
{
7750+
if (node == NULL)
7751+
return false; /* probably shouldn't happen */
7752+
switch (nodeTag(node))
7753+
{
7754+
case T_FuncExpr:
7755+
/* OK, unless it's going to deparse as a cast */
7756+
return (((FuncExpr *) node)->funcformat == COERCE_EXPLICIT_CALL);
7757+
case T_NullIfExpr:
7758+
case T_CoalesceExpr:
7759+
case T_MinMaxExpr:
7760+
case T_XmlExpr:
7761+
/* these are all accepted by func_expr_common_subexpr */
7762+
return true;
7763+
default:
7764+
break;
7765+
}
7766+
return false;
7767+
}
7768+
77097769

77107770
/*
77117771
* get_oper_expr - Parse back an OpExpr node
@@ -8507,7 +8567,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
85078567
if (list_length(rte->functions) == 1 &&
85088568
(rtfunc1->funccolnames == NIL || !rte->funcordinality))
85098569
{
8510-
get_rule_expr(rtfunc1->funcexpr, context, true);
8570+
get_rule_expr_funccall(rtfunc1->funcexpr, context, true);
85118571
/* we'll print the coldeflist below, if it has one */
85128572
}
85138573
else
@@ -8570,7 +8630,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
85708630

85718631
if (funcno > 0)
85728632
appendStringInfoString(buf, ", ");
8573-
get_rule_expr(rtfunc->funcexpr, context, true);
8633+
get_rule_expr_funccall(rtfunc->funcexpr, context, true);
85748634
if (rtfunc->funccolnames != NIL)
85758635
{
85768636
/* Reconstruct the column definition list */

‎src/test/regress/expected/create_view.out

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,6 +1567,29 @@ select pg_get_viewdef('tt19v', true);
15671567
'foo'::text = ANY ((( SELECT ARRAY['abc'::text, 'def'::text, 'foo'::text] AS "array"))::text[]) AS c2;
15681568
(1 row)
15691569

1570+
-- check display of assorted RTE_FUNCTION expressions
1571+
create view tt20v as
1572+
select * from
1573+
coalesce(1,2) as c,
1574+
collation for ('x'::text) col,
1575+
current_date as d,
1576+
cast(1+2 as int4) as i4,
1577+
cast(1+2 as int8) as i8;
1578+
select pg_get_viewdef('tt20v', true);
1579+
pg_get_viewdef
1580+
---------------------------------------------
1581+
SELECT c.c, +
1582+
col.col, +
1583+
d.d, +
1584+
i4.i4, +
1585+
i8.i8 +
1586+
FROM COALESCE(1, 2) c(c), +
1587+
pg_collation_for('x'::text) col(col), +
1588+
CAST('now'::text::date AS date) d(d), +
1589+
CAST(1 + 2 AS integer) i4(i4), +
1590+
CAST((1 + 2)::bigint AS bigint) i8(i8);
1591+
(1 row)
1592+
15701593
-- clean up all the random objects we made above
15711594
set client_min_messages = warning;
15721595
DROP SCHEMA temp_view_test CASCADE;

‎src/test/regress/sql/create_view.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,17 @@ select 'foo'::text = any(array['abc','def','foo']::text[]) c1,
520520
'foo'::text = any((select array['abc','def','foo']::text[])::text[]) c2;
521521
select pg_get_viewdef('tt19v', true);
522522

523+
-- check display of assorted RTE_FUNCTION expressions
524+
525+
create view tt20v as
526+
select * from
527+
coalesce(1,2) as c,
528+
collation for ('x'::text) col,
529+
current_date as d,
530+
cast(1+2 as int4) as i4,
531+
cast(1+2 as int8) as i8;
532+
select pg_get_viewdef('tt20v', true);
533+
523534
-- clean up all the random objects we made above
524535
set client_min_messages = warning;
525536
DROP SCHEMA temp_view_test CASCADE;

0 commit comments

Comments
 (0)