From 40c24bfef92530bd846e111c1742c2a54441c62c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 4 Nov 2020 12:34:50 -0500 Subject: [PATCH] Improve our ability to regurgitate SQL-syntax function calls. The SQL spec calls out nonstandard syntax for certain function calls, for example substring() with numeric position info is supposed to be spelled "SUBSTRING(string FROM start FOR count)". We accept many of these things, but up to now would not print them in the same format, instead simplifying down to "substring"(string, start, count). That's long annoyed me because it creates an interoperability problem: we're gratuitously injecting Postgres-specific syntax into what might otherwise be a perfectly spec-compliant view definition. However, the real reason for addressing it right now is to support a planned change in the semantics of EXTRACT() a/k/a date_part(). When we switch that to returning numeric, we'll have the parser translate EXTRACT() to some new function name (might as well be "extract" if you ask me) and then teach ruleutils.c to reverse-list that per SQL spec. In this way existing calls to date_part() will continue to have the old semantics. To implement this, invent a new CoercionForm value COERCE_SQL_SYNTAX, and make the parser insert that rather than COERCE_EXPLICIT_CALL when the input has SQL-spec decoration. (But if the input has the form of a plain function call, continue to mark it COERCE_EXPLICIT_CALL, even if it's calling one of these functions.) Then ruleutils.c recognizes COERCE_SQL_SYNTAX as a cue to emit SQL call syntax. It can know which decoration to emit using hard-wired knowledge about the functions that could be called this way. (While this solution isn't extensible without manual additions, neither is the grammar, so this doesn't seem unmaintainable.) Notice that this solution will reverse-list a function call with SQL decoration only if it was entered that way; so dump-and-reload will not by itself produce any changes in the appearance of views. This requires adding a CoercionForm field to struct FuncCall. (I couldn't resist the temptation to rearrange that struct's field order a tad while I was at it.) FuncCall doesn't appear in stored rules, so that change isn't a reason for a catversion bump, but I did one anyway because the new enum value for CoercionForm fields could confuse old backend code. Possible future work: * Perhaps CoercionForm should now be renamed to DisplayForm, or something like that, to reflect its more general meaning. This'd require touching a couple hundred places, so it's not clear it's worth the code churn. * The SQLValueFunction node type, which was invented partly for the same goal of improving SQL-compatibility of view output, could perhaps be replaced with regular function calls marked with COERCE_SQL_SYNTAX. It's unclear if this would be a net code savings, however. Discussion: https://postgr.es/m/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu --- src/backend/nodes/copyfuncs.c | 3 +- src/backend/nodes/equalfuncs.c | 3 +- src/backend/nodes/makefuncs.c | 5 +- src/backend/nodes/outfuncs.c | 3 +- src/backend/parser/gram.y | 194 +++++++++++---- src/backend/parser/parse_clause.c | 4 +- src/backend/parser/parse_func.c | 6 +- src/backend/parser/parse_utilcmd.c | 1 + src/backend/utils/adt/ruleutils.c | 232 +++++++++++++++++- src/include/catalog/catversion.h | 2 +- src/include/nodes/makefuncs.h | 3 +- src/include/nodes/parsenodes.h | 5 +- src/include/nodes/primnodes.h | 8 +- .../modules/test_rls_hooks/test_rls_hooks.c | 10 +- src/test/regress/expected/create_view.out | 44 +++- src/test/regress/expected/timestamptz.out | 6 +- src/test/regress/sql/create_view.sql | 22 ++ 17 files changed, 480 insertions(+), 71 deletions(-) diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 530aac68a7..3031c52991 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -2682,11 +2682,12 @@ _copyFuncCall(const FuncCall *from) COPY_NODE_FIELD(args); COPY_NODE_FIELD(agg_order); COPY_NODE_FIELD(agg_filter); + COPY_NODE_FIELD(over); COPY_SCALAR_FIELD(agg_within_group); COPY_SCALAR_FIELD(agg_star); COPY_SCALAR_FIELD(agg_distinct); COPY_SCALAR_FIELD(func_variadic); - COPY_NODE_FIELD(over); + COPY_SCALAR_FIELD(funcformat); COPY_LOCATION_FIELD(location); return newnode; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 0cf90ef33c..9aa853748d 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2369,11 +2369,12 @@ _equalFuncCall(const FuncCall *a, const FuncCall *b) COMPARE_NODE_FIELD(args); COMPARE_NODE_FIELD(agg_order); COMPARE_NODE_FIELD(agg_filter); + COMPARE_NODE_FIELD(over); COMPARE_SCALAR_FIELD(agg_within_group); COMPARE_SCALAR_FIELD(agg_star); COMPARE_SCALAR_FIELD(agg_distinct); COMPARE_SCALAR_FIELD(func_variadic); - COMPARE_NODE_FIELD(over); + COMPARE_SCALAR_FIELD(funcformat); COMPARE_LOCATION_FIELD(location); return true; diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c index 49de285f01..ee033ae779 100644 --- a/src/backend/nodes/makefuncs.c +++ b/src/backend/nodes/makefuncs.c @@ -582,7 +582,7 @@ makeDefElemExtended(char *nameSpace, char *name, Node *arg, * supply. Any non-default parameters have to be inserted by the caller. */ FuncCall * -makeFuncCall(List *name, List *args, int location) +makeFuncCall(List *name, List *args, CoercionForm funcformat, int location) { FuncCall *n = makeNode(FuncCall); @@ -590,11 +590,12 @@ makeFuncCall(List *name, List *args, int location) n->args = args; n->agg_order = NIL; n->agg_filter = NULL; + n->over = NULL; n->agg_within_group = false; n->agg_star = false; n->agg_distinct = false; n->func_variadic = false; - n->over = NULL; + n->funcformat = funcformat; n->location = location; return n; } diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 7e324c12e2..4504b1503b 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2765,11 +2765,12 @@ _outFuncCall(StringInfo str, const FuncCall *node) WRITE_NODE_FIELD(args); WRITE_NODE_FIELD(agg_order); WRITE_NODE_FIELD(agg_filter); + WRITE_NODE_FIELD(over); WRITE_BOOL_FIELD(agg_within_group); WRITE_BOOL_FIELD(agg_star); WRITE_BOOL_FIELD(agg_distinct); WRITE_BOOL_FIELD(func_variadic); - WRITE_NODE_FIELD(over); + WRITE_ENUM_FIELD(funcformat, CoercionForm); WRITE_LOCATION_FIELD(location); } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 357ab93fb6..95e256883b 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -490,7 +490,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type rowsfrom_item rowsfrom_list opt_col_def_list %type opt_ordinality %type ExclusionConstraintList ExclusionConstraintElem -%type func_arg_list +%type func_arg_list func_arg_list_opt %type func_arg_expr %type row explicit_row implicit_row type_list array_expr_list %type case_expr case_arg when_clause case_default @@ -12969,6 +12969,7 @@ a_expr: c_expr { $$ = $1; } { $$ = (Node *) makeFuncCall(SystemFuncName("timezone"), list_make2($5, $1), + COERCE_SQL_SYNTAX, @2); } /* @@ -13032,6 +13033,7 @@ a_expr: c_expr { $$ = $1; } { FuncCall *n = makeFuncCall(SystemFuncName("like_escape"), list_make2($3, $5), + COERCE_EXPLICIT_CALL, @2); $$ = (Node *) makeSimpleA_Expr(AEXPR_LIKE, "~~", $1, (Node *) n, @2); @@ -13045,6 +13047,7 @@ a_expr: c_expr { $$ = $1; } { FuncCall *n = makeFuncCall(SystemFuncName("like_escape"), list_make2($4, $6), + COERCE_EXPLICIT_CALL, @2); $$ = (Node *) makeSimpleA_Expr(AEXPR_LIKE, "!~~", $1, (Node *) n, @2); @@ -13058,6 +13061,7 @@ a_expr: c_expr { $$ = $1; } { FuncCall *n = makeFuncCall(SystemFuncName("like_escape"), list_make2($3, $5), + COERCE_EXPLICIT_CALL, @2); $$ = (Node *) makeSimpleA_Expr(AEXPR_ILIKE, "~~*", $1, (Node *) n, @2); @@ -13071,6 +13075,7 @@ a_expr: c_expr { $$ = $1; } { FuncCall *n = makeFuncCall(SystemFuncName("like_escape"), list_make2($4, $6), + COERCE_EXPLICIT_CALL, @2); $$ = (Node *) makeSimpleA_Expr(AEXPR_ILIKE, "!~~*", $1, (Node *) n, @2); @@ -13080,6 +13085,7 @@ a_expr: c_expr { $$ = $1; } { FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"), list_make1($4), + COERCE_EXPLICIT_CALL, @2); $$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "~", $1, (Node *) n, @2); @@ -13088,6 +13094,7 @@ a_expr: c_expr { $$ = $1; } { FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"), list_make2($4, $6), + COERCE_EXPLICIT_CALL, @2); $$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "~", $1, (Node *) n, @2); @@ -13096,6 +13103,7 @@ a_expr: c_expr { $$ = $1; } { FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"), list_make1($5), + COERCE_EXPLICIT_CALL, @2); $$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "!~", $1, (Node *) n, @2); @@ -13104,6 +13112,7 @@ a_expr: c_expr { $$ = $1; } { FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"), list_make2($5, $7), + COERCE_EXPLICIT_CALL, @2); $$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "!~", $1, (Node *) n, @2); @@ -13164,6 +13173,7 @@ a_expr: c_expr { $$ = $1; } parser_errposition(@3))); $$ = (Node *) makeFuncCall(SystemFuncName("overlaps"), list_concat($1, $3), + COERCE_SQL_SYNTAX, @2); } | a_expr IS TRUE_P %prec IS @@ -13351,19 +13361,33 @@ a_expr: c_expr { $$ = $1; } } | a_expr IS NORMALIZED %prec IS { - $$ = (Node *) makeFuncCall(SystemFuncName("is_normalized"), list_make1($1), @2); + $$ = (Node *) makeFuncCall(SystemFuncName("is_normalized"), + list_make1($1), + COERCE_SQL_SYNTAX, + @2); } | a_expr IS unicode_normal_form NORMALIZED %prec IS { - $$ = (Node *) makeFuncCall(SystemFuncName("is_normalized"), list_make2($1, makeStringConst($3, @3)), @2); + $$ = (Node *) makeFuncCall(SystemFuncName("is_normalized"), + list_make2($1, makeStringConst($3, @3)), + COERCE_SQL_SYNTAX, + @2); } | a_expr IS NOT NORMALIZED %prec IS { - $$ = makeNotExpr((Node *) makeFuncCall(SystemFuncName("is_normalized"), list_make1($1), @2), @2); + $$ = makeNotExpr((Node *) makeFuncCall(SystemFuncName("is_normalized"), + list_make1($1), + COERCE_SQL_SYNTAX, + @2), + @2); } | a_expr IS NOT unicode_normal_form NORMALIZED %prec IS { - $$ = makeNotExpr((Node *) makeFuncCall(SystemFuncName("is_normalized"), list_make2($1, makeStringConst($4, @4)), @2), @2); + $$ = makeNotExpr((Node *) makeFuncCall(SystemFuncName("is_normalized"), + list_make2($1, makeStringConst($4, @4)), + COERCE_SQL_SYNTAX, + @2), + @2); } | DEFAULT { @@ -13613,31 +13637,41 @@ c_expr: columnref { $$ = $1; } func_application: func_name '(' ')' { - $$ = (Node *) makeFuncCall($1, NIL, @1); + $$ = (Node *) makeFuncCall($1, NIL, + COERCE_EXPLICIT_CALL, + @1); } | func_name '(' func_arg_list opt_sort_clause ')' { - FuncCall *n = makeFuncCall($1, $3, @1); + FuncCall *n = makeFuncCall($1, $3, + COERCE_EXPLICIT_CALL, + @1); n->agg_order = $4; $$ = (Node *)n; } | func_name '(' VARIADIC func_arg_expr opt_sort_clause ')' { - FuncCall *n = makeFuncCall($1, list_make1($4), @1); + FuncCall *n = makeFuncCall($1, list_make1($4), + COERCE_EXPLICIT_CALL, + @1); n->func_variadic = true; n->agg_order = $5; $$ = (Node *)n; } | func_name '(' func_arg_list ',' VARIADIC func_arg_expr opt_sort_clause ')' { - FuncCall *n = makeFuncCall($1, lappend($3, $6), @1); + FuncCall *n = makeFuncCall($1, lappend($3, $6), + COERCE_EXPLICIT_CALL, + @1); n->func_variadic = true; n->agg_order = $7; $$ = (Node *)n; } | func_name '(' ALL func_arg_list opt_sort_clause ')' { - FuncCall *n = makeFuncCall($1, $4, @1); + FuncCall *n = makeFuncCall($1, $4, + COERCE_EXPLICIT_CALL, + @1); n->agg_order = $5; /* Ideally we'd mark the FuncCall node to indicate * "must be an aggregate", but there's no provision @@ -13647,7 +13681,9 @@ func_application: func_name '(' ')' } | func_name '(' DISTINCT func_arg_list opt_sort_clause ')' { - FuncCall *n = makeFuncCall($1, $4, @1); + FuncCall *n = makeFuncCall($1, $4, + COERCE_EXPLICIT_CALL, + @1); n->agg_order = $5; n->agg_distinct = true; $$ = (Node *)n; @@ -13664,7 +13700,9 @@ func_application: func_name '(' ')' * so that later processing can detect what the argument * really was. */ - FuncCall *n = makeFuncCall($1, NIL, @1); + FuncCall *n = makeFuncCall($1, NIL, + COERCE_EXPLICIT_CALL, + @1); n->agg_star = true; $$ = (Node *)n; } @@ -13738,6 +13776,7 @@ func_expr_common_subexpr: { $$ = (Node *) makeFuncCall(SystemFuncName("pg_collation_for"), list_make1($4), + COERCE_SQL_SYNTAX, @1); } | CURRENT_DATE @@ -13804,31 +13843,77 @@ func_expr_common_subexpr: { $$ = makeTypeCast($3, $5, @1); } | EXTRACT '(' extract_list ')' { - $$ = (Node *) makeFuncCall(SystemFuncName("date_part"), $3, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("date_part"), + $3, + COERCE_SQL_SYNTAX, + @1); } | NORMALIZE '(' a_expr ')' { - $$ = (Node *) makeFuncCall(SystemFuncName("normalize"), list_make1($3), @1); + $$ = (Node *) makeFuncCall(SystemFuncName("normalize"), + list_make1($3), + COERCE_SQL_SYNTAX, + @1); } | NORMALIZE '(' a_expr ',' unicode_normal_form ')' { - $$ = (Node *) makeFuncCall(SystemFuncName("normalize"), list_make2($3, makeStringConst($5, @5)), @1); + $$ = (Node *) makeFuncCall(SystemFuncName("normalize"), + list_make2($3, makeStringConst($5, @5)), + COERCE_SQL_SYNTAX, + @1); } | OVERLAY '(' overlay_list ')' { - $$ = (Node *) makeFuncCall(SystemFuncName("overlay"), $3, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("overlay"), + $3, + COERCE_SQL_SYNTAX, + @1); + } + | OVERLAY '(' func_arg_list_opt ')' + { + /* + * allow functions named overlay() to be called without + * special syntax + */ + $$ = (Node *) makeFuncCall(list_make1(makeString("overlay")), + $3, + COERCE_EXPLICIT_CALL, + @1); } | POSITION '(' position_list ')' { - /* position(A in B) is converted to position(B, A) */ - $$ = (Node *) makeFuncCall(SystemFuncName("position"), $3, @1); + /* + * position(A in B) is converted to position(B, A) + * + * We deliberately don't offer a "plain syntax" option + * for position(), because the reversal of the arguments + * creates too much risk of confusion. + */ + $$ = (Node *) makeFuncCall(SystemFuncName("position"), + $3, + COERCE_SQL_SYNTAX, + @1); } | SUBSTRING '(' substr_list ')' { /* substring(A from B for C) is converted to * substring(A, B, C) - thomas 2000-11-28 */ - $$ = (Node *) makeFuncCall(SystemFuncName("substring"), $3, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("substring"), + $3, + COERCE_SQL_SYNTAX, + @1); + } + | SUBSTRING '(' func_arg_list_opt ')' + { + /* + * allow functions named substring() to be called without + * special syntax + */ + $$ = (Node *) makeFuncCall(list_make1(makeString("substring")), + $3, + COERCE_EXPLICIT_CALL, + @1); } | TREAT '(' a_expr AS Typename ')' { @@ -13841,28 +13926,41 @@ func_expr_common_subexpr: * Convert SystemTypeName() to SystemFuncName() even though * at the moment they result in the same thing. */ - $$ = (Node *) makeFuncCall(SystemFuncName(((Value *)llast($5->names))->val.str), - list_make1($3), - @1); + $$ = (Node *) makeFuncCall(SystemFuncName(((Value *) llast($5->names))->val.str), + list_make1($3), + COERCE_EXPLICIT_CALL, + @1); } | TRIM '(' BOTH trim_list ')' { /* various trim expressions are defined in SQL * - thomas 1997-07-19 */ - $$ = (Node *) makeFuncCall(SystemFuncName("btrim"), $4, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("btrim"), + $4, + COERCE_SQL_SYNTAX, + @1); } | TRIM '(' LEADING trim_list ')' { - $$ = (Node *) makeFuncCall(SystemFuncName("ltrim"), $4, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("ltrim"), + $4, + COERCE_SQL_SYNTAX, + @1); } | TRIM '(' TRAILING trim_list ')' { - $$ = (Node *) makeFuncCall(SystemFuncName("rtrim"), $4, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("rtrim"), + $4, + COERCE_SQL_SYNTAX, + @1); } | TRIM '(' trim_list ')' { - $$ = (Node *) makeFuncCall(SystemFuncName("btrim"), $3, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("btrim"), + $3, + COERCE_SQL_SYNTAX, + @1); } | NULLIF '(' a_expr ',' a_expr ')' { @@ -13915,7 +14013,10 @@ func_expr_common_subexpr: { /* xmlexists(A PASSING [BY REF] B [BY REF]) is * converted to xmlexists(A, B)*/ - $$ = (Node *) makeFuncCall(SystemFuncName("xmlexists"), list_make2($3, $4), @1); + $$ = (Node *) makeFuncCall(SystemFuncName("xmlexists"), + list_make2($3, $4), + COERCE_SQL_SYNTAX, + @1); } | XMLFOREST '(' xml_attribute_list ')' { @@ -14399,6 +14500,10 @@ func_arg_expr: a_expr } ; +func_arg_list_opt: func_arg_list { $$ = $1; } + | /*EMPTY*/ { $$ = NIL; } + ; + type_list: Typename { $$ = list_make1($1); } | type_list ',' Typename { $$ = lappend($1, $3); } ; @@ -14427,7 +14532,6 @@ extract_list: { $$ = list_make2(makeStringConst($1, @1), $3); } - | /*EMPTY*/ { $$ = NIL; } ; /* Allow delimited string Sconst in extract_arg as an SQL extension. @@ -14445,10 +14549,10 @@ extract_arg: ; unicode_normal_form: - NFC { $$ = "nfc"; } - | NFD { $$ = "nfd"; } - | NFKC { $$ = "nfkc"; } - | NFKD { $$ = "nfkd"; } + NFC { $$ = "NFC"; } + | NFD { $$ = "NFD"; } + | NFKC { $$ = "NFKC"; } + | NFKD { $$ = "NFKD"; } ; /* OVERLAY() arguments */ @@ -14468,29 +14572,24 @@ overlay_list: /* position_list uses b_expr not a_expr to avoid conflict with general IN */ position_list: b_expr IN_P b_expr { $$ = list_make2($3, $1); } - | /*EMPTY*/ { $$ = NIL; } ; /* * SUBSTRING() arguments * * Note that SQL:1999 has both - * * text FROM int FOR int - * * and - * * text FROM pattern FOR escape * * In the parser we map them both to a call to the substring() function and * rely on type resolution to pick the right one. * * In SQL:2003, the second variant was changed to - * * text SIMILAR pattern ESCAPE escape - * * We could in theory map that to a different function internally, but - * since we still support the SQL:1999 version, we don't. + * since we still support the SQL:1999 version, we don't. However, + * ruleutils.c will reverse-list the call in the newer style. */ substr_list: a_expr FROM a_expr FOR a_expr @@ -14504,6 +14603,13 @@ substr_list: } | a_expr FROM a_expr { + /* + * Because we aren't restricting data types here, this + * syntax can end up resolving to textregexsubstr(). + * We've historically allowed that to happen, so continue + * to accept it. However, ruleutils.c will reverse-list + * such a call in regular function call syntax. + */ $$ = list_make2($1, $3); } | a_expr FOR a_expr @@ -14527,16 +14633,6 @@ substr_list: { $$ = list_make3($1, $3, $5); } - /* - * We also want to support generic substring functions that - * accept the usual generic list of arguments. - */ - | expr_list - { - $$ = $1; - } - | /*EMPTY*/ - { $$ = NIL; } ; trim_list: a_expr FROM expr_list { $$ = lappend($3, $1); } diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 7460e61160..ea4a1f5aeb 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -541,10 +541,11 @@ transformRangeFunction(ParseState *pstate, RangeFunction *r) list_length(fc->args) > 1 && fc->agg_order == NIL && fc->agg_filter == NULL && + fc->over == NULL && !fc->agg_star && !fc->agg_distinct && !fc->func_variadic && - fc->over == NULL && + fc->funcformat == COERCE_EXPLICIT_CALL && coldeflist == NIL) { ListCell *lc; @@ -558,6 +559,7 @@ transformRangeFunction(ParseState *pstate, RangeFunction *r) newfc = makeFuncCall(SystemFuncName("unnest"), list_make1(arg), + COERCE_EXPLICIT_CALL, fc->location); newfexpr = transformExpr(pstate, (Node *) newfc, diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index a7a31704fb..8b4e3ca5e1 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -91,11 +91,12 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, bool is_column = (fn == NULL); List *agg_order = (fn ? fn->agg_order : NIL); Expr *agg_filter = NULL; + WindowDef *over = (fn ? fn->over : NULL); bool agg_within_group = (fn ? fn->agg_within_group : false); bool agg_star = (fn ? fn->agg_star : false); bool agg_distinct = (fn ? fn->agg_distinct : false); bool func_variadic = (fn ? fn->func_variadic : false); - WindowDef *over = (fn ? fn->over : NULL); + CoercionForm funcformat = (fn ? fn->funcformat : COERCE_EXPLICIT_CALL); bool could_be_projection; Oid rettype; Oid funcid; @@ -221,6 +222,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, agg_order == NIL && agg_filter == NULL && !agg_star && !agg_distinct && over == NULL && !func_variadic && argnames == NIL && + funcformat == COERCE_EXPLICIT_CALL && list_length(funcname) == 1 && (actual_arg_types[0] == RECORDOID || ISCOMPLEX(actual_arg_types[0]))); @@ -742,7 +744,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, funcexpr->funcresulttype = rettype; funcexpr->funcretset = retset; funcexpr->funcvariadic = func_variadic; - funcexpr->funcformat = COERCE_EXPLICIT_CALL; + funcexpr->funcformat = funcformat; /* funccollid and inputcollid will be set by parse_collate.c */ funcexpr->args = fargs; funcexpr->location = location; diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 015b0538e3..254c0f65c2 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -604,6 +604,7 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) castnode->location = -1; funccallnode = makeFuncCall(SystemFuncName("nextval"), list_make1(castnode), + COERCE_EXPLICIT_CALL, -1); constraint = makeNode(Constraint); constraint->contype = CONSTR_DEFAULT; diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 28f56074c0..3fabcca82f 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -443,6 +443,7 @@ static void get_agg_expr(Aggref *aggref, deparse_context *context, static void get_agg_combine_expr(Node *node, deparse_context *context, void *callback_arg); static void get_windowfunc_expr(WindowFunc *wfunc, deparse_context *context); +static bool get_func_sql_syntax(FuncExpr *expr, deparse_context *context); static void get_coercion_expr(Node *arg, deparse_context *context, Oid resulttype, int32 resulttypmod, Node *parentNode); @@ -9155,7 +9156,8 @@ looks_like_function(Node *node) { case T_FuncExpr: /* OK, unless it's going to deparse as a cast */ - return (((FuncExpr *) node)->funcformat == COERCE_EXPLICIT_CALL); + return (((FuncExpr *) node)->funcformat == COERCE_EXPLICIT_CALL || + ((FuncExpr *) node)->funcformat == COERCE_SQL_SYNTAX); case T_NullIfExpr: case T_CoalesceExpr: case T_MinMaxExpr: @@ -9257,6 +9259,17 @@ get_func_expr(FuncExpr *expr, deparse_context *context, return; } + /* + * If the function was called using one of the SQL spec's random special + * syntaxes, try to reproduce that. If we don't recognize the function, + * fall through. + */ + if (expr->funcformat == COERCE_SQL_SYNTAX) + { + if (get_func_sql_syntax(expr, context)) + return; + } + /* * Normal function: display as proname(args). First we need to extract * the argument datatypes. @@ -9492,6 +9505,223 @@ get_windowfunc_expr(WindowFunc *wfunc, deparse_context *context) } } +/* + * get_func_sql_syntax - Parse back a SQL-syntax function call + * + * Returns true if we successfully deparsed, false if we did not + * recognize the function. + */ +static bool +get_func_sql_syntax(FuncExpr *expr, deparse_context *context) +{ + StringInfo buf = context->buf; + Oid funcoid = expr->funcid; + + switch (funcoid) + { + case F_TIMEZONE_INTERVAL_TIMESTAMP: + case F_TIMEZONE_INTERVAL_TIMESTAMPTZ: + case F_TIMEZONE_INTERVAL_TIMETZ: + case F_TIMEZONE_TEXT_TIMESTAMP: + case F_TIMEZONE_TEXT_TIMESTAMPTZ: + case F_TIMEZONE_TEXT_TIMETZ: + /* AT TIME ZONE ... note reversed argument order */ + appendStringInfoChar(buf, '('); + get_rule_expr((Node *) lsecond(expr->args), context, false); + appendStringInfoString(buf, " AT TIME ZONE "); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoChar(buf, ')'); + return true; + + case F_OVERLAPS_TIMESTAMPTZ_INTERVAL_TIMESTAMPTZ_INTERVAL: + case F_OVERLAPS_TIMESTAMPTZ_INTERVAL_TIMESTAMPTZ_TIMESTAMPTZ: + case F_OVERLAPS_TIMESTAMPTZ_TIMESTAMPTZ_TIMESTAMPTZ_INTERVAL: + case F_OVERLAPS_TIMESTAMPTZ_TIMESTAMPTZ_TIMESTAMPTZ_TIMESTAMPTZ: + case F_OVERLAPS_TIMESTAMP_INTERVAL_TIMESTAMP_INTERVAL: + case F_OVERLAPS_TIMESTAMP_INTERVAL_TIMESTAMP_TIMESTAMP: + case F_OVERLAPS_TIMESTAMP_TIMESTAMP_TIMESTAMP_INTERVAL: + case F_OVERLAPS_TIMESTAMP_TIMESTAMP_TIMESTAMP_TIMESTAMP: + case F_OVERLAPS_TIMETZ_TIMETZ_TIMETZ_TIMETZ: + case F_OVERLAPS_TIME_INTERVAL_TIME_INTERVAL: + case F_OVERLAPS_TIME_INTERVAL_TIME_TIME: + case F_OVERLAPS_TIME_TIME_TIME_INTERVAL: + case F_OVERLAPS_TIME_TIME_TIME_TIME: + /* (x1, x2) OVERLAPS (y1, y2) */ + appendStringInfoString(buf, "(("); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoString(buf, ", "); + get_rule_expr((Node *) lsecond(expr->args), context, false); + appendStringInfoString(buf, ") OVERLAPS ("); + get_rule_expr((Node *) lthird(expr->args), context, false); + appendStringInfoString(buf, ", "); + get_rule_expr((Node *) lfourth(expr->args), context, false); + appendStringInfoString(buf, "))"); + return true; + + case F_IS_NORMALIZED: + /* IS xxx NORMALIZED */ + appendStringInfoString(buf, "(("); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoString(buf, ") IS"); + if (list_length(expr->args) == 2) + { + Const *con = (Const *) lsecond(expr->args); + + Assert(IsA(con, Const) && + con->consttype == TEXTOID && + !con->constisnull); + appendStringInfo(buf, " %s", + TextDatumGetCString(con->constvalue)); + } + appendStringInfoString(buf, " NORMALIZED)"); + return true; + + case F_PG_COLLATION_FOR: + /* COLLATION FOR */ + appendStringInfoString(buf, "COLLATION FOR ("); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoChar(buf, ')'); + return true; + + /* + * XXX EXTRACT, a/k/a date_part(), is intentionally not covered + * yet. Add it after we change the return type to numeric. + */ + + case F_NORMALIZE: + /* NORMALIZE() */ + appendStringInfoString(buf, "NORMALIZE("); + get_rule_expr((Node *) linitial(expr->args), context, false); + if (list_length(expr->args) == 2) + { + Const *con = (Const *) lsecond(expr->args); + + Assert(IsA(con, Const) && + con->consttype == TEXTOID && + !con->constisnull); + appendStringInfo(buf, ", %s", + TextDatumGetCString(con->constvalue)); + } + appendStringInfoChar(buf, ')'); + return true; + + case F_OVERLAY_BIT_BIT_INT4: + case F_OVERLAY_BIT_BIT_INT4_INT4: + case F_OVERLAY_BYTEA_BYTEA_INT4: + case F_OVERLAY_BYTEA_BYTEA_INT4_INT4: + case F_OVERLAY_TEXT_TEXT_INT4: + case F_OVERLAY_TEXT_TEXT_INT4_INT4: + /* OVERLAY() */ + appendStringInfoString(buf, "OVERLAY("); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoString(buf, " PLACING "); + get_rule_expr((Node *) lsecond(expr->args), context, false); + appendStringInfoString(buf, " FROM "); + get_rule_expr((Node *) lthird(expr->args), context, false); + if (list_length(expr->args) == 4) + { + appendStringInfoString(buf, " FOR "); + get_rule_expr((Node *) lfourth(expr->args), context, false); + } + appendStringInfoChar(buf, ')'); + return true; + + case F_POSITION_BIT_BIT: + case F_POSITION_BYTEA_BYTEA: + case F_POSITION_TEXT_TEXT: + /* POSITION() ... extra parens since args are b_expr not a_expr */ + appendStringInfoString(buf, "POSITION(("); + get_rule_expr((Node *) lsecond(expr->args), context, false); + appendStringInfoString(buf, ") IN ("); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoString(buf, "))"); + return true; + + case F_SUBSTRING_BIT_INT4: + case F_SUBSTRING_BIT_INT4_INT4: + case F_SUBSTRING_BYTEA_INT4: + case F_SUBSTRING_BYTEA_INT4_INT4: + case F_SUBSTRING_TEXT_INT4: + case F_SUBSTRING_TEXT_INT4_INT4: + /* SUBSTRING FROM/FOR (i.e., integer-position variants) */ + appendStringInfoString(buf, "SUBSTRING("); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoString(buf, " FROM "); + get_rule_expr((Node *) lsecond(expr->args), context, false); + if (list_length(expr->args) == 3) + { + appendStringInfoString(buf, " FOR "); + get_rule_expr((Node *) lthird(expr->args), context, false); + } + appendStringInfoChar(buf, ')'); + return true; + + case F_SUBSTRING_TEXT_TEXT_TEXT: + /* SUBSTRING SIMILAR/ESCAPE */ + appendStringInfoString(buf, "SUBSTRING("); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoString(buf, " SIMILAR "); + get_rule_expr((Node *) lsecond(expr->args), context, false); + appendStringInfoString(buf, " ESCAPE "); + get_rule_expr((Node *) lthird(expr->args), context, false); + appendStringInfoChar(buf, ')'); + return true; + + case F_BTRIM_BYTEA_BYTEA: + case F_BTRIM_TEXT: + case F_BTRIM_TEXT_TEXT: + /* TRIM() */ + appendStringInfoString(buf, "TRIM(BOTH"); + if (list_length(expr->args) == 2) + { + appendStringInfoChar(buf, ' '); + get_rule_expr((Node *) lsecond(expr->args), context, false); + } + appendStringInfoString(buf, " FROM "); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoChar(buf, ')'); + return true; + + case F_LTRIM_TEXT: + case F_LTRIM_TEXT_TEXT: + /* TRIM() */ + appendStringInfoString(buf, "TRIM(LEADING"); + if (list_length(expr->args) == 2) + { + appendStringInfoChar(buf, ' '); + get_rule_expr((Node *) lsecond(expr->args), context, false); + } + appendStringInfoString(buf, " FROM "); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoChar(buf, ')'); + return true; + + case F_RTRIM_TEXT: + case F_RTRIM_TEXT_TEXT: + /* TRIM() */ + appendStringInfoString(buf, "TRIM(TRAILING"); + if (list_length(expr->args) == 2) + { + appendStringInfoChar(buf, ' '); + get_rule_expr((Node *) lsecond(expr->args), context, false); + } + appendStringInfoString(buf, " FROM "); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoChar(buf, ')'); + return true; + + case F_XMLEXISTS: + /* XMLEXISTS ... extra parens because args are c_expr */ + appendStringInfoString(buf, "XMLEXISTS(("); + get_rule_expr((Node *) linitial(expr->args), context, false); + appendStringInfoString(buf, ") PASSING ("); + get_rule_expr((Node *) lsecond(expr->args), context, false); + appendStringInfoString(buf, "))"); + return true; + } + return false; +} + /* ---------- * get_coercion_expr * diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 302b9f6b2b..b3d0bc609b 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202011041 +#define CATALOG_VERSION_NO 202011042 #endif diff --git a/src/include/nodes/makefuncs.h b/src/include/nodes/makefuncs.h index 31d9aedeeb..7ebd794713 100644 --- a/src/include/nodes/makefuncs.h +++ b/src/include/nodes/makefuncs.h @@ -79,7 +79,8 @@ extern ColumnDef *makeColumnDef(const char *colname, extern FuncExpr *makeFuncExpr(Oid funcid, Oid rettype, List *args, Oid funccollid, Oid inputcollid, CoercionForm fformat); -extern FuncCall *makeFuncCall(List *name, List *args, int location); +extern FuncCall *makeFuncCall(List *name, List *args, + CoercionForm funcformat, int location); extern Expr *make_opclause(Oid opno, Oid opresulttype, bool opretset, Expr *leftop, Expr *rightop, diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index e1aeea2560..7ef9b0eac0 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -353,11 +353,12 @@ typedef struct FuncCall List *args; /* the arguments (list of exprs) */ List *agg_order; /* ORDER BY (list of SortBy) */ Node *agg_filter; /* FILTER clause, if any */ + struct WindowDef *over; /* OVER clause, if any */ bool agg_within_group; /* ORDER BY appeared in WITHIN GROUP */ bool agg_star; /* argument was really '*' */ bool agg_distinct; /* arguments were labeled DISTINCT */ bool func_variadic; /* last argument was labeled VARIADIC */ - struct WindowDef *over; /* OVER clause, if any */ + CoercionForm funcformat; /* how to display this node */ int location; /* token location, or -1 if unknown */ } FuncCall; @@ -1854,7 +1855,7 @@ typedef enum AlterTableType AT_AddIdentity, /* ADD IDENTITY */ AT_SetIdentity, /* SET identity column options */ AT_DropIdentity, /* DROP IDENTITY */ - AT_AlterCollationRefreshVersion /* ALTER COLLATION ... REFRESH VERSION */ + AT_AlterCollationRefreshVersion /* ALTER COLLATION ... REFRESH VERSION */ } AlterTableType; typedef struct ReplicaIdentityStmt diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index fd65ee8f9c..5b190bb99b 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -445,7 +445,10 @@ typedef enum CoercionContext } CoercionContext; /* - * CoercionForm - how to display a node that could have come from a cast + * CoercionForm - how to display a FuncExpr or related node + * + * "Coercion" is a bit of a misnomer, since this value records other + * special syntaxes besides casts, but for now we'll keep this naming. * * NB: equal() ignores CoercionForm fields, therefore this *must* not carry * any semantically significant information. We need that behavior so that @@ -457,7 +460,8 @@ typedef enum CoercionForm { COERCE_EXPLICIT_CALL, /* display as a function call */ COERCE_EXPLICIT_CAST, /* display as an explicit cast */ - COERCE_IMPLICIT_CAST /* implicit cast, so hide it */ + COERCE_IMPLICIT_CAST, /* implicit cast, so hide it */ + COERCE_SQL_SYNTAX /* display with SQL-mandated special syntax */ } CoercionForm; /* diff --git a/src/test/modules/test_rls_hooks/test_rls_hooks.c b/src/test/modules/test_rls_hooks/test_rls_hooks.c index 0bfa878a25..c0aaabdcdb 100644 --- a/src/test/modules/test_rls_hooks/test_rls_hooks.c +++ b/src/test/modules/test_rls_hooks/test_rls_hooks.c @@ -95,7 +95,10 @@ test_rls_hooks_permissive(CmdType cmdtype, Relation relation) */ n = makeFuncCall(list_make2(makeString("pg_catalog"), - makeString("current_user")), NIL, 0); + makeString("current_user")), + NIL, + COERCE_EXPLICIT_CALL, + -1); c = makeNode(ColumnRef); c->fields = list_make1(makeString("username")); @@ -155,7 +158,10 @@ test_rls_hooks_restrictive(CmdType cmdtype, Relation relation) policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true, TYPALIGN_INT); n = makeFuncCall(list_make2(makeString("pg_catalog"), - makeString("current_user")), NIL, 0); + makeString("current_user")), + NIL, + COERCE_EXPLICIT_CALL, + -1); c = makeNode(ColumnRef); c->fields = list_make1(makeString("supervisor")); diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out index f10a3a7a12..b234d2d4f9 100644 --- a/src/test/regress/expected/create_view.out +++ b/src/test/regress/expected/create_view.out @@ -1710,13 +1710,52 @@ select pg_get_viewdef('tt20v', true); i4.i4, + i8.i8 + FROM COALESCE(1, 2) c(c), + - pg_collation_for('x'::text) col(col), + + COLLATION FOR ('x'::text) col(col), + CURRENT_DATE d(d), + LOCALTIMESTAMP(3) t(t), + CAST(1 + 2 AS integer) i4(i4), + CAST((1 + 2)::bigint AS bigint) i8(i8); (1 row) +-- reverse-listing of various special function syntaxes required by SQL +create view tt201v as +select + extract(day from now()) as extr, + (now(), '1 day'::interval) overlaps + (current_timestamp(2), '1 day'::interval) as o, + 'foo' is normalized isn, + 'foo' is nfkc normalized isnn, + normalize('foo') as n, + normalize('foo', nfkd) as nfkd, + overlay('foo' placing 'bar' from 2) as ovl, + overlay('foo' placing 'bar' from 2 for 3) as ovl2, + position('foo' in 'foobar') as p, + substring('foo' from 2 for 3) as s, + substring('foo' similar 'f' escape '#') as ss, + substring('foo' from 'oo') as ssf, -- historically-permitted abuse + trim(' ' from ' foo ') as bt, + trim(leading ' ' from ' foo ') as lt, + trim(trailing ' foo ') as rt; +select pg_get_viewdef('tt201v', true); + pg_get_viewdef +----------------------------------------------------------------------------------------------- + SELECT date_part('day'::text, now()) AS extr, + + ((now(), '@ 1 day'::interval) OVERLAPS (CURRENT_TIMESTAMP(2), '@ 1 day'::interval)) AS o,+ + (('foo'::text) IS NORMALIZED) AS isn, + + (('foo'::text) IS NFKC NORMALIZED) AS isnn, + + NORMALIZE('foo'::text) AS n, + + NORMALIZE('foo'::text, NFKD) AS nfkd, + + OVERLAY('foo'::text PLACING 'bar'::text FROM 2) AS ovl, + + OVERLAY('foo'::text PLACING 'bar'::text FROM 2 FOR 3) AS ovl2, + + POSITION(('foo'::text) IN ('foobar'::text)) AS p, + + SUBSTRING('foo'::text FROM 2 FOR 3) AS s, + + SUBSTRING('foo'::text SIMILAR 'f'::text ESCAPE '#'::text) AS ss, + + "substring"('foo'::text, 'oo'::text) AS ssf, + + TRIM(BOTH ' '::text FROM ' foo '::text) AS bt, + + TRIM(LEADING ' '::text FROM ' foo '::text) AS lt, + + TRIM(TRAILING FROM ' foo '::text) AS rt; +(1 row) + -- corner cases with empty join conditions create view tt21v as select * from tt5 natural inner join tt6; @@ -1904,7 +1943,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 67 other objects +NOTICE: drop cascades to 68 other objects DETAIL: drop cascades to table t1 drop cascades to view temporal1 drop cascades to view temporal2 @@ -1966,6 +2005,7 @@ drop cascades to view tt17v drop cascades to view tt18v drop cascades to view tt19v drop cascades to view tt20v +drop cascades to view tt201v drop cascades to view tt21v drop cascades to view tt22v drop cascades to view tt23v diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out index 639b50308e..c300965554 100644 --- a/src/test/regress/expected/timestamptz.out +++ b/src/test/regress/expected/timestamptz.out @@ -2726,10 +2726,10 @@ create temp table tmptz (f1 timestamptz primary key); insert into tmptz values ('2017-01-18 00:00+00'); explain (costs off) select * from tmptz where f1 at time zone 'utc' = '2017-01-18 00:00'; - QUERY PLAN -------------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------------------------------------------- Seq Scan on tmptz - Filter: (timezone('utc'::text, f1) = 'Wed Jan 18 00:00:00 2017'::timestamp without time zone) + Filter: ((f1 AT TIME ZONE 'utc'::text) = 'Wed Jan 18 00:00:00 2017'::timestamp without time zone) (2 rows) select * from tmptz where f1 at time zone 'utc' = '2017-01-18 00:00'; diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql index e7af0bf2fa..6d4dd53965 100644 --- a/src/test/regress/sql/create_view.sql +++ b/src/test/regress/sql/create_view.sql @@ -586,6 +586,28 @@ select * from cast(1+2 as int8) as i8; select pg_get_viewdef('tt20v', true); +-- reverse-listing of various special function syntaxes required by SQL + +create view tt201v as +select + extract(day from now()) as extr, + (now(), '1 day'::interval) overlaps + (current_timestamp(2), '1 day'::interval) as o, + 'foo' is normalized isn, + 'foo' is nfkc normalized isnn, + normalize('foo') as n, + normalize('foo', nfkd) as nfkd, + overlay('foo' placing 'bar' from 2) as ovl, + overlay('foo' placing 'bar' from 2 for 3) as ovl2, + position('foo' in 'foobar') as p, + substring('foo' from 2 for 3) as s, + substring('foo' similar 'f' escape '#') as ss, + substring('foo' from 'oo') as ssf, -- historically-permitted abuse + trim(' ' from ' foo ') as bt, + trim(leading ' ' from ' foo ') as lt, + trim(trailing ' foo ') as rt; +select pg_get_viewdef('tt201v', true); + -- corner cases with empty join conditions create view tt21v as