From 1d2d6e4803f3ab3e9d2c29efcc8dee0f8a53d699 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 25 Oct 2022 14:15:14 +0900 Subject: [PATCH v3 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/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 +++++++++++++-------------- 8 files changed, 55 insertions(+), 92 deletions(-) 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 f71f551782..f6dd27edcc 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 e5fc708c8a..0fdbf82f3a 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; -- 2.37.2