From 8d1a424e44f46f3107a6f0f066da22c19e4e4f3b Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 21 Oct 2022 14:06:03 +0900 Subject: [PATCH v2 1/2] Remove from SQLValueFunction all the entries using "name" as return type This commit changes six SQL keywords to use COERCE_SQL_SYNTAX rather than SQLValueFunction: - CURRENT_ROLE - CURRENT_USER - USER - SESSION_USER - CURRENT_CATALOG - CURRENT_SCHEMA Among the six, "user", "current_role" and "current_catalog" require specific SQL functions to allow ruleutils.c to map them to the SQL keywords these require when using COERCE_SQL_SYNTAX. Having pg_proc.proname match with the keyword ensures that the compatibility remains the same when projecting any of these keywords in a FROM clause to an attribute name when an alias is not specified. Tests are added to make sure that the mapping happens from the SQL keyword is correct, using a view defition that feeds on these keywords in one of the tests introduced by 40c24bf (both in a SELECT clause and when using each keyword in a FROM clause). SQLValueFunction is reduced to half its contents after this change, simplifying its logic a bit as there is no need to enforce a C collation anymore. I have made a few performance tests, with a million-ish calls to these keywords without seeing a difference in run-time or in perf profiles. The remaining SQLValueFunctions relate to timestamps. Bump catalog version. Reviewed-by: Corey Huinker Discussion: https://postgr.es/m/YzaG3MoryCguUOym@paquier.xyz --- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 9 ++++++ src/include/nodes/primnodes.h | 8 +---- src/backend/executor/execExprInterp.c | 27 ----------------- src/backend/nodes/nodeFuncs.c | 11 ++----- src/backend/parser/gram.y | 30 ++++++++++++++---- src/backend/parser/parse_expr.c | 8 ----- src/backend/parser/parse_target.c | 18 ----------- src/backend/utils/adt/ruleutils.c | 36 +++++++++++----------- src/test/regress/expected/create_view.out | 37 +++++++++++++++++++++-- src/test/regress/sql/create_view.sql | 15 ++++++++- 11 files changed, 105 insertions(+), 96 deletions(-) diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 4c930c189b..032a429345 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202210141 +#define CATALOG_VERSION_NO 202210211 #endif diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 62a5b8e655..241366fc8e 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -1505,6 +1505,15 @@ { oid => '745', descr => 'current user name', proname => 'current_user', provolatile => 's', prorettype => 'name', proargtypes => '', prosrc => 'current_user' }, +{ oid => '9695', descr => 'current role name', + proname => 'current_role', provolatile => 's', prorettype => 'name', + proargtypes => '', prosrc => 'current_user' }, +{ oid => '9696', descr => 'user name', + proname => 'user', provolatile => 's', prorettype => 'name', + proargtypes => '', prosrc => 'current_user' }, +{ oid => '9697', descr => 'name of the current database', + proname => 'current_catalog', provolatile => 's', prorettype => 'name', + proargtypes => '', prosrc => 'current_database' }, { oid => '746', descr => 'session user name', proname => 'session_user', provolatile => 's', prorettype => 'name', proargtypes => '', prosrc => 'session_user' }, diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 40661334bb..e818231e15 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1313,13 +1313,7 @@ typedef enum SQLValueFunctionOp SVFOP_LOCALTIME, SVFOP_LOCALTIME_N, SVFOP_LOCALTIMESTAMP, - SVFOP_LOCALTIMESTAMP_N, - SVFOP_CURRENT_ROLE, - SVFOP_CURRENT_USER, - SVFOP_USER, - SVFOP_SESSION_USER, - SVFOP_CURRENT_CATALOG, - SVFOP_CURRENT_SCHEMA + SVFOP_LOCALTIMESTAMP_N } SQLValueFunctionOp; typedef struct SQLValueFunction diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 9b9bbf00a9..6ebf5c287e 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -2495,15 +2495,10 @@ ExecEvalParamExtern(ExprState *state, ExprEvalStep *op, ExprContext *econtext) void ExecEvalSQLValueFunction(ExprState *state, ExprEvalStep *op) { - LOCAL_FCINFO(fcinfo, 0); SQLValueFunction *svf = op->d.sqlvaluefunction.svf; *op->resnull = false; - /* - * Note: current_schema() can return NULL. current_user() etc currently - * cannot, but might as well code those cases the same way for safety. - */ switch (svf->op) { case SVFOP_CURRENT_DATE: @@ -2525,28 +2520,6 @@ ExecEvalSQLValueFunction(ExprState *state, ExprEvalStep *op) case SVFOP_LOCALTIMESTAMP_N: *op->resvalue = TimestampGetDatum(GetSQLLocalTimestamp(svf->typmod)); break; - case SVFOP_CURRENT_ROLE: - case SVFOP_CURRENT_USER: - case SVFOP_USER: - InitFunctionCallInfoData(*fcinfo, NULL, 0, InvalidOid, NULL, NULL); - *op->resvalue = current_user(fcinfo); - *op->resnull = fcinfo->isnull; - break; - case SVFOP_SESSION_USER: - InitFunctionCallInfoData(*fcinfo, NULL, 0, InvalidOid, NULL, NULL); - *op->resvalue = session_user(fcinfo); - *op->resnull = fcinfo->isnull; - break; - case SVFOP_CURRENT_CATALOG: - InitFunctionCallInfoData(*fcinfo, NULL, 0, InvalidOid, NULL, NULL); - *op->resvalue = current_database(fcinfo); - *op->resnull = fcinfo->isnull; - break; - case SVFOP_CURRENT_SCHEMA: - InitFunctionCallInfoData(*fcinfo, NULL, 0, InvalidOid, NULL, NULL); - *op->resvalue = current_schema(fcinfo); - *op->resnull = fcinfo->isnull; - break; } } diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c index 0a7b22f97e..2585a3175c 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -917,11 +917,8 @@ exprCollation(const Node *expr) coll = ((const MinMaxExpr *) expr)->minmaxcollid; break; case T_SQLValueFunction: - /* Returns either NAME or a non-collatable type */ - if (((const SQLValueFunction *) expr)->type == NAMEOID) - coll = C_COLLATION_OID; - else - coll = InvalidOid; + /* Returns a non-collatable type */ + coll = InvalidOid; break; case T_XmlExpr: @@ -1144,9 +1141,7 @@ exprSetCollation(Node *expr, Oid collation) ((MinMaxExpr *) expr)->minmaxcollid = collation; break; case T_SQLValueFunction: - Assert((((SQLValueFunction *) expr)->type == NAMEOID) ? - (collation == C_COLLATION_OID) : - (collation == InvalidOid)); + Assert(collation == InvalidOid); break; case T_XmlExpr: Assert((((XmlExpr *) expr)->op == IS_XMLSERIALIZE) ? diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 737bd2d06d..605e9ec096 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -15229,15 +15229,24 @@ func_expr_common_subexpr: } | CURRENT_ROLE { - $$ = makeSQLValueFunction(SVFOP_CURRENT_ROLE, -1, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("current_role"), + NIL, + COERCE_SQL_SYNTAX, + @1); } | CURRENT_USER { - $$ = makeSQLValueFunction(SVFOP_CURRENT_USER, -1, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("current_user"), + NIL, + COERCE_SQL_SYNTAX, + @1); } | SESSION_USER { - $$ = makeSQLValueFunction(SVFOP_SESSION_USER, -1, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("session_user"), + NIL, + COERCE_SQL_SYNTAX, + @1); } | SYSTEM_USER { @@ -15248,15 +15257,24 @@ func_expr_common_subexpr: } | USER { - $$ = makeSQLValueFunction(SVFOP_USER, -1, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("user"), + NIL, + COERCE_SQL_SYNTAX, + @1); } | CURRENT_CATALOG { - $$ = makeSQLValueFunction(SVFOP_CURRENT_CATALOG, -1, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("current_catalog"), + NIL, + COERCE_SQL_SYNTAX, + @1); } | CURRENT_SCHEMA { - $$ = makeSQLValueFunction(SVFOP_CURRENT_SCHEMA, -1, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("current_schema"), + NIL, + COERCE_SQL_SYNTAX, + @1); } | CAST '(' a_expr AS Typename ')' { $$ = makeTypeCast($3, $5, @1); } diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 7aaf1c673f..9bde33b407 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -2231,14 +2231,6 @@ transformSQLValueFunction(ParseState *pstate, SQLValueFunction *svf) svf->type = TIMESTAMPOID; svf->typmod = anytimestamp_typmod_check(false, svf->typmod); break; - case SVFOP_CURRENT_ROLE: - case SVFOP_CURRENT_USER: - case SVFOP_USER: - case SVFOP_SESSION_USER: - case SVFOP_CURRENT_CATALOG: - case SVFOP_CURRENT_SCHEMA: - svf->type = NAMEOID; - break; } return (Node *) svf; diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index bd8057bc3e..f54591a987 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -1895,24 +1895,6 @@ FigureColnameInternal(Node *node, char **name) case SVFOP_LOCALTIMESTAMP_N: *name = "localtimestamp"; return 2; - case SVFOP_CURRENT_ROLE: - *name = "current_role"; - return 2; - case SVFOP_CURRENT_USER: - *name = "current_user"; - return 2; - case SVFOP_USER: - *name = "user"; - return 2; - case SVFOP_SESSION_USER: - *name = "session_user"; - return 2; - case SVFOP_CURRENT_CATALOG: - *name = "current_catalog"; - return 2; - case SVFOP_CURRENT_SCHEMA: - *name = "current_schema"; - return 2; } break; case T_XmlExpr: diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 70d723e80c..f95289f11c 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -9199,24 +9199,6 @@ get_rule_expr(Node *node, deparse_context *context, appendStringInfo(buf, "LOCALTIMESTAMP(%d)", svf->typmod); break; - case SVFOP_CURRENT_ROLE: - appendStringInfoString(buf, "CURRENT_ROLE"); - break; - case SVFOP_CURRENT_USER: - appendStringInfoString(buf, "CURRENT_USER"); - break; - case SVFOP_USER: - appendStringInfoString(buf, "USER"); - break; - case SVFOP_SESSION_USER: - appendStringInfoString(buf, "SESSION_USER"); - break; - case SVFOP_CURRENT_CATALOG: - appendStringInfoString(buf, "CURRENT_CATALOG"); - break; - case SVFOP_CURRENT_SCHEMA: - appendStringInfoString(buf, "CURRENT_SCHEMA"); - break; } } break; @@ -10318,6 +10300,24 @@ get_func_sql_syntax(FuncExpr *expr, deparse_context *context) appendStringInfoChar(buf, ')'); return true; + case F_CURRENT_CATALOG: + appendStringInfoString(buf, "CURRENT_CATALOG"); + return true; + case F_CURRENT_ROLE: + appendStringInfoString(buf, "CURRENT_ROLE"); + return true; + case F_CURRENT_SCHEMA: + appendStringInfoString(buf, "CURRENT_SCHEMA"); + return true; + case F_CURRENT_USER: + appendStringInfoString(buf, "CURRENT_USER"); + return true; + case F_USER: + appendStringInfoString(buf, "USER"); + return true; + case F_SESSION_USER: + appendStringInfoString(buf, "SESSION_USER"); + return true; case F_SYSTEM_USER: appendStringInfoString(buf, "SYSTEM_USER"); return true; diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out index bf4ff30d86..e7f7323113 100644 --- a/src/test/regress/expected/create_view.out +++ b/src/test/regress/expected/create_view.out @@ -1941,7 +1941,20 @@ select trim(E'\\000'::bytea from E'\\000Tom\\000'::bytea) as btb, trim(leading E'\\000'::bytea from E'\\000Tom\\000'::bytea) as ltb, trim(trailing E'\\000'::bytea from E'\\000Tom\\000'::bytea) as rtb, - SYSTEM_USER as su; + CURRENT_CATALOG as ca, + (select * from CURRENT_CATALOG) as ca2, + CURRENT_ROLE as cr, + (select * from CURRENT_ROLE) as cr2, + CURRENT_SCHEMA as cs, + (select * from CURRENT_SCHEMA) as cs2, + CURRENT_USER as cu, + (select * from CURRENT_USER) as cu2, + USER as us, + (select * from USER) as us2, + SESSION_USER seu, + (select * from SESSION_USER) as seu2, + SYSTEM_USER as su, + (select * from SYSTEM_USER) as su2; select pg_get_viewdef('tt201v', true); pg_get_viewdef ----------------------------------------------------------------------------------------------- @@ -1963,7 +1976,27 @@ select pg_get_viewdef('tt201v', true); TRIM(BOTH '\x00'::bytea FROM '\x00546f6d00'::bytea) AS btb, + TRIM(LEADING '\x00'::bytea FROM '\x00546f6d00'::bytea) AS ltb, + TRIM(TRAILING '\x00'::bytea FROM '\x00546f6d00'::bytea) AS rtb, + - SYSTEM_USER AS su; + CURRENT_CATALOG AS ca, + + ( SELECT "current_catalog"."current_catalog" + + FROM CURRENT_CATALOG "current_catalog"("current_catalog")) AS ca2, + + CURRENT_ROLE AS cr, + + ( SELECT "current_role"."current_role" + + FROM CURRENT_ROLE "current_role"("current_role")) AS cr2, + + CURRENT_SCHEMA AS cs, + + ( SELECT "current_schema"."current_schema" + + FROM CURRENT_SCHEMA "current_schema"("current_schema")) AS cs2, + + CURRENT_USER AS cu, + + ( SELECT "current_user"."current_user" + + FROM CURRENT_USER "current_user"("current_user")) AS cu2, + + USER AS us, + + ( SELECT "user"."user" + + FROM USER "user"("user")) AS us2, + + SESSION_USER AS seu, + + ( SELECT "session_user"."session_user" + + FROM SESSION_USER "session_user"("session_user")) AS seu2, + + SYSTEM_USER AS su, + + ( SELECT "system_user"."system_user" + + FROM SYSTEM_USER "system_user"("system_user")) AS su2; (1 row) -- corner cases with empty join conditions diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql index 913b4ee460..04fc0ba35e 100644 --- a/src/test/regress/sql/create_view.sql +++ b/src/test/regress/sql/create_view.sql @@ -722,7 +722,20 @@ select trim(E'\\000'::bytea from E'\\000Tom\\000'::bytea) as btb, trim(leading E'\\000'::bytea from E'\\000Tom\\000'::bytea) as ltb, trim(trailing E'\\000'::bytea from E'\\000Tom\\000'::bytea) as rtb, - SYSTEM_USER as su; + CURRENT_CATALOG as ca, + (select * from CURRENT_CATALOG) as ca2, + CURRENT_ROLE as cr, + (select * from CURRENT_ROLE) as cr2, + CURRENT_SCHEMA as cs, + (select * from CURRENT_SCHEMA) as cs2, + CURRENT_USER as cu, + (select * from CURRENT_USER) as cu2, + USER as us, + (select * from USER) as us2, + SESSION_USER seu, + (select * from SESSION_USER) as seu2, + SYSTEM_USER as su, + (select * from SYSTEM_USER) as su2; select pg_get_viewdef('tt201v', true); -- corner cases with empty join conditions -- 2.37.2