Subject: Re: benchmark results comparing versions 15.2 and 16

From: Michael Paquier <[email-masked]>
Date: Mon, 15 May 2023 14:20:24 +0900

On Thu, May 11, 2023 at 01:28:40PM +0900, Michael Paquier wrote:
> On Tue, May 09, 2023 at 09:48:24AM -0700, Andres Freund wrote:
>> On 2023-05-08 12:11:17 -0700, Andres Freund wrote:
>>> I can reproduce a significant regression due to f193883fc of a workload just
>>> running
>>> SELECT CURRENT_TIMESTAMP;
>>> 
>>> A single session running it on my workstation via pgbench -Mprepared gets
>>> before:
>>> tps = 89359.128359 (without initial connection time)
>>> after:
>>> tps = 83843.585152 (without initial connection time)
>>> 
>>> Obviously this is an extreme workload, but that nevertheless seems too large
>>> to just accept...

Extreme is adapted for a worst-case scenario.  Looking at my notes
from a few months back, that's kind of what I did on my laptop, which
was the only machine I had at hand back then:
- Compilation of code with -O2.
- Prepared statement of a simple SELECT combined with a DO block
running executed the query in a loop a few million times on a single
backend:
PREPARE test AS SELECT CURRENT_TIMESTAMP;
DO $$
BEGIN
  FOR i IN 1..10000000
  LOOP
    EXECUTE 'EXECUTE test';
  END LOOP;
END $$;
- The second test is mentioned at [1], with a generate_series() on a
keyword.
- And actually I recall some pgbench runs similar to that..  But I
don't have any traces of that in my notes.

This was not showing much difference, and it does not now, either.
Funny thing is that the pre-patch period was showing signs of being a
bit slower in this environment.  Anyway, I have just tested the DO
case in a second "bigbox" environment that I have set up for
benchmarking a few days ago, and the DO test is showing me a 1%~1.5%
regression in runtime.  That's repeatable so that's not noise.

I have re-run a bit more pgbench (1 client, prepared query with a
single SELECT on a SQL keyword, etc.).  And, TBH, I am not seeing as
much difference as you do (nothing with default pgbench setup, FWIW),
still that's able to show a bit more difference than the other two
cases.  HEAD shows me an average output close to 43900 TPS (3 run of
60s each, for instance), while relying on SQLValueFunction shows an
average of 45000TPS.  That counts for ~2.4% output regression here
on bigbox based on these numbers.  Not a regression as high as
mentioned above, still that's visible.

>> Added an open item for this.
> 
> Thanks for the report, I'll come back to it and look at it at the
> beginning of next week.  In the worst case, that would mean a revert
> of this refactoring, I assume.

So, this involves commits 7aa81c6, f193883 and fb32748.  7aa81c6 has
added some tests, so I would let the tests it added be on HEAD as the
precision was not tested for the SQL keywords this has added cover
for.

fb32748 has removed the dependency of SQLValueFunction on collations
by making the name functions use COERCE_SQL_SYNTAX.  One case where
these could be heavily used is RLS, for example, so that could be
visible.  f193883 has removed the rest and the timestamp keywords.

I am not going to let that hang in the air with beta1 getting released
next week, though, so attached are two patches to revert the change
(these would be combined, just posted this way for clarity).  The only
conflict I could see is caused by the query jumbling where a
SQLValueFunction needs "typmod" and "op" in the computation, ignoring
the rest, so the node definition primnodes.h gains the required
node_attr.  Making the execution path cheaper while avoiding the 
collation tweaks for SQLValueFunction would be nice, but not at this
time of the year on this branch.

[1]: https://www.postgresql.org/message-id/Y0+dHDYA46UnnLs/@paquier.xyz
--
Michael
From 29d4b8b47ee521b573cf091b3c5794616e5a6963 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 11 May 2023 14:34:17 +0900
Subject: [PATCH v1 1/2] Revert "Replace SQLValueFunction by COERCE_SQL_SYNTAX"

This reverts commit f193883fc9cebe8fa20359b0797832837a788112.
---
 src/include/catalog/catversion.h          |  2 +-
 src/include/catalog/pg_proc.dat           | 17 -----
 src/include/executor/execExpr.h           |  8 ++
 src/include/nodes/primnodes.h             | 37 +++++++++
 src/include/utils/date.h                  |  4 +
 src/include/utils/timestamp.h             |  4 +
 src/backend/catalog/system_functions.sql  | 26 -------
 src/backend/executor/execExpr.c           | 11 +++
 src/backend/executor/execExprInterp.c     | 46 ++++++++++++
 src/backend/jit/llvm/llvmjit_expr.c       |  6 ++
 src/backend/jit/llvm/llvmjit_types.c      |  1 +
 src/backend/nodes/nodeFuncs.c             | 27 ++++++-
 src/backend/optimizer/path/costsize.c     |  1 +
 src/backend/optimizer/util/clauses.c      | 39 ++++++++--
 src/backend/parser/gram.y                 | 59 ++++++---------
 src/backend/parser/parse_expr.c           | 52 +++++++++++++
 src/backend/parser/parse_target.c         | 25 ++++++
 src/backend/utils/adt/date.c              | 75 ++++++++----------
 src/backend/utils/adt/ruleutils.c         | 92 +++++++++++------------
 src/backend/utils/adt/timestamp.c         | 66 +++++++---------
 src/test/regress/expected/expressions.out |  2 +-
 src/test/regress/sql/expressions.sql      |  2 +-
 src/tools/pgindent/typedefs.list          |  2 +
 23 files changed, 385 insertions(+), 219 deletions(-)

diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index d10cc28b0c..cdd51094cd 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	202305121
+#define CATALOG_VERSION_NO	202305141
 
 #endif
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index b2bc81b15f..0a0f749385 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -1520,23 +1520,6 @@
 { oid => '9977', descr => 'system user name',
   proname => 'system_user', provolatile => 's', prorettype => 'text',
   proargtypes => '', prosrc => 'system_user' },
-{ oid => '9978', descr => 'current date',
-  proname => 'current_date', provolatile => 's', prorettype => 'date',
-  proargtypes => '', prosrc => 'current_date' },
-{ oid => '9979', descr => 'current time',
-  proname => 'current_time', proisstrict => 'f', provolatile => 's',
-  prorettype => 'timetz', proargtypes => 'int4', prosrc => 'current_time' },
-{ oid => '9980', descr => 'current timestamp',
-  proname => 'current_timestamp', proisstrict => 'f', provolatile => 's',
-  prorettype => 'timestamptz', proargtypes => 'int4',
-  prosrc => 'current_timestamp' },
-{ oid => '9981', descr => 'local time',
-  proname => 'localtime', proisstrict => 'f', provolatile => 's',
-  prorettype => 'time', proargtypes => 'int4', prosrc => 'sql_localtime' },
-{ oid => '9982', descr => 'local timestamp',
-  proname => 'localtimestamp', proisstrict => 'f', provolatile => 's',
-  prorettype => 'timestamp', proargtypes => 'int4',
-  prosrc => 'sql_localtimestamp' },
 
 { oid => '744',
   proname => 'array_eq', prorettype => 'bool',
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 157b0d85f2..048573c2bc 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -171,6 +171,7 @@ typedef enum ExprEvalOp
 	EEOP_DISTINCT,
 	EEOP_NOT_DISTINCT,
 	EEOP_NULLIF,
+	EEOP_SQLVALUEFUNCTION,
 	EEOP_CURRENTOFEXPR,
 	EEOP_NEXTVALUEEXPR,
 	EEOP_ARRAYEXPR,
@@ -418,6 +419,12 @@ typedef struct ExprEvalStep
 			FunctionCallInfo fcinfo_data_in;
 		}			iocoerce;
 
