public inbox for [email protected]  
help / color / mirror / Atom feed
From: Alvaro Herrera <[email protected]>
To: Dean Rasheed <[email protected]>
Cc: Vik Fearing <[email protected]>
Cc: Isaac Morland <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: MERGE ... RETURNING
Date: Sun, 22 Jan 2023 19:58:39 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAEZATCViD_PvYUP0scY_LAbm5Ueg0Z4A4xbdDkKYo2eWO4hAuw@mail.gmail.com>

> diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
> new file mode 100644
> index e34f583..aa3cca0
> --- a/src/backend/commands/copy.c
> +++ b/src/backend/commands/copy.c
> @@ -274,12 +274,6 @@ DoCopy(ParseState *pstate, const CopyStm
>  	{
>  		Assert(stmt->query);
>  
> -		/* MERGE is allowed by parser, but unimplemented. Reject for now */
> -		if (IsA(stmt->query, MergeStmt))
> -			ereport(ERROR,
> -					errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> -					errmsg("MERGE not supported in COPY"));

Does this COPY stuff come from another branch where you're adding
support for MERGE in COPY?  I see that you add a test that MERGE without
RETURNING fails, but you didn't add any tests that it works with
RETURNING.  Anyway, I suspect these small changes shouldn't be here.


Overall, the idea of using Postgres-specific functions for extracting
context in the RETURNING clause looks acceptable to me.  We can change
that to add support to whatever clauses the SQL committee offers, when
they get around to offering something.  (We do have to keep our fingers
crossed that they will decide to use the same RETURNING syntax as we do
in this patch, of course.)

Regarding mas_action_idx, I would have thought that it belongs in
MergeAction rather than MergeActionState.  After all, you determine it
once at parse time, and it is a constant from there onwards, right?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/






view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: MERGE ... RETURNING
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox