Re: support for MERGE

Alvaro Herrera <alvherre@alvh.no-ip.org>

From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Ranier Vilela <ranier.vf@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Justin Pryzby <pryzby@telsasoft.com>, Amit Langote <amitlangote09@gmail.com>, Japin Li <japinli@hotmail.com>, Zhihong Yu <zyu@yugabyte.com>, simon.riggs@enterprisedb.co, Tomas Vondra <tomas.vondra@enterprisedb.com>, Daniel Westermann <dwe@dbi-services.com>, Erik Rijkers <er@xs4all.nl>, Jaime Casanova <jcasanov@systemguards.com.ec>, Andres Freund <andres@anarazel.de>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2022-04-02T15:02:01Z
Lists: pgsql-hackers
On 2022-Mar-31, Daniel Gustafsson wrote:

> > On 31 Mar 2022, at 19:38, Ranier Vilela <ranier.vf@gmail.com> wrote:
> 
> > I think that there is an oversight at 7103ebb
> > There is no chance of Assert preventing this bug.
> 
> This seems reasonable from brief reading of the code, NULL is a legitimate
> value for the map and that should yield an empty list AFAICT.

There's no bug here and this is actually intentional: if the map is
NULL, this function should not be called.

In the code before this commit, there was an assert that this variable
was not null:

  static List *
  adjust_partition_colnos(List *colnos, ResultRelInfo *leaf_part_rri)
  {
-   List       *new_colnos = NIL;
    TupleConversionMap *map = ExecGetChildToRootMap(leaf_part_rri);
!   AttrMap    *attrMap;
    ListCell   *lc;
  
!   Assert(map != NULL);        /* else we shouldn't be here */
!   attrMap = map->attrMap;
  
    foreach(lc, colnos)
    {


We could add an Assert that map is not null in the new function, but
really there's no point: if the map is null, we'll crash just fine in
the following line.

I would argue that we should *remove* the Assert() that I left in
adjust_partition_colnos_with_map.

Even if we wanted to make the function handle the case of a NULL map,
then the right fix is not to return NIL, but rather we should return the
original list.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/



Commits

  1. adjust_partition_colnos mustn't be called if not needed

  2. Add support for MERGE SQL command