+		/* for EEOP_SQLVALUEFUNCTION */
+		struct
+		{
+			SQLValueFunction *svf;
+		}			sqlvaluefunction;
+
 		/* for EEOP_NEXTVALUEEXPR */
 		struct
 		{
@@ -769,6 +776,7 @@ extern void ExecEvalParamExec(ExprState *state, ExprEvalStep *op,
 							  ExprContext *econtext);
 extern void ExecEvalParamExtern(ExprState *state, ExprEvalStep *op,
 								ExprContext *econtext);
+extern void ExecEvalSQLValueFunction(ExprState *state, ExprEvalStep *op);
 extern void ExecEvalCurrentOfExpr(ExprState *state, ExprEvalStep *op);
 extern void ExecEvalNextValueExpr(ExprState *state, ExprEvalStep *op);
 extern void ExecEvalRowNull(ExprState *state, ExprEvalStep *op,
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index be9c29f0bf..7ade0d83f7 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1445,6 +1445,43 @@ typedef struct MinMaxExpr
 	int			location;
 } MinMaxExpr;
 
+/*
+ * SQLValueFunction - parameterless functions with special grammar productions
+ *
+ * The SQL standard categorizes some of these as <datetime value function>
+ * and others as <general value specification>.  We call 'em SQLValueFunctions
+ * for lack of a better term.  We store type and typmod of the result so that
+ * some code doesn't need to know each function individually, and because
+ * we would need to store typmod anyway for some of the datetime functions.
+ * Note that currently, all variants return non-collating datatypes, so we do
+ * not need a collation field; also, all these functions are stable.
+ */
+typedef enum SQLValueFunctionOp
+{
+	SVFOP_CURRENT_DATE,
+	SVFOP_CURRENT_TIME,
+	SVFOP_CURRENT_TIME_N,
+	SVFOP_CURRENT_TIMESTAMP,
+	SVFOP_CURRENT_TIMESTAMP_N,
+	SVFOP_LOCALTIME,
+	SVFOP_LOCALTIME_N,
+	SVFOP_LOCALTIMESTAMP,
+	SVFOP_LOCALTIMESTAMP_N
+} SQLValueFunctionOp;
+
+typedef struct SQLValueFunction
+{
+	Expr		xpr;
+	SQLValueFunctionOp op;		/* which function this is */
+	/*
+	 * Result type/typmod.  Type is fully determined by "op", so no need to
+	 * include this Oid in the query jumbling.
+	 */
+	Oid			type pg_node_attr(query_jumble_ignore);
+	int32		typmod;
+	int			location;		/* token location, or -1 if unknown */
+} SQLValueFunction;
+
 /*
  * XmlExpr - various SQL/XML functions requiring special grammar productions
  *
diff --git a/src/include/utils/date.h b/src/include/utils/date.h
index 5fd886b3db..97e1a02121 100644
--- a/src/include/utils/date.h
+++ b/src/include/utils/date.h
@@ -96,6 +96,7 @@ TimeTzADTPGetDatum(const TimeTzADT *X)

 /* date.c */
+extern int32 anytime_typmod_check(bool istz, int32 typmod);
 extern double date2timestamp_no_overflow(DateADT dateVal);
 extern Timestamp date2timestamp_opt_overflow(DateADT dateVal, int *overflow);
 extern TimestampTz date2timestamptz_opt_overflow(DateADT dateVal, int *overflow);
@@ -103,6 +104,9 @@ extern int32 date_cmp_timestamp_internal(DateADT dateVal, Timestamp dt2);
 extern int32 date_cmp_timestamptz_internal(DateADT dateVal, TimestampTz dt2);
 
 extern void EncodeSpecialDate(DateADT dt, char *str);
+extern DateADT GetSQLCurrentDate(void);
+extern TimeTzADT *GetSQLCurrentTime(int32 typmod);
+extern TimeADT GetSQLLocalTime(int32 typmod);
 extern int	time2tm(TimeADT time, struct pg_tm *tm, fsec_t *fsec);
 extern int	timetz2tm(TimeTzADT *time, struct pg_tm *tm, fsec_t *fsec, int *tzp);
 extern int	tm2time(struct pg_tm *tm, fsec_t fsec, TimeADT *result);
diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h
index edd59dc432..c4dd96c8c9 100644
--- a/src/include/utils/timestamp.h
+++ b/src/include/utils/timestamp.h
@@ -95,7 +95,11 @@ extern PGDLLIMPORT TimestampTz PgReloadTime;
 
 /* Internal routines (not fmgr-callable) */
 
+extern int32 anytimestamp_typmod_check(bool istz, int32 typmod);
+
 extern TimestampTz GetCurrentTimestamp(void);
+extern TimestampTz GetSQLCurrentTimestamp(int32 typmod);
+extern Timestamp GetSQLLocalTimestamp(int32 typmod);
 extern void TimestampDifference(TimestampTz start_time, TimestampTz stop_time,
 								long *secs, int *microsecs);
 extern long TimestampDifferenceMilliseconds(TimestampTz start_time,
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index b7c65ea37d..07c0d89c4f 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -601,32 +601,6 @@ LANGUAGE internal
 STRICT IMMUTABLE PARALLEL SAFE
 AS 'unicode_is_normalized';
 
--- Functions with SQL-mandated special syntax and some defaults.
-CREATE OR REPLACE FUNCTION
-  "current_time"(int4 DEFAULT NULL)
- RETURNS timetz
- LANGUAGE internal
- STABLE PARALLEL SAFE
-AS 'current_time';
-CREATE OR REPLACE FUNCTION
-  "current_timestamp"(int4 DEFAULT NULL)
- RETURNS timestamptz
- LANGUAGE internal
- STABLE PARALLEL SAFE
- AS 'current_timestamp';
-CREATE OR REPLACE FUNCTION
-  "localtime"(int4 DEFAULT NULL)
- RETURNS time
- LANGUAGE internal
- STABLE PARALLEL SAFE
- AS 'sql_localtime';
-CREATE OR REPLACE FUNCTION
-  "localtimestamp"(int4 DEFAULT NULL)
- RETURNS timestamp
- LANGUAGE internal
- STABLE PARALLEL SAFE
- AS 'sql_localtimestamp';
-
 --
 -- The default permissions for functions mean that anyone can execute them.
 -- A number of functions shouldn't be executable by just anyone, but rather
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index dcf56446c7..bf257a41c8 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -2213,6 +2213,17 @@ ExecInitExprRec(Expr *node, ExprState *state,
 				break;
 			}
 
+		case T_SQLValueFunction:
+			{
+				SQLValueFunction *svf = (SQLValueFunction *) node;
+
+				scratch.opcode = EEOP_SQLVALUEFUNCTION;
+				scratch.d.sqlvaluefunction.svf = svf;
+
+				ExprEvalPushStep(state, &scratch);
+				break;
+			}
+
 		case T_XmlExpr:
 			{
 				XmlExpr    *xexpr = (XmlExpr *) node;
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index ca44d39100..d98710bfb3 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -455,6 +455,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 		&&CASE_EEOP_DISTINCT,
 		&&CASE_EEOP_NOT_DISTINCT,
 		&&CASE_EEOP_NULLIF,
+		&&CASE_EEOP_SQLVALUEFUNCTION,
 		&&CASE_EEOP_CURRENTOFEXPR,
 		&&CASE_EEOP_NEXTVALUEEXPR,
 		&&CASE_EEOP_ARRAYEXPR,
@@ -1305,6 +1306,17 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 			EEO_NEXT();
 		}
 
+		EEO_CASE(EEOP_SQLVALUEFUNCTION)
+		{
+			/*
+			 * Doesn't seem worthwhile to have an inline implementation
+			 * efficiency-wise.
+			 */
+			ExecEvalSQLValueFunction(state, op);
+
+			EEO_NEXT();
+		}
+
 		EEO_CASE(EEOP_CURRENTOFEXPR)
 		{
 			/* error invocation uses space, and shouldn't ever occur */
@@ -2497,6 +2509,40 @@ ExecEvalParamExtern(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
 			 errmsg("no value found for parameter %d", paramId)));
 }
 
+/*
+ * Evaluate a SQLValueFunction expression.
+ */
+void
+ExecEvalSQLValueFunction(ExprState *state, ExprEvalStep *op)
+{
+	SQLValueFunction *svf = op->d.sqlvaluefunction.svf;
+
+	*op->resnull = false;
+
+	switch (svf->op)
+	{
+		case SVFOP_CURRENT_DATE:
+			*op->resvalue = DateADTGetDatum(GetSQLCurrentDate());
+			break;
+		case SVFOP_CURRENT_TIME:
+		case SVFOP_CURRENT_TIME_N:
+			*op->resvalue = TimeTzADTPGetDatum(GetSQLCurrentTime(svf->typmod));
+			break;
+		case SVFOP_CURRENT_TIMESTAMP:
+		case SVFOP_CURRENT_TIMESTAMP_N:
+			*op->resvalue = TimestampTzGetDatum(GetSQLCurrentTimestamp(svf->typmod));
+			break;
+		case SVFOP_LOCALTIME:
+		case SVFOP_LOCALTIME_N:
+			*op->resvalue = TimeADTGetDatum(GetSQLLocalTime(svf->typmod));
+			break;
+		case SVFOP_LOCALTIMESTAMP:
+		case SVFOP_LOCALTIMESTAMP_N:
+			*op->resvalue = TimestampGetDatum(GetSQLLocalTimestamp(svf->typmod));
+			break;
+	}
+}
+
 /*
  * Raise error if a CURRENT OF expression is evaluated.
  *
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 2c3d64ea6e..774db57ae2 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -1549,6 +1549,12 @@ llvm_compile_expr(ExprState *state)
 					break;
 				}
 
+			case EEOP_SQLVALUEFUNCTION:
+				build_EvalXFunc(b, mod, "ExecEvalSQLValueFunction",
+								v_state, op);
+				LLVMBuildBr(b, opblocks[opno + 1]);
+				break;
+
 			case EEOP_CURRENTOFEXPR:
 				build_EvalXFunc(b, mod, "ExecEvalCurrentOfExpr",
 								v_state, op);
diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c
index feb8208b79..41ac4c6f45 100644
--- a/src/backend/jit/llvm/llvmjit_types.c
+++ b/src/backend/jit/llvm/llvmjit_types.c
@@ -126,6 +126,7 @@ void	   *referenced_functions[] =
 	ExecEvalRow,
 	ExecEvalRowNotNull,
 	ExecEvalRowNull,
+	ExecEvalSQLValueFunction,
 	ExecEvalScalarArrayOp,
 	ExecEvalHashedScalarArrayOp,
 	ExecEvalSubPlan,
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index fe3a113c8f..5252efdf21 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -210,6 +210,9 @@ exprType(const Node *expr)
 		case T_MinMaxExpr:
 			type = ((const MinMaxExpr *) expr)->minmaxtype;
 			break;
+		case T_SQLValueFunction:
+			type = ((const SQLValueFunction *) expr)->type;
+			break;
 		case T_XmlExpr:
 			if (((const XmlExpr *) expr)->op == IS_DOCUMENT)
 				type = BOOLOID;
@@ -486,6 +489,8 @@ exprTypmod(const Node *expr)
 				return typmod;
 			}
 			break;
+		case T_SQLValueFunction:
+			return ((const SQLValueFunction *) expr)->typmod;
 		case T_JsonValueExpr:
 			return exprTypmod((Node *) ((const JsonValueExpr *) expr)->formatted_expr);
 		case T_JsonConstructorExpr:
@@ -930,6 +935,10 @@ exprCollation(const Node *expr)
 		case T_MinMaxExpr:
 			coll = ((const MinMaxExpr *) expr)->minmaxcollid;
 			break;
+		case T_SQLValueFunction:
+			/* Returns a non-collatable type */
+			coll = InvalidOid;
+			break;
 		case T_XmlExpr:
 
 			/*
@@ -1167,6 +1176,9 @@ exprSetCollation(Node *expr, Oid collation)
 		case T_MinMaxExpr:
 			((MinMaxExpr *) expr)->minmaxcollid = collation;
 			break;
+		case T_SQLValueFunction:
+			Assert(collation == InvalidOid);
+			break;
 		case T_XmlExpr:
 			Assert((((XmlExpr *) expr)->op == IS_XMLSERIALIZE) ?
 				   (collation == DEFAULT_COLLATION_OID) :
@@ -1468,6 +1480,10 @@ exprLocation(const Node *expr)
 			/* GREATEST/LEAST keyword should always be the first thing */
 			loc = ((const MinMaxExpr *) expr)->location;
 			break;
+		case T_SQLValueFunction:
+			/* function keyword should always be the first thing */
+			loc = ((const SQLValueFunction *) expr)->location;
+			break;
 		case T_XmlExpr:
 			{
 				const XmlExpr *xexpr = (const XmlExpr *) expr;
@@ -1789,10 +1805,10 @@ set_sa_opfuncid(ScalarArrayOpExpr *opexpr)
  * for themselves, in case additional checks should be made, or because they
  * have special rules about which parts of the tree need to be visited.
  *
- * Note: we ignore MinMaxExpr, XmlExpr, CoerceToDomain, and NextValueExpr
- * nodes, because they do not contain SQL function OIDs.  However, they can
- * invoke SQL-visible functions, so callers should take thought about how
- * to treat them.
+ * Note: we ignore MinMaxExpr, SQLValueFunction, XmlExpr, CoerceToDomain,
+ * and NextValueExpr nodes, because they do not contain SQL function OIDs.
+ * However, they can invoke SQL-visible functions, so callers should take
+ * thought about how to treat them.
  */
 bool
 check_functions_in_node(Node *node, check_function_callback checker,
@@ -2008,6 +2024,7 @@ expression_tree_walker_impl(Node *node,
 		case T_Const:
 		case T_Param:
 		case T_CaseTestExpr:
+		case T_SQLValueFunction:
 		case T_CoerceToDomainValue:
 		case T_SetToDefault:
 		case T_CurrentOfExpr:
@@ -2836,6 +2853,7 @@ expression_tree_mutator_impl(Node *node,
 			break;
 		case T_Param:
 		case T_CaseTestExpr:
+		case T_SQLValueFunction:
 		case T_JsonFormat:
 		case T_CoerceToDomainValue:
 		case T_SetToDefault:
@@ -3797,6 +3815,7 @@ raw_expression_tree_walker_impl(Node *node,
 		case T_JsonFormat:
 		case T_SetToDefault:
 		case T_CurrentOfExpr:
+		case T_SQLValueFunction:
 		case T_Integer:
 		case T_Float:
 		case T_Boolean:
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 0a2562c149..e60603df81 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -4606,6 +4606,7 @@ cost_qual_eval_walker(Node *node, cost_qual_eval_context *context)
 		}
 	}
 	else if (IsA(node, MinMaxExpr) ||
+			 IsA(node, SQLValueFunction) ||
 			 IsA(node, XmlExpr) ||
 			 IsA(node, CoerceToDomain) ||
 			 IsA(node, NextValueExpr))
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 11269fee3e..7f453b04f8 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -412,6 +412,12 @@ contain_mutable_functions_walker(Node *node, void *context)
 		/* Check all subnodes */
 	}
 
+	if (IsA(node, SQLValueFunction))
+	{
+		/* all variants of SQLValueFunction are stable */
+		return true;
+	}
+
 	if (IsA(node, NextValueExpr))
 	{
 		/* NextValueExpr is volatile */
@@ -560,8 +566,8 @@ contain_volatile_functions_walker(Node *node, void *context)
 
 	/*
 	 * See notes in contain_mutable_functions_walker about why we treat
-	 * MinMaxExpr, XmlExpr, and CoerceToDomain as immutable.  Hence, none of
-	 * them are of interest here.
+	 * MinMaxExpr, XmlExpr, and CoerceToDomain as immutable, while
+	 * SQLValueFunction is stable.  Hence, none of them are of interest here.
 	 */
 
 	/* Recurse to check arguments */
@@ -606,9 +612,10 @@ contain_volatile_functions_not_nextval_walker(Node *node, void *context)
 
 	/*
 	 * See notes in contain_mutable_functions_walker about why we treat
-	 * MinMaxExpr, XmlExpr, and CoerceToDomain as immutable.  Hence, none of
-	 * them are of interest here.  Also, since we're intentionally ignoring
-	 * nextval(), presumably we should ignore NextValueExpr.
+	 * MinMaxExpr, XmlExpr, and CoerceToDomain as immutable, while
+	 * SQLValueFunction is stable.  Hence, none of them are of interest here.
+	 * Also, since we're intentionally ignoring nextval(), presumably we
+	 * should ignore NextValueExpr.
 	 */
 
 	/* Recurse to check arguments */
@@ -754,8 +761,8 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context)
 	 * (Note: in principle that's wrong because a domain constraint could
 	 * contain a parallel-unsafe function; but useful constraints probably
 	 * never would have such, and assuming they do would cripple use of
-	 * parallel query in the presence of domain types.)  NextValueExpr is
-	 * parallel-unsafe.
+	 * parallel query in the presence of domain types.)  SQLValueFunction
+	 * should be safe in all cases.  NextValueExpr is parallel-unsafe.
 	 */
 	if (IsA(node, CoerceToDomain))
 	{
@@ -1202,6 +1209,7 @@ contain_leaked_vars_walker(Node *node, void *context)
 		case T_CaseExpr:
 		case T_CaseTestExpr:
 		case T_RowExpr:
+		case T_SQLValueFunction:
 		case T_NullTest:
 		case T_BooleanTest:
 		case T_NextValueExpr:
@@ -3243,6 +3251,23 @@ eval_const_expressions_mutator(Node *node,
 				newcoalesce->location = coalesceexpr->location;
 				return (Node *) newcoalesce;
 			}
+		case T_SQLValueFunction:
+			{
+				/*
+				 * All variants of SQLValueFunction are stable, so if we are
+				 * estimating the expression's value, we should evaluate the
+				 * current function value.  Otherwise just copy.
+				 */
+				SQLValueFunction *svf = (SQLValueFunction *) node;
+
+				if (context->estimate)
+					return (Node *) evaluate_expr((Expr *) svf,
+												  svf->type,
+												  svf->typmod,
+												  InvalidOid);
+				else
+					return copyObject((Node *) svf);
+			}
 		case T_FieldSelect:
 			{
 				/*
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index d6426f3b8e..62f521cf10 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -198,6 +198,8 @@ static Node *makeAndExpr(Node *lexpr, Node *rexpr, int location);
 static Node *makeOrExpr(Node *lexpr, Node *rexpr, int location);
 static Node *makeNotExpr(Node *expr, int location);
 static Node *makeAArrayExpr(List *elements, int location);
+static Node *makeSQLValueFunction(SQLValueFunctionOp op, int32 typmod,
+								  int location);
 static Node *makeXmlExpr(XmlExprOp op, char *name, List *named_args,
 						 List *args, int location);
 static List *mergeTableFuncParameters(List *func_args, List *columns);
@@ -15288,66 +15290,39 @@ func_expr_common_subexpr:
 				}
 			| CURRENT_DATE
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("current_date"),
-											   NIL,
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_CURRENT_DATE, -1, @1);
 				}
 			| CURRENT_TIME
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("current_time"),
-											   NIL,
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_CURRENT_TIME, -1, @1);
 				}
 			| CURRENT_TIME '(' Iconst ')'
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("current_time"),
-											   list_make1(makeIntConst($3, @3)),
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_CURRENT_TIME_N, $3, @1);
 				}
 			| CURRENT_TIMESTAMP
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("current_timestamp"),
-											   NIL,
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_CURRENT_TIMESTAMP, -1, @1);
 				}
 			| CURRENT_TIMESTAMP '(' Iconst ')'
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("current_timestamp"),
-											   list_make1(makeIntConst($3, @3)),
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_CURRENT_TIMESTAMP_N, $3, @1);
 				}
 			| LOCALTIME
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("localtime"),
-											   NIL,
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_LOCALTIME, -1, @1);
 				}
 			| LOCALTIME '(' Iconst ')'
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("localtime"),
-											   list_make1(makeIntConst($3, @3)),
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_LOCALTIME_N, $3, @1);
 				}
 			| LOCALTIMESTAMP
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("localtimestamp"),
-											   NIL,
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_LOCALTIMESTAMP, -1, @1);
 				}
 			| LOCALTIMESTAMP '(' Iconst ')'
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("localtimestamp"),
-											   list_make1(makeIntConst($3, @3)),
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_LOCALTIMESTAMP_N, $3, @1);
 				}
 			| CURRENT_ROLE
 				{
@@ -18519,6 +18494,18 @@ makeAArrayExpr(List *elements, int location)
 	return (Node *) n;
 }
 
+static Node *
+makeSQLValueFunction(SQLValueFunctionOp op, int32 typmod, int location)
+{
+	SQLValueFunction *svf = makeNode(SQLValueFunction);
+
+	svf->op = op;
+	/* svf->type will be filled during parse analysis */
+	svf->typmod = typmod;
+	svf->location = location;
+	return (Node *) svf;
+}
+
 static Node *
 makeXmlExpr(XmlExprOp op, char *name, List *named_args, List *args,
 			int location)
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 64356436ef..f0369eee41 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -64,6 +64,8 @@ static Node *transformArrayExpr(ParseState *pstate, A_ArrayExpr *a,
 static Node *transformRowExpr(ParseState *pstate, RowExpr *r, bool allowDefault);
 static Node *transformCoalesceExpr(ParseState *pstate, CoalesceExpr *c);
 static Node *transformMinMaxExpr(ParseState *pstate, MinMaxExpr *m);
+static Node *transformSQLValueFunction(ParseState *pstate,
+									   SQLValueFunction *svf);
 static Node *transformXmlExpr(ParseState *pstate, XmlExpr *x);
 static Node *transformXmlSerialize(ParseState *pstate, XmlSerialize *xs);
 static Node *transformBooleanTest(ParseState *pstate, BooleanTest *b);
@@ -250,6 +252,11 @@ transformExprRecurse(ParseState *pstate, Node *expr)
 			result = transformMinMaxExpr(pstate, (MinMaxExpr *) expr);
 			break;
 
+		case T_SQLValueFunction:
+			result = transformSQLValueFunction(pstate,
+											   (SQLValueFunction *) expr);
+			break;
+
 		case T_XmlExpr:
 			result = transformXmlExpr(pstate, (XmlExpr *) expr);
 			break;
@@ -2220,6 +2227,51 @@ transformMinMaxExpr(ParseState *pstate, MinMaxExpr *m)
 	return (Node *) newm;
 }
 
+static Node *
+transformSQLValueFunction(ParseState *pstate, SQLValueFunction *svf)
+{
+	/*
+	 * All we need to do is insert the correct result type and (where needed)
+	 * validate the typmod, so we just modify the node in-place.
+	 */
+	switch (svf->op)
+	{
+		case SVFOP_CURRENT_DATE:
+			svf->type = DATEOID;
+			break;
+		case SVFOP_CURRENT_TIME:
+			svf->type = TIMETZOID;
+			break;
+		case SVFOP_CURRENT_TIME_N:
+			svf->type = TIMETZOID;
+			svf->typmod = anytime_typmod_check(true, svf->typmod);
+			break;
+		case SVFOP_CURRENT_TIMESTAMP:
+			svf->type = TIMESTAMPTZOID;
+			break;
+		case SVFOP_CURRENT_TIMESTAMP_N:
+			svf->type = TIMESTAMPTZOID;
+			svf->typmod = anytimestamp_typmod_check(true, svf->typmod);
+			break;
+		case SVFOP_LOCALTIME:
+			svf->type = TIMEOID;
+			break;
+		case SVFOP_LOCALTIME_N:
+			svf->type = TIMEOID;
+			svf->typmod = anytime_typmod_check(false, svf->typmod);
+			break;
+		case SVFOP_LOCALTIMESTAMP:
+			svf->type = TIMESTAMPOID;
+			break;
+		case SVFOP_LOCALTIMESTAMP_N:
+			svf->type = TIMESTAMPOID;
+			svf->typmod = anytimestamp_typmod_check(false, svf->typmod);
+			break;
+	}
+
+	return (Node *) svf;
+}
+
 static Node *
 transformXmlExpr(ParseState *pstate, XmlExpr *x)
 {
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index e77b542fd7..4702f5f79e 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -1876,6 +1876,31 @@ FigureColnameInternal(Node *node, char **name)
 					return 2;
 			}
 			break;
+		case T_SQLValueFunction:
+			/* make these act like a function or variable */
+			switch (((SQLValueFunction *) node)->op)
+			{
+				case SVFOP_CURRENT_DATE:
+					*name = "current_date";
+					return 2;
+				case SVFOP_CURRENT_TIME:
+				case SVFOP_CURRENT_TIME_N:
+					*name = "current_time";
+					return 2;
+				case SVFOP_CURRENT_TIMESTAMP:
+				case SVFOP_CURRENT_TIMESTAMP_N:
+					*name = "current_timestamp";
+					return 2;
+				case SVFOP_LOCALTIME:
+				case SVFOP_LOCALTIME_N:
+					*name = "localtime";
+					return 2;
+				case SVFOP_LOCALTIMESTAMP:
+				case SVFOP_LOCALTIMESTAMP_N:
+					*name = "localtimestamp";
+					return 2;
+			}
+			break;
 		case T_XmlExpr:
 			/* make SQL/XML functions act like a regular function */
 			switch (((XmlExpr *) node)->op)
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index a163fbb4ab..ae0f24de2c 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -46,6 +46,27 @@
 
 /* common code for timetypmodin and timetztypmodin */
 static int32
+anytime_typmodin(bool istz, ArrayType *ta)
+{
+	int32	   *tl;
+	int			n;
+
+	tl = ArrayGetIntegerTypmods(ta, &n);
+
+	/*
+	 * we're not too tense about good error message here because grammar
+	 * shouldn't allow wrong number of modifiers for TIME
+	 */
+	if (n != 1)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("invalid type modifier")));
+
+	return anytime_typmod_check(istz, tl[0]);
+}
+
+/* exported so parse_expr.c can use it */
+int32
 anytime_typmod_check(bool istz, int32 typmod)
 {
 	if (typmod < 0)
@@ -66,26 +87,6 @@ anytime_typmod_check(bool istz, int32 typmod)
 	return typmod;
 }
 
-static int32
-anytime_typmodin(bool istz, ArrayType *ta)
-{
-	int32	   *tl;
-	int			n;
-
-	tl = ArrayGetIntegerTypmods(ta, &n);
-
-	/*
-	 * we're not too tense about good error message here because grammar
-	 * shouldn't allow wrong number of modifiers for TIME
-	 */
-	if (n != 1)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("invalid type modifier")));
-
-	return anytime_typmod_check(istz, tl[0]);
-}
-
 /* common code for timetypmodout and timetztypmodout */
 static char *
 anytime_typmodout(bool istz, int32 typmod)
