Re: BUG #18238: Cross-partitition MERGE/UPDATE with delete-preventing trigger leads to incorrect memory access

On Sun, Dec 10, 2023 at 5:55 PM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> On Sun, Dec 10, 2023 at 1:10 AM PG Bug reporting form
> <noreply(at)postgresql(dot)org> wrote:
> >
> > The following bug has been logged on the website:
> >
> > Bug reference: 18238
> > Logged by: Alexander Lakhin
> > Email address: exclusion(at)gmail(dot)com
> > PostgreSQL version: 16.1
> > Operating system: Ubuntu 22.04
> > Description:
> >
> > When the following query:
> > CREATE TABLE t (a int) PARTITION BY LIST (a);
> > CREATE TABLE tp1 PARTITION OF t FOR VALUES IN (1);
> > CREATE TABLE tp2 PARTITION OF t FOR VALUES IN (2);
> > INSERT INTO t VALUES (1);
> >
> > CREATE FUNCTION tf() RETURNS TRIGGER LANGUAGE plpgsql AS
> > $$ BEGIN RETURN NULL; END; $$;
> >
> > CREATE TRIGGER tr BEFORE DELETE ON t
> > FOR EACH ROW EXECUTE PROCEDURE tf();
> >
> > MERGE INTO t USING t st ON TRUE WHEN MATCHED THEN UPDATE SET a = 2;
> >
>
> --- a/src/backend/executor/nodeModifyTable.c
> +++ b/src/backend/executor/nodeModifyTable.c
> @@ -1828,10 +1828,10 @@ ExecCrossPartitionUpdate(ModifyTableContext *context,
> * additional rechecking, and might end up executing a different
> * action entirely).
> */
> - if (context->relaction != NULL)
> - return false;
> - else if (TupIsNull(epqslot))
> + if (TupIsNull(epqslot))
> return true;
> + else if (context->relaction != NULL)
> + return false;
> else
> {
> /* Fetch the most recent version of old tuple. */
>
> seems to work.
> but now the new command tag is MERGE 1. should be MERGE 0.

This should be ok. Now the command tag is MERGE 0.
But we may need extra words to document this.

diff --git a/src/backend/executor/nodeModifyTable.c
b/src/backend/executor/nodeModifyTable.c
index b16fbe9e..854b1ed4 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1828,10 +1828,10 @@ ExecCrossPartitionUpdate(ModifyTableContext *context,
* additional rechecking, and might end up executing a different
* action entirely).
*/
- if (context->relaction != NULL)
- return false;
- else if (TupIsNull(epqslot))
+ if (TupIsNull(epqslot))
return true;
+ else if (context->relaction != NULL)
+ return false;
else
{
/* Fetch the most recent version of old tuple. */
@@ -2909,9 +2909,12 @@ lmerge_matched:
*/
if (updateCxt.crossPartUpdate)
{
- mtstate->mt_merge_updated += 1;
- if (canSetTag)
- (estate->es_processed)++;
+ if (context->cpUpdateReturningSlot)
+ {
+ mtstate->mt_merge_updated += 1;
+ if (canSetTag)
+
(estate->es_processed)++;
+ }
return true;
}