v3-0001-Remove-from-SQLValueFunction-all-the-entries-usin.patch

From 1d2d6e4803f3ab3e9d2c29efcc8dee0f8a53d699 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
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