@@ -301,10 +302,10 @@ EncodeSpecialDate(DateADT dt, char *str)

 /*
- * current_date -- implements CURRENT_DATE
+ * GetSQLCurrentDate -- implements CURRENT_DATE
  */
-Datum
-current_date(PG_FUNCTION_ARGS)
+DateADT
+GetSQLCurrentDate(void)
 {
 	struct pg_tm tm;
 
@@ -330,56 +331,46 @@ current_date(PG_FUNCTION_ARGS)
 		cache_mday = tm.tm_mday;
 	}
 
-	return DateADTGetDatum(cache_date);
+	return cache_date;
 }
 
 /*
- * current_time -- implements CURRENT_TIME, CURRENT_TIME(n)
+ * GetSQLCurrentTime -- implements CURRENT_TIME, CURRENT_TIME(n)
  */
-Datum
-current_time(PG_FUNCTION_ARGS)
+TimeTzADT *
+GetSQLCurrentTime(int32 typmod)
 {
 	TimeTzADT  *result;
 	struct pg_tm tt,
 			   *tm = &tt;
 	fsec_t		fsec;
 	int			tz;
-	int32		typmod = -1;
-
-	if (!PG_ARGISNULL(0))
-		typmod = anytime_typmod_check(true, PG_GETARG_INT32(0));
 
 	GetCurrentTimeUsec(tm, &fsec, &tz);
 
 	result = (TimeTzADT *) palloc(sizeof(TimeTzADT));
 	tm2timetz(tm, fsec, tz, result);
 	AdjustTimeForTypmod(&(result->time), typmod);
-
-	return TimeTzADTPGetDatum(result);
+	return result;
 }
 
 /*
- * sql_localtime -- implements LOCALTIME, LOCALTIME(n)
+ * GetSQLLocalTime -- implements LOCALTIME, LOCALTIME(n)
  */
-Datum
-sql_localtime(PG_FUNCTION_ARGS)
+TimeADT
+GetSQLLocalTime(int32 typmod)
 {
 	TimeADT		result;
 	struct pg_tm tt,
 			   *tm = &tt;
 	fsec_t		fsec;
 	int			tz;
-	int32		typmod = -1;
-
-	if (!PG_ARGISNULL(0))
-		typmod = anytime_typmod_check(false, PG_GETARG_INT32(0));
 
 	GetCurrentTimeUsec(tm, &fsec, &tz);
 
 	tm2time(tm, fsec, &result);
 	AdjustTimeForTypmod(&result, typmod);
-
-	return TimeADTGetDatum(result);
+	return result;
 }

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 60f9d08d5d..b815a9441d 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -8254,6 +8254,7 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags)
 		case T_RowExpr:
 		case T_CoalesceExpr:
 		case T_MinMaxExpr:
+		case T_SQLValueFunction:
 		case T_XmlExpr:
 		case T_NextValueExpr:
 		case T_NullIfExpr:
@@ -9242,6 +9243,49 @@ get_rule_expr(Node *node, deparse_context *context,
 			}
 			break;
 
+		case T_SQLValueFunction:
+			{
+				SQLValueFunction *svf = (SQLValueFunction *) node;
+
+				/*
+				 * Note: this code knows that typmod for time, timestamp, and
+				 * timestamptz just prints as integer.
+				 */
+				switch (svf->op)
+				{
+					case SVFOP_CURRENT_DATE:
+						appendStringInfoString(buf, "CURRENT_DATE");
+						break;
+					case SVFOP_CURRENT_TIME:
+						appendStringInfoString(buf, "CURRENT_TIME");
+						break;
+					case SVFOP_CURRENT_TIME_N:
+						appendStringInfo(buf, "CURRENT_TIME(%d)", svf->typmod);
+						break;
+					case SVFOP_CURRENT_TIMESTAMP:
+						appendStringInfoString(buf, "CURRENT_TIMESTAMP");
+						break;
+					case SVFOP_CURRENT_TIMESTAMP_N:
+						appendStringInfo(buf, "CURRENT_TIMESTAMP(%d)",
+										 svf->typmod);
+						break;
+					case SVFOP_LOCALTIME:
+						appendStringInfoString(buf, "LOCALTIME");
+						break;
+					case SVFOP_LOCALTIME_N:
+						appendStringInfo(buf, "LOCALTIME(%d)", svf->typmod);
+						break;
+					case SVFOP_LOCALTIMESTAMP:
+						appendStringInfoString(buf, "LOCALTIMESTAMP");
+						break;
+					case SVFOP_LOCALTIMESTAMP_N:
+						appendStringInfo(buf, "LOCALTIMESTAMP(%d)",
+										 svf->typmod);
+						break;
+				}
+			}
+			break;
+
 		case T_XmlExpr:
 			{
 				XmlExpr    *xexpr = (XmlExpr *) node;
@@ -9816,6 +9860,7 @@ looks_like_function(Node *node)
 		case T_NullIfExpr:
 		case T_CoalesceExpr:
 		case T_MinMaxExpr:
+		case T_SQLValueFunction:
 		case T_XmlExpr:
 			/* these are all accepted by func_expr_common_subexpr */
 			return true;
@@ -10217,33 +10262,6 @@ get_windowfunc_expr_helper(WindowFunc *wfunc, deparse_context *context,
 	}
 }
 
