Re: support for MERGE

Em sáb., 2 de abr. de 2022 às 12:01, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
escreveu:

> On 2022-Mar-31, Daniel Gustafsson wrote:
>
> > > On 31 Mar 2022, at 19:38, Ranier Vilela <ranier(dot)vf(at)gmail(dot)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.
>
IMHO, actually there are bug here.
ExecGetChildToRootMap is clear, is possible returning NULL.
To discover if the map is NULL, ExecGetChildToRootMap needs to process
"ResultRelInfo *leaf_part_rri".
So, the argument "if the map is NULL, this function should not be called",
is contradictory.

Actually, with Assert at function adjust_partition_colnos_using_map,
will never be checked, because it crashed before, both
production and debug modes.

> 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.
>
If the right fix is to return the original list, here is the patch attached.

regards
Ranier Vilela