avoid MERGE_ACTION keyword?
Started by Peter Eisentrautover 1 year ago2 messages
I wonder if we can avoid making MERGE_ACTION a keyword.
I think we could parse it initially as a function and then transform it
to a more special node later. In the attached patch, I'm doing this in
parse analysis. We could try to do it even later and actually execute
it as a function, if we could get the proper context passed into it somehow.
I'm thinking about this with half an eye on future features. For
example, row pattern recognition might introduce similar magic functions
match_number() and classifier() (somewhat the inspiration for the
merge_action() syntax), which could use similar treatment.
Thoughts?
Attachments:
0001-WIP-Avoid-MERGE_ACTION-keyword.patchtext/plain; charset=UTF-8; name=0001-WIP-Avoid-MERGE_ACTION-keyword.patchDownload
From 6e0132ca3f3472fb6c5f8c5b2ec7296de1f83811 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 16 May 2024 16:09:57 +0200
Subject: [PATCH] WIP: Avoid MERGE_ACTION keyword
---
src/backend/parser/gram.y | 12 +-----
src/backend/parser/parse_expr.c | 73 ++++++++++++++++++---------------
src/include/catalog/pg_proc.dat | 2 +
src/include/parser/kwlist.h | 1 -
4 files changed, 44 insertions(+), 44 deletions(-)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 4d582950b72..b717b704972 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -751,7 +751,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P LOCKED LOGGED
- MAPPING MATCH MATCHED MATERIALIZED MAXVALUE MERGE MERGE_ACTION METHOD
+ MAPPING MATCH MATCHED MATERIALIZED MAXVALUE MERGE METHOD
MINUTE_P MINVALUE MODE MONTH_P MOVE
NAME_P NAMES NATIONAL NATURAL NCHAR NESTED NEW NEXT NFC NFD NFKC NFKD NO
@@ -16077,14 +16077,6 @@ func_expr_common_subexpr:
n->location = @1;
$$ = (Node *) n;
}
- | MERGE_ACTION '(' ')'
- {
- MergeSupportFunc *m = makeNode(MergeSupportFunc);
-
- m->msftype = TEXTOID;
- m->location = @1;
- $$ = (Node *) m;
- }
| JSON_QUERY '('
json_value_expr ',' a_expr json_passing_clause_opt
json_returning_clause_opt
@@ -17936,7 +17928,6 @@ col_name_keyword:
| JSON_TABLE
| JSON_VALUE
| LEAST
- | MERGE_ACTION
| NATIONAL
| NCHAR
| NONE
@@ -18334,7 +18325,6 @@ bare_label_keyword:
| MATERIALIZED
| MAXVALUE
| MERGE
- | MERGE_ACTION
| METHOD
| MINVALUE
| MODE
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index aba3546ed1a..12fc6829fc0 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -55,7 +55,6 @@ static Node *transformAExprDistinct(ParseState *pstate, A_Expr *a);
static Node *transformAExprNullIf(ParseState *pstate, A_Expr *a);
static Node *transformAExprIn(ParseState *pstate, A_Expr *a);
static Node *transformAExprBetween(ParseState *pstate, A_Expr *a);
-static Node *transformMergeSupportFunc(ParseState *pstate, MergeSupportFunc *f);
static Node *transformBoolExpr(ParseState *pstate, BoolExpr *a);
static Node *transformFuncCall(ParseState *pstate, FuncCall *fn);
static Node *transformMultiAssignRef(ParseState *pstate, MultiAssignRef *maref);
@@ -238,11 +237,6 @@ transformExprRecurse(ParseState *pstate, Node *expr)
result = transformGroupingFunc(pstate, (GroupingFunc *) expr);
break;
- case T_MergeSupportFunc:
- result = transformMergeSupportFunc(pstate,
- (MergeSupportFunc *) expr);
- break;
-
case T_NamedArgExpr:
{
NamedArgExpr *na = (NamedArgExpr *) expr;
@@ -1374,31 +1368,6 @@ transformAExprBetween(ParseState *pstate, A_Expr *a)
return transformExprRecurse(pstate, result);
}
-static Node *
-transformMergeSupportFunc(ParseState *pstate, MergeSupportFunc *f)
-{
- /*
- * All we need to do is check that we're in the RETURNING list of a MERGE
- * command. If so, we just return the node as-is.
- */
- if (pstate->p_expr_kind != EXPR_KIND_MERGE_RETURNING)
- {
- ParseState *parent_pstate = pstate->parentParseState;
-
- while (parent_pstate &&
- parent_pstate->p_expr_kind != EXPR_KIND_MERGE_RETURNING)
- parent_pstate = parent_pstate->parentParseState;
-
- if (!parent_pstate)
- ereport(ERROR,
- errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("MERGE_ACTION() can only be used in the RETURNING list of a MERGE command"),
- parser_errposition(pstate, f->location));
- }
-
- return (Node *) f;
-}
-
static Node *
transformBoolExpr(ParseState *pstate, BoolExpr *a)
{
@@ -1441,6 +1410,7 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
Node *last_srf = pstate->p_last_srf;
List *targs;
ListCell *args;
+ Node *result;
/* Transform the list of arguments ... */
targs = NIL;
@@ -1471,13 +1441,52 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
}
/* ... and hand off to ParseFuncOrColumn */
- return ParseFuncOrColumn(pstate,
+ result = ParseFuncOrColumn(pstate,
fn->funcname,
targs,
last_srf,
fn,
false,
fn->location);
+
+ if (IsA(result, FuncExpr))
+ {
+ FuncExpr *fe = castNode(FuncExpr, result);
+
+ if (fe->funcid == F_MERGE_ACTION)
+ {
+ MergeSupportFunc *m;
+
+ if (pstate->p_expr_kind != EXPR_KIND_MERGE_RETURNING)
+ {
+ ParseState *parent_pstate = pstate->parentParseState;
+
+ while (parent_pstate &&
+ parent_pstate->p_expr_kind != EXPR_KIND_MERGE_RETURNING)
+ parent_pstate = parent_pstate->parentParseState;
+
+ if (!parent_pstate)
+ ereport(ERROR,
+ errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("MERGE_ACTION() can only be used in the RETURNING list of a MERGE command"),
+ parser_errposition(pstate, fe->location));
+ }
+
+ m = makeNode(MergeSupportFunc);
+ m->msftype = TEXTOID;
+ m->location = fe->location;
+
+ return (Node *) m;
+ }
+ }
+
+ return result;
+}
+
+Datum
+merge_support_dummy(PG_FUNCTION_ARGS)
+{
+ return 0;
}
static Node *
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6a5476d3c4c..09dc6b44fd1 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12185,4 +12185,6 @@
proargnames => '{summarized_tli,summarized_lsn,pending_lsn,summarizer_pid}',
prosrc => 'pg_get_wal_summarizer_state' },
+{ oid => 6313, descr => 'MERGE action', proname => 'merge_action', provolatile => 's', prorettype => 'text', proargtypes => '', prosrc => 'merge_support_dummy' },
+
]
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index f7fe834cf45..74c2e506e6b 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -274,7 +274,6 @@ PG_KEYWORD("matched", MATCHED, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("materialized", MATERIALIZED, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("maxvalue", MAXVALUE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("merge", MERGE, UNRESERVED_KEYWORD, BARE_LABEL)
-PG_KEYWORD("merge_action", MERGE_ACTION, COL_NAME_KEYWORD, BARE_LABEL)
PG_KEYWORD("method", METHOD, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("minute", MINUTE_P, UNRESERVED_KEYWORD, AS_LABEL)
PG_KEYWORD("minvalue", MINVALUE, UNRESERVED_KEYWORD, BARE_LABEL)
--
2.44.0
Re: avoid MERGE_ACTION keyword?
On Thu, 16 May 2024 at 15:15, Peter Eisentraut <peter@eisentraut.org> wrote:
I wonder if we can avoid making MERGE_ACTION a keyword.
Yeah, there was a lot of back and forth on this point on the original
thread, and I'm still not sure which approach is best.
I think we could parse it initially as a function and then transform it
to a more special node later. In the attached patch, I'm doing this in
parse analysis. We could try to do it even later and actually execute
it as a function, if we could get the proper context passed into it somehow.
Whichever way it's done, I do think it's preferable to have the parse
analysis check, to ensure that it's being used in the right part of
the query, rather than leaving that to plan/execution time.
If it is turned into a function, the patch also needs to update the
ruleutils code --- it needs to be prepared to output a
schema-qualified function name, if necessary (something that the
keyword approach saves us from).
Regards,
Dean