Re: support for MERGE - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: support for MERGE
Date
Msg-id 20210118152131.GA17413@alvherre.pgsql
Whole thread Raw
In response to Re: support for MERGE  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: "Drouvot, Bertrand"
Date:
Subject: Standby recovery conflicts: add information when the cancellation occurs
Next
From: Zhihong Yu
Date:
Subject: Re: POC: postgres_fdw insert batching