-/*
- * get_func_sql_syntax_time
- *
- * Parse back argument of SQL-syntax function call related to a time or a
- * timestamp.  These require a specific handling when their typmod is given
- * by the function caller through their SQL keyword.
- */
-static void
-get_func_sql_syntax_time(List *args, deparse_context *context)
-{
-	StringInfo	buf = context->buf;
-	Const	   *cons;
-
-	if (list_length(args) != 1)
-		return;
-
-	cons = (Const *) linitial(args);
-	Assert(IsA(cons, Const));
-
-	if (!cons->constisnull)
-	{
-		appendStringInfoString(buf, "(");
-		get_rule_expr((Node *) cons, context, false);
-		appendStringInfoString(buf, ")");
-	}
-}
-
 /*
  * get_func_sql_syntax		- Parse back a SQL-syntax function call
  *
@@ -10492,26 +10510,6 @@ get_func_sql_syntax(FuncExpr *expr, deparse_context *context)
 			appendStringInfoString(buf, "SYSTEM_USER");
 			return true;
 
-		case F_CURRENT_DATE:
-			appendStringInfoString(buf, "CURRENT_DATE");
-			return true;
-		case F_CURRENT_TIME:
-			appendStringInfoString(buf, "CURRENT_TIME");
-			get_func_sql_syntax_time(expr->args, context);
-			return true;
-		case F_CURRENT_TIMESTAMP:
-			appendStringInfoString(buf, "CURRENT_TIMESTAMP");
-			get_func_sql_syntax_time(expr->args, context);
-			return true;
-		case F_LOCALTIME:
-			appendStringInfoString(buf, "LOCALTIME");
-			get_func_sql_syntax_time(expr->args, context);
-			return true;
-		case F_LOCALTIMESTAMP:
-			appendStringInfoString(buf, "LOCALTIMESTAMP");
-			get_func_sql_syntax_time(expr->args, context);
-			return true;
-
 		case F_XMLEXISTS:
 			/* XMLEXISTS ... extra parens because args are c_expr */
 			appendStringInfoString(buf, "XMLEXISTS((");
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index aaadc68ae6..0e50aaec5a 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -83,6 +83,27 @@ static Timestamp timestamptz2timestamp(TimestampTz timestamp);
 
 /* common code for timestamptypmodin and timestamptztypmodin */
 static int32
+anytimestamp_typmodin(bool istz, ArrayType *ta)
+{
+	int32	   *tl;
+	int			n;
+
+	tl = ArrayGetIntegerTypmods(ta, &n);
+
+	/*
+	 * we're not too tense about good error message here because grammar
+	 * shouldn't allow wrong number of modifiers for TIMESTAMP
+	 */
+	if (n != 1)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("invalid type modifier")));
+
+	return anytimestamp_typmod_check(istz, tl[0]);
+}
+
+/* exported so parse_expr.c can use it */
+int32
 anytimestamp_typmod_check(bool istz, int32 typmod)
 {
 	if (typmod < 0)
@@ -103,26 +124,6 @@ anytimestamp_typmod_check(bool istz, int32 typmod)
 	return typmod;
 }
 
-static int32
-anytimestamp_typmodin(bool istz, ArrayType *ta)
-{
-	int32	   *tl;
-	int			n;
-
-	tl = ArrayGetIntegerTypmods(ta, &n);
-
-	/*
-	 * we're not too tense about good error message here because grammar
-	 * shouldn't allow wrong number of modifiers for TIMESTAMP
-	 */
-	if (n != 1)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("invalid type modifier")));
-
-	return anytimestamp_typmod_check(istz, tl[0]);
-}
-
 /* common code for timestamptypmodout and timestamptztypmodout */
 static char *
 anytimestamp_typmodout(bool istz, int32 typmod)
@@ -1594,42 +1595,33 @@ GetCurrentTimestamp(void)
 }
 
 /*
- * current_timestamp -- implements CURRENT_TIMESTAMP, CURRENT_TIMESTAMP(n)
+ * GetSQLCurrentTimestamp -- implements CURRENT_TIMESTAMP, CURRENT_TIMESTAMP(n)
  */
-Datum
-current_timestamp(PG_FUNCTION_ARGS)
+TimestampTz
+GetSQLCurrentTimestamp(int32 typmod)
 {
 	TimestampTz ts;
-	int32		typmod = -1;
-
-	if (!PG_ARGISNULL(0))
-		typmod = anytimestamp_typmod_check(true, PG_GETARG_INT32(0));
 
 	ts = GetCurrentTransactionStartTimestamp();
 	if (typmod >= 0)
 		AdjustTimestampForTypmod(&ts, typmod, NULL);
-	return TimestampTzGetDatum(ts);
+	return ts;
 }
 
 /*
- * sql_localtimestamp -- implements LOCALTIMESTAMP, LOCALTIMESTAMP(n)
+ * GetSQLLocalTimestamp -- implements LOCALTIMESTAMP, LOCALTIMESTAMP(n)
  */
-Datum
-sql_localtimestamp(PG_FUNCTION_ARGS)
+Timestamp
+GetSQLLocalTimestamp(int32 typmod)
 {
 	Timestamp	ts;
-	int32		typmod = -1;
-
-	if (!PG_ARGISNULL(0))
-		typmod = anytimestamp_typmod_check(false, PG_GETARG_INT32(0));
 
 	ts = timestamptz2timestamp(GetCurrentTransactionStartTimestamp());
 	if (typmod >= 0)
 		AdjustTimestampForTypmod(&ts, typmod, NULL);
-	return TimestampGetDatum(ts);
+	return ts;
 }
 
-
 /*
  * timeofday(*) -- returns the current time as a text.
  */
diff --git a/src/test/regress/expected/expressions.out b/src/test/regress/expected/expressions.out
index d2c6db1bd5..caeeb19674 100644
--- a/src/test/regress/expected/expressions.out
+++ b/src/test/regress/expected/expressions.out
@@ -2,7 +2,7 @@
 -- expression evaluation tests that don't fit into a more specific file
 --
 --
--- Tests for various FuncCalls with COERCE_SQL_SYNTAX.
+-- Tests for SQLValueFunction
 --
 -- current_date  (always matches because of transactional behaviour)
 SELECT date(now())::text = current_date::text;
diff --git a/src/test/regress/sql/expressions.sql b/src/test/regress/sql/expressions.sql
index d315ef5af5..e02c21f336 100644
--- a/src/test/regress/sql/expressions.sql
+++ b/src/test/regress/sql/expressions.sql
@@ -3,7 +3,7 @@
 --
 
 --
--- Tests for various FuncCalls with COERCE_SQL_SYNTAX.
+-- Tests for SQLValueFunction
 --

diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index b4058b88c3..06da7cd428 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2425,6 +2425,8 @@ SQLFunctionCache
 SQLFunctionCachePtr
 SQLFunctionParseInfo
 SQLFunctionParseInfoPtr
+SQLValueFunction
+SQLValueFunctionOp
 SSL
 SSLExtensionInfoContext
 SSL_CTX
-- 
2.40.1

From 5ac3c30fc1f9b7c265f33dae7999b56ea5c776e8 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 11 May 2023 14:39:22 +0900
Subject: [PATCH v1 2/2] Revert "Switch SQLValueFunction on "name" to use
 COERCE_SQL_SYNTAX"

This reverts commit fb32748e32e2b6b2fcb32220980b93d5436f855e.
---
 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, 92 insertions(+), 55 deletions(-)

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 0a0f749385..d7a60b39b5 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -1505,15 +1505,6 @@
 { 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 7ade0d83f7..08e7dae73f 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1466,7 +1466,13 @@ typedef enum SQLValueFunctionOp
 	SVFOP_LOCALTIME,
 	SVFOP_LOCALTIME_N,
 	SVFOP_LOCALTIMESTAMP,
-	SVFOP_LOCALTIMESTAMP_N
+	SVFOP_LOCALTIMESTAMP_N,
+	SVFOP_CURRENT_ROLE,
+	SVFOP_CURRENT_USER,
+	SVFOP_USER,
+	SVFOP_SESSION_USER,
+	SVFOP_CURRENT_CATALOG,
+	SVFOP_CURRENT_SCHEMA
 } SQLValueFunctionOp;
 
 typedef struct SQLValueFunction
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index d98710bfb3..7cc443ec52 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -2515,10 +2515,15 @@ 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:
@@ -2540,6 +2545,28 @@ 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 5252efdf21..0ed8712a63 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -936,8 +936,11 @@ exprCollation(const Node *expr)
 			coll = ((const MinMaxExpr *) expr)->minmaxcollid;
 			break;
 		case T_SQLValueFunction:
-			/* Returns a non-collatable type */
-			coll = InvalidOid;
+			/* Returns either NAME or a non-collatable type */
+			if (((const SQLValueFunction *) expr)->type == NAMEOID)
+				coll = C_COLLATION_OID;
+			else
+				coll = InvalidOid;
 			break;
 		case T_XmlExpr:
 
@@ -1177,7 +1180,9 @@ exprSetCollation(Node *expr, Oid collation)
 			((MinMaxExpr *) expr)->minmaxcollid = collation;
 			break;
 		case T_SQLValueFunction:
-			Assert(collation == InvalidOid);
+			Assert((((SQLValueFunction *) expr)->type == NAMEOID) ?
+				   (collation == C_COLLATION_OID) :
+				   (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 62f521cf10..6f5aa8a3cb 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -15326,24 +15326,15 @@ func_expr_common_subexpr:
 				}
 			| CURRENT_ROLE
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("current_role"),
-											   NIL,
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_CURRENT_ROLE, -1, @1);
 				}
 			| CURRENT_USER
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("current_user"),
-											   NIL,
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_CURRENT_USER, -1, @1);
 				}
 			| SESSION_USER
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("session_user"),
-											   NIL,
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_SESSION_USER, -1, @1);
 				}
 			| SYSTEM_USER
 				{
@@ -15354,24 +15345,15 @@ func_expr_common_subexpr:
 				}
 			| USER
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("user"),
-											   NIL,
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_USER, -1, @1);
 				}
 			| CURRENT_CATALOG
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("current_catalog"),
-											   NIL,
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_CURRENT_CATALOG, -1, @1);
 				}
 			| CURRENT_SCHEMA
 				{
-					$$ = (Node *) makeFuncCall(SystemFuncName("current_schema"),
-											   NIL,
-											   COERCE_SQL_SYNTAX,
-											   @1);
+					$$ = makeSQLValueFunction(SVFOP_CURRENT_SCHEMA, -1, @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 f0369eee41..0b3632735b 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -2267,6 +2267,14 @@ 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 4702f5f79e..4cca97ff9c 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -1899,6 +1899,24 @@ 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 b815a9441d..e93d66a7ec 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -9282,6 +9282,24 @@ 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;
@@ -10488,24 +10506,6 @@ 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.40.1

---

From: Alexander Lakhin <[email-masked]>
Date: Tue, 16 May 2023 18:00:00 +0300

15.05.2023 08:20, Michael Paquier wrote:
> (these would be combined, just posted this way for clarity).

I can confirm that the patches improve (restore) performance for my test:
pgbench -i benchdb
pgbench -c 10 -T 300 benchdb

tps (over three runs):
HEAD (08c45ae23):
10238.441580, 10697.202119, 10706.764703

HEAD with the patches:
11134.510118, 11176.554996, 11150.338488

f193883fc~1 (240e0dbac)
11082.561388, 11233.604446, 11087.071768

15.3 (8382864eb)
11328.699555, 11128.057883, 11057.934392

Best regards,
Alexander

---

From: Michael Paquier <[email-masked]>
Date: Wed, 17 May 2023 10:25:04 +0900

> pgbench -c 10 -T 300 benchdb

Thanks for running these!

> HEAD (08c45ae23):

The numbers between f193883fc~1 and HEAD+patch are close to each
other.  It does not seem to make the whole difference with 15.3, but
most of it.  The difference can also be explained with some noise,
based on the number patterns of the third runs?

I have now applied the revert, ready for beta1.  Thanks for the
feedback!
--
Michael

---

From: Alexander Lakhin <[email-masked]>
Date: Wed, 17 May 2023 13:00:00 +0300

> I have now applied the revert, ready for beta1.  Thanks for the

Thank you for paying attention to it!

Yes, I ran the benchmark on my workstation, so numbers could vary due parallel
activity. Now I've compared 15.3 (8382864eb) with d8c3106bb and 1d369c9e9,
this time with the CPU boost mode disabled:
1d369c9e9:
10007.130326, 10047.722122, 9920.612426, 10016.053846, 10060.606408
d8c3106bb:
10492.100485, 10505.326827, 10535.918137, 10625.904871, 10573.608859
15.3:
10458.752330, 10308.677192, 10366.496526, 10489.395275, 10319.458041

Best "1d369c9e9" worse than "15.3" by 4.1 percents (10060.61 < 10489.40)
Average "1d369c9e9" worse than "15.3" by 3.6 percents (10010.43 < 10388.56)

Best "d8c3106bb" better than "15.3" by 1.3 percents (10625.90 > 10489.40)
Average "d8c3106bb" better than "15.3" by 1.5 percents (10546.57 > 10388.56)

So it seems that there is nothing left on this plate.

Best regards,
Alexander