Date: Thu, 20 Jan 2022 18:02:22 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Japin Li <japinli@hotmail.com>
Cc: Erik Rijkers <er@xs4all.nl>, Andrew Dunstan <andrew@dunslane.net>,
	Simon Riggs <simon.riggs@enterprisedb.com>,
	Tomas Vondra <tomas.vondra@enterprisedb.com>,
	Zhihong Yu <zyu@yugabyte.com>,
	Daniel Westermann <dwe@dbi-services.com>,
	Amit Langote <amitlangote09@gmail.com>,
	Justin Pryzby <pryzby@telsasoft.com>,
	Pavan Deolasee <pavan.deolasee@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: support for MERGE
Message-ID: <202201202102.ivimvlzzk4rv@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <MEYP282MB16695D606207B2090372E374B6569@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
Archived-At: 
 <https://www.postgresql.org/message-id/202201202102.ivimvlzzk4rv%40alvherre.pgsql>
Precedence: bulk

On 2022-Jan-17, Japin Li wrote:

> So for NOT MATCHED, we are expected not use the target table columns.
> 
> The code comes from execMerge.c says:
> 
>     /*
>      * Make source tuple available to ExecQual and ExecProject. We don't need
>      * the target tuple, since the WHEN quals and the targetlist can't refer to
>      * the target columns.
>      */
>     econtext->ecxt_scantuple = NULL;
>     econtext->ecxt_innertuple = slot;
>     econtext->ecxt_outertuple = NULL;
> 
> It will set econtext->ecxt_scantuple to NULL, which leads the crash.

Right.  So this was broken by the fact that I recently allowed MATCHED
actions to target DO NOTHING; previously, only NOT MATCHED actions could
do so.  So the bug was present, but it wasn't accessible.

> Should we setNamespaceVisibilityForRTE() for CMD_NOTHING?  I try to set it
> and it works as expected.  OTOH, the system attributes from target table
> also cannot be accessible.  I'm not sure the v6 patch how to implement this
> limitation.

I changed this block so that it depends on whether the clause is MATCHED
or NOT MATCHED, rather than the action.  I think it was pretty
nonsensical for it to be keyed on action type, and it made the code
needlessly longer.

Thank you!

-- 
Álvaro Herrera              Valdivia, Chile  —  https://www.EnterpriseDB.com/
"All rings of power are equal,
But some rings of power are more equal than others."
                                 (George Orwell's The Lord of the Rings)