On Sun, Nov 20, 2022 at 08:21:10AM -0800, Ted Yu wrote: > For get_func_sql_syntax(), the code for cases > of F_CURRENT_TIME, F_CURRENT_TIMESTAMP, F_LOCALTIME and F_LOCALTIMESTAMP is > mostly the same. > Maybe we can introduce a helper so that code duplication is reduced.
It would. Thanks for the suggestion. Do you like something like the patch 0002 attached? This reduces a bit the overall size of the patch. Both ought to be merged in the same commit, still it is easier to see the simplification created. -- Michael
From 3611cfa6f0171dcd65eeb88461f4c48989cd3f1a Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Sun, 20 Nov 2022 11:53:28 +0900 Subject: [PATCH v5 1/2] Replace SQLValueFunction by direct function calls This impacts 9 patterns where the SQL grammar takes priority: - localtime, that can take a typmod. - localtimestamp, that can take a typmod. - current_time, that can take a typmod. - current_timestamp, that can take a typmod. - current_date. Five new entries are added to pg_proc to compensate the removal of SQLValueFunction to keep compatibility (when a keyword is specified in a SELECT or in a FROM clause without an alias), with tests to cover all that. XXX: bump catalog version. Reviewed-by: Corey Huinker Discussion: https://postgr.es/m/yzag3morycguu...@paquier.xyz --- src/include/catalog/pg_proc.dat | 15 +++ src/include/executor/execExpr.h | 8 -- src/include/nodes/primnodes.h | 33 ------- 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 | 81 +++++++++------- src/backend/utils/adt/ruleutils.c | 109 +++++++++++++--------- src/backend/utils/adt/timestamp.c | 72 ++++++++------ src/backend/utils/misc/queryjumble.c | 9 -- src/test/regress/expected/expressions.out | 2 +- src/test/regress/sql/expressions.sql | 2 +- src/tools/pgindent/typedefs.list | 2 - 23 files changed, 245 insertions(+), 389 deletions(-) diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index fd2559442e..35dc369728 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -1520,6 +1520,21 @@ { 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', provolatile => 's', prorettype => 'timetz', + proargtypes => 'int4', prosrc => 'current_time', proisstrict => 'f' }, +{ oid => '9980', descr => 'current timestamp', + proname => 'current_timestamp', provolatile => 's', prorettype => 'timestamptz', + proargtypes => 'int4', prosrc => 'current_timestamp', proisstrict => 'f' }, +{ oid => '9981', descr => 'local time', + proname => 'localtime', provolatile => 's', prorettype => 'time', + proargtypes => 'int4', prosrc => 'sql_localtime', proisstrict => 'f' }, +{ oid => '9982', descr => 'local timestamp', + proname => 'localtimestamp', provolatile => 's', prorettype => 'timestamp', + proargtypes => 'int4', prosrc => 'sql_localtimestamp', proisstrict => 'f' }, { oid => '744', proname => 'array_eq', prorettype => 'bool', diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index e14f15d435..0557302b92 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -170,7 +170,6 @@ typedef enum ExprEvalOp EEOP_DISTINCT, EEOP_NOT_DISTINCT, EEOP_NULLIF, - EEOP_SQLVALUEFUNCTION, EEOP_CURRENTOFEXPR, EEOP_NEXTVALUEEXPR, EEOP_ARRAYEXPR, @@ -416,12 +415,6 @@ typedef struct ExprEvalStep FunctionCallInfo fcinfo_data_in; } iocoerce; - /* for EEOP_SQLVALUEFUNCTION */ - struct - { - SQLValueFunction *svf; - } sqlvaluefunction; - /* for EEOP_NEXTVALUEEXPR */ struct { @@ -741,7 +734,6 @@ 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 f6dd27edcc..74f228d959 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1292,39 +1292,6 @@ typedef struct MinMaxExpr int location; /* token location, or -1 if unknown */ } 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 */ - Oid type; /* result type/typmod */ - 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 0bbe889128..fad4878722 100644 --- a/src/include/utils/date.h +++ b/src/include/utils/date.h @@ -96,7 +96,6 @@ 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); @@ -104,9 +103,6 @@ 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 76b7b4a3ca..7fd0b58825 100644 --- a/src/include/utils/timestamp.h +++ b/src/include/utils/timestamp.h @@ -93,11 +93,7 @@ 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 30a048f6b0..4c3b4c3f8e 100644 --- a/src/backend/catalog/system_functions.sql +++ b/src/backend/catalog/system_functions.sql @@ -594,6 +594,32 @@ 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 0ecb2f8610..81429b9f05 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -2210,17 +2210,6 @@ 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 6ebf5c287e..1dab2787b7 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -452,7 +452,6 @@ 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, @@ -1301,17 +1300,6 @@ 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 */ @@ -2489,40 +2477,6 @@ 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 95d0807bdd..f114337f8e 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -1549,12 +1549,6 @@ 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 90ac6b83b5..5b416c5642 100644 --- a/src/backend/jit/llvm/llvmjit_types.c +++ b/src/backend/jit/llvm/llvmjit_types.c @@ -126,7 +126,6 @@ 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 2585a3175c..af8620ceb7 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -210,9 +210,6 @@ 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; @@ -474,8 +471,6 @@ exprTypmod(const Node *expr) return typmod; } break; - case T_SQLValueFunction: - return ((const SQLValueFunction *) expr)->typmod; case T_CoerceToDomain: return ((const CoerceToDomain *) expr)->resulttypmod; case T_CoerceToDomainValue: @@ -916,10 +911,6 @@ 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: /* @@ -1140,9 +1131,6 @@ 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) : @@ -1426,10 +1414,6 @@ 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; @@ -1717,10 +1701,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, 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. + * 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. */ bool check_functions_in_node(Node *node, check_function_callback checker, @@ -1936,7 +1920,6 @@ 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: @@ -2673,7 +2656,6 @@ expression_tree_mutator_impl(Node *node, break; case T_Param: case T_CaseTestExpr: - case T_SQLValueFunction: case T_CoerceToDomainValue: case T_SetToDefault: case T_CurrentOfExpr: @@ -3587,7 +3569,6 @@ raw_expression_tree_walker_impl(Node *node, { 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 4c6b1d1f55..897309d7ec 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -4603,7 +4603,6 @@ 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 33790a4f46..c61d9c68a0 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -383,12 +383,6 @@ contain_mutable_functions_walker(Node *node, void *context) context)) return true; - if (IsA(node, SQLValueFunction)) - { - /* all variants of SQLValueFunction are stable */ - return true; - } - if (IsA(node, NextValueExpr)) { /* NextValueExpr is volatile */ @@ -537,8 +531,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, while - * SQLValueFunction is stable. Hence, none of them are of interest here. + * MinMaxExpr, XmlExpr, and CoerceToDomain as immutable. Hence, none + * of them are of interest here. */ /* Recurse to check arguments */ @@ -583,10 +577,9 @@ 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, while - * SQLValueFunction is stable. 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. Hence, none + * of them are of interest here. Also, since we're intentionally + * ignoring nextval(), presumably we should ignore NextValueExpr. */ /* Recurse to check arguments */ @@ -732,8 +725,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.) SQLValueFunction - * should be safe in all cases. NextValueExpr is parallel-unsafe. + * parallel query in the presence of domain types.) NextValueExpr is + * parallel-unsafe. */ if (IsA(node, CoerceToDomain)) { @@ -1180,7 +1173,6 @@ 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: @@ -3194,23 +3186,6 @@ 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 9054742427..9384214942 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -198,8 +198,6 @@ 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); @@ -15195,39 +15193,66 @@ func_expr_common_subexpr: } | CURRENT_DATE { - $$ = makeSQLValueFunction(SVFOP_CURRENT_DATE, -1, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("current_date"), + NIL, + COERCE_SQL_SYNTAX, + @1); } | CURRENT_TIME { - $$ = makeSQLValueFunction(SVFOP_CURRENT_TIME, -1, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("current_time"), + NIL, + COERCE_SQL_SYNTAX, + @1); } | CURRENT_TIME '(' Iconst ')' { - $$ = makeSQLValueFunction(SVFOP_CURRENT_TIME_N, $3, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("current_time"), + list_make1(makeIntConst($3, @3)), + COERCE_SQL_SYNTAX, + @1); } | CURRENT_TIMESTAMP { - $$ = makeSQLValueFunction(SVFOP_CURRENT_TIMESTAMP, -1, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("current_timestamp"), + NIL, + COERCE_SQL_SYNTAX, + @1); } | CURRENT_TIMESTAMP '(' Iconst ')' { - $$ = makeSQLValueFunction(SVFOP_CURRENT_TIMESTAMP_N, $3, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("current_timestamp"), + list_make1(makeIntConst($3, @3)), + COERCE_SQL_SYNTAX, + @1); } | LOCALTIME { - $$ = makeSQLValueFunction(SVFOP_LOCALTIME, -1, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("localtime"), + NIL, + COERCE_SQL_SYNTAX, + @1); } | LOCALTIME '(' Iconst ')' { - $$ = makeSQLValueFunction(SVFOP_LOCALTIME_N, $3, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("localtime"), + list_make1(makeIntConst($3, @3)), + COERCE_SQL_SYNTAX, + @1); } | LOCALTIMESTAMP { - $$ = makeSQLValueFunction(SVFOP_LOCALTIMESTAMP, -1, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("localtimestamp"), + NIL, + COERCE_SQL_SYNTAX, + @1); } | LOCALTIMESTAMP '(' Iconst ')' { - $$ = makeSQLValueFunction(SVFOP_LOCALTIMESTAMP_N, $3, @1); + $$ = (Node *) makeFuncCall(SystemFuncName("localtimestamp"), + list_make1(makeIntConst($3, @3)), + COERCE_SQL_SYNTAX, + @1); } | CURRENT_ROLE { @@ -18166,18 +18191,6 @@ 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 0fdbf82f3a..150a8099c2 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -61,8 +61,6 @@ 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); @@ -240,11 +238,6 @@ 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; @@ -2191,51 +2184,6 @@ 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 f54591a987..8e0d6fd01f 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -1872,31 +1872,6 @@ 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 a2bdde0459..10c11e00db 100644 --- a/src/backend/utils/adt/date.c +++ b/src/backend/utils/adt/date.c @@ -46,27 +46,6 @@ /* 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) @@ -87,6 +66,26 @@ 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) @@ -296,10 +295,10 @@ EncodeSpecialDate(DateADT dt, char *str) /* - * GetSQLCurrentDate -- implements CURRENT_DATE + * current_date -- implements CURRENT_DATE */ -DateADT -GetSQLCurrentDate(void) +Datum +current_date(PG_FUNCTION_ARGS) { struct pg_tm tm; @@ -325,46 +324,62 @@ GetSQLCurrentDate(void) cache_mday = tm.tm_mday; } - return cache_date; + return DateADTGetDatum(cache_date); } /* - * GetSQLCurrentTime -- implements CURRENT_TIME, CURRENT_TIME(n) + * current_time -- implements CURRENT_TIME, CURRENT_TIME(n) */ -TimeTzADT * -GetSQLCurrentTime(int32 typmod) +Datum +current_time(PG_FUNCTION_ARGS) { TimeTzADT *result; struct pg_tm tt, *tm = &tt; fsec_t fsec; int tz; + int32 typmod = -1; + + if (!PG_ARGISNULL(0)) + { + typmod = PG_GETARG_INT32(0); + anytime_typmod_check(true, typmod); + } GetCurrentTimeUsec(tm, &fsec, &tz); result = (TimeTzADT *) palloc(sizeof(TimeTzADT)); tm2timetz(tm, fsec, tz, result); AdjustTimeForTypmod(&(result->time), typmod); - return result; + + return TimeTzADTPGetDatum(result); } /* - * GetSQLLocalTime -- implements LOCALTIME, LOCALTIME(n) + * sql_localtime -- implements LOCALTIME, LOCALTIME(n) */ -TimeADT -GetSQLLocalTime(int32 typmod) +Datum +sql_localtime(PG_FUNCTION_ARGS) { TimeADT result; struct pg_tm tt, *tm = &tt; fsec_t fsec; int tz; + int32 typmod = -1; + + if (!PG_ARGISNULL(0)) + { + typmod = PG_GETARG_INT32(0); + anytime_typmod_check(false, typmod); + } GetCurrentTimeUsec(tm, &fsec, &tz); tm2time(tm, fsec, &result); AdjustTimeForTypmod(&result, typmod); - return result; + + return TimeADTGetDatum(result); } diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 129f3333fb..c00dc5e32e 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -8150,7 +8150,6 @@ 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: @@ -9130,49 +9129,6 @@ 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; @@ -9698,7 +9654,6 @@ 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; @@ -10292,6 +10247,70 @@ 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"); + if (list_length(expr->args) == 1) + { + Const *cons = (Const *) linitial(expr->args); + + Assert(IsA(cons, Const)); + if (!cons->constisnull) + { + appendStringInfoString(buf, "("); + get_rule_expr((Node *) cons, context, false); + appendStringInfoString(buf, ")"); + } + } + return true; + case F_CURRENT_TIMESTAMP: + appendStringInfoString(buf, "CURRENT_TIMESTAMP"); + if (list_length(expr->args) == 1) + { + Const *cons = (Const *) linitial(expr->args); + + Assert(IsA(cons, Const)); + if (!cons->constisnull) + { + appendStringInfoString(buf, "("); + get_rule_expr((Node *) cons, context, false); + appendStringInfoString(buf, ")"); + } + } + return true; + case F_LOCALTIME: + appendStringInfoString(buf, "LOCALTIME"); + if (list_length(expr->args) == 1) + { + Const *cons = (Const *) linitial(expr->args); + + Assert(IsA(cons, Const)); + if (!cons->constisnull) + { + appendStringInfoString(buf, "("); + get_rule_expr((Node *) cons, context, false); + appendStringInfoString(buf, ")"); + } + } + return true; + case F_LOCALTIMESTAMP: + appendStringInfoString(buf, "LOCALTIMESTAMP"); + if (list_length(expr->args) == 1) + { + Const *cons = (Const *) linitial(expr->args); + + Assert(IsA(cons, Const)); + if (!cons->constisnull) + { + appendStringInfoString(buf, "("); + get_rule_expr((Node *) cons, context, false); + appendStringInfoString(buf, ")"); + } + } + 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 d8552a1f18..ef92323fd0 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -81,27 +81,6 @@ 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) @@ -122,6 +101,26 @@ 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) @@ -1586,33 +1585,48 @@ GetCurrentTimestamp(void) } /* - * GetSQLCurrentTimestamp -- implements CURRENT_TIMESTAMP, CURRENT_TIMESTAMP(n) + * current_timestamp -- implements CURRENT_TIMESTAMP, CURRENT_TIMESTAMP(n) */ -TimestampTz -GetSQLCurrentTimestamp(int32 typmod) +Datum +current_timestamp(PG_FUNCTION_ARGS) { TimestampTz ts; + int32 typmod = -1; + + if (!PG_ARGISNULL(0)) + { + typmod = PG_GETARG_INT32(0); + anytimestamp_typmod_check(true, typmod); + } ts = GetCurrentTransactionStartTimestamp(); if (typmod >= 0) AdjustTimestampForTypmod(&ts, typmod); - return ts; + return TimestampTzGetDatum(ts); } /* - * GetSQLLocalTimestamp -- implements LOCALTIMESTAMP, LOCALTIMESTAMP(n) + * sql_localtimestamp -- implements LOCALTIMESTAMP, LOCALTIMESTAMP(n) */ -Timestamp -GetSQLLocalTimestamp(int32 typmod) +Datum +sql_localtimestamp(PG_FUNCTION_ARGS) { Timestamp ts; + int32 typmod = -1; + + if (!PG_ARGISNULL(0)) + { + typmod = PG_GETARG_INT32(0); + anytimestamp_typmod_check(false, typmod); + } ts = timestamptz2timestamp(GetCurrentTransactionStartTimestamp()); if (typmod >= 0) AdjustTimestampForTypmod(&ts, typmod); - return ts; + return TimestampGetDatum(ts); } + /* * timeofday(*) -- returns the current time as a text. */ diff --git a/src/backend/utils/misc/queryjumble.c b/src/backend/utils/misc/queryjumble.c index a8508463e7..0ace74de78 100644 --- a/src/backend/utils/misc/queryjumble.c +++ b/src/backend/utils/misc/queryjumble.c @@ -606,15 +606,6 @@ JumbleExpr(JumbleState *jstate, Node *node) JumbleExpr(jstate, (Node *) mmexpr->args); } break; - case T_SQLValueFunction: - { - SQLValueFunction *svf = (SQLValueFunction *) node; - - APP_JUMB(svf->op); - /* type is fully determined by op */ - APP_JUMB(svf->typmod); - } - break; case T_XmlExpr: { XmlExpr *xexpr = (XmlExpr *) node; diff --git a/src/test/regress/expected/expressions.out b/src/test/regress/expected/expressions.out index 5bf39fd9aa..28a20900f1 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 SQLValueFunction +-- Tests for various FuncCalls with COERCE_SQL_SYNTAX. -- -- 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 0e163cc0d7..f9a0299d17 100644 --- a/src/test/regress/sql/expressions.sql +++ b/src/test/regress/sql/expressions.sql @@ -3,7 +3,7 @@ -- -- --- Tests for SQLValueFunction +-- Tests for various FuncCalls with COERCE_SQL_SYNTAX. -- diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index f8302f1ed1..2f5802195d 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2397,8 +2397,6 @@ SQLFunctionCache SQLFunctionCachePtr SQLFunctionParseInfo SQLFunctionParseInfoPtr -SQLValueFunction -SQLValueFunctionOp SSL SSLExtensionInfoContext SSL_CTX -- 2.38.1
From 5ec10492cda8930c014f8db315650cffa7ea57e1 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 21 Nov 2022 08:08:44 +0900
Subject: [PATCH v5 2/2] Simplify back-parsing of SQL-callable timestamp
functions
Per suggestion from Ted Yu.
---
src/backend/utils/adt/ruleutils.c | 79 ++++++++++++-------------------
1 file changed, 31 insertions(+), 48 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index c00dc5e32e..ab926bedd4 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -9997,6 +9997,33 @@ get_windowfunc_expr(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 with 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
*
@@ -10252,63 +10279,19 @@ get_func_sql_syntax(FuncExpr *expr, deparse_context *context)
return true;
case F_CURRENT_TIME:
appendStringInfoString(buf, "CURRENT_TIME");
- if (list_length(expr->args) == 1)
- {
- Const *cons = (Const *) linitial(expr->args);
-
- Assert(IsA(cons, Const));
- if (!cons->constisnull)
- {
- appendStringInfoString(buf, "(");
- get_rule_expr((Node *) cons, context, false);
- appendStringInfoString(buf, ")");
- }
- }
+ get_func_sql_syntax_time(expr->args, context);
return true;
case F_CURRENT_TIMESTAMP:
appendStringInfoString(buf, "CURRENT_TIMESTAMP");
- if (list_length(expr->args) == 1)
- {
- Const *cons = (Const *) linitial(expr->args);
-
- Assert(IsA(cons, Const));
- if (!cons->constisnull)
- {
- appendStringInfoString(buf, "(");
- get_rule_expr((Node *) cons, context, false);
- appendStringInfoString(buf, ")");
- }
- }
+ get_func_sql_syntax_time(expr->args, context);
return true;
case F_LOCALTIME:
appendStringInfoString(buf, "LOCALTIME");
- if (list_length(expr->args) == 1)
- {
- Const *cons = (Const *) linitial(expr->args);
-
- Assert(IsA(cons, Const));
- if (!cons->constisnull)
- {
- appendStringInfoString(buf, "(");
- get_rule_expr((Node *) cons, context, false);
- appendStringInfoString(buf, ")");
- }
- }
+ get_func_sql_syntax_time(expr->args, context);
return true;
case F_LOCALTIMESTAMP:
appendStringInfoString(buf, "LOCALTIMESTAMP");
- if (list_length(expr->args) == 1)
- {
- Const *cons = (Const *) linitial(expr->args);
-
- Assert(IsA(cons, Const));
- if (!cons->constisnull)
- {
- appendStringInfoString(buf, "(");
- get_rule_expr((Node *) cons, context, false);
- appendStringInfoString(buf, ")");
- }
- }
+ get_func_sql_syntax_time(expr->args, context);
return true;
case F_XMLEXISTS:
--
2.38.1
signature.asc
Description: PGP signature
