Feedback-ID: ia2694551:Fastmail
Date: Sun, 22 Jan 2023 19:58:39 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Vik Fearing <vik@postgresfriends.org>,
	Isaac Morland <isaac.morland@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: MERGE ... RETURNING
Message-ID: <20230122185839.3btblknlarddawtj@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAEZATCViD_PvYUP0scY_LAbm5Ueg0Z4A4xbdDkKYo2eWO4hAuw@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/20230122185839.3btblknlarddawtj%40alvherre.pgsql>
Precedence: bulk

> 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/