Date: Mon, 18 Jan 2021 12:21:31 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Jaime Casanova <jcasanov@systemguards.com.ec>
Subject: Re: support for MERGE
Message-ID: <20210118152131.GA17413@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20210108192200.GA25633@alvherre.pgsql>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

Jaime Casanova just reported that this patch causes a crash on the
regression database with this query:

 MERGE INTO public.pagg_tab_ml_p3 as target_0 
 USING public.prt2_l_p3_p2 as ref_0 ON target_0.a = ref_0.a 
 WHEN MATCHED   AND cast(null as tid) <= cast(null as tid)    THEN DELETE;

The reason is down to adjust_partition_tlist() not being willing to deal
with empty tlists.  So this is the most direct fix:

diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 1fa4d84c42..d6b478ec33 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -976,7 +976,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
 				conv_tl = map_partition_varattnos((List *) action->targetList,
 												  firstVarno,
 												  partrel, firstResultRel);
-				conv_tl = adjust_partition_tlist(conv_tl, map);
+				if (conv_tl != NIL)
+					conv_tl = adjust_partition_tlist(conv_tl, map);
 				tupdesc = ExecTypeFromTL(conv_tl);
 				/* XXX gotta pfree conv_tl and tupdesc? */
 
 

But I wonder if it wouldn't be better to patch adjust_partition_tlist()
to return NIL on NIL input, instead, like this:

diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 1fa4d84c42..6a170eea03 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1589,6 +1589,9 @@ adjust_partition_tlist(List *tlist, TupleConversionMap *map)
 	AttrMap    *attrMap = map->attrMap;
 	AttrNumber	attrno;
 
+	if (tlist == NIL)
+		return NIL;
+
 	Assert(tupdesc->natts == attrMap->maplen);
 	for (attrno = 1; attrno <= tupdesc->natts; attrno++)
 	{

I lean towards the latter myself.

-- 
Álvaro Herrera       Valdivia, Chile