RE: [BUG] (firsttupleslot)==NULL is redundant or is possible nulldereference? - Mailing list pgsql-hackers

From Ranier Vilela
Subject RE: [BUG] (firsttupleslot)==NULL is redundant or is possible nulldereference?
Date
Msg-id MN2PR18MB29272E5ECF35B22328808185E3490@MN2PR18MB2927.namprd18.prod.outlook.com
Whole thread Raw
In response to Re: [BUG] (firsttupleslot)==NULL is redundant or is possible nulldereference?  (Tomas Vondra <tomas.vondra@2ndquadrant.com>)
Responses Re: [BUG] (firsttupleslot)==NULL is redundant or is possible nulldereference?  (Tomas Vondra <tomas.vondra@2ndquadrant.com>)
List pgsql-hackers
Hi,
Sorry, you are right.
Had not seen this line:
firsttupleslot = node->ss.ss_ScanTupleSlot;

Best regards.
Ranier Vilela
________________________________________
De: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Enviado: sexta-feira, 22 de novembro de 2019 22:54
Para: Ranier Vilela
Cc: pgsql-hackers@postgresql.org
Assunto: Re: [BUG] (firsttupleslot)==NULL is redundant or is possible null dereference?

On Fri, Nov 22, 2019 at 10:32:11PM +0000, Ranier Vilela wrote:
>Hi,
>This is real bug? firsttupleslot == NULL.
>

Ranier, I don't want to be rude, but I personally am getting a bit
annoyed by this torrent of bug reports that are essentially just a bunch
of copy-pasted chunks of code, without any specification of bench,
position in the file, etc.

And more importantly, without any clear explanation why you think it is
a bug (or even a demonstration of an issue), and "Is this a bug?"

>\backend\executor\nodeGroup.c
>       if (TupIsNull(firsttupleslot))
>       {
>               outerslot = ExecProcNode(outerPlanState(node));
>               if (TupIsNull(outerslot))
>               {
>                       /* empty input, so return nothing */
>                       node->grp_done = true;
>                       return NULL;
>               }
>               /* Copy tuple into firsttupleslot */
>               ExecCopySlot(firsttupleslot, outerslot);
>
>include\executor\tuptable.h:
>#define TupIsNull(slot) \
>       ((slot) == NULL || TTS_EMPTY(slot))
>
>static inline TupleTableSlot *
>ExecCopySlot(TupleTableSlot *dstslot, TupleTableSlot *srcslot)
>{
>       Assert(!TTS_EMPTY(srcslot));
>
>       dstslot->tts_ops->copyslot(dstslot, srcslot);
>
>       return dstslot;
>}
>

And why do you think this is a bug? Immediately before the part of code
you copied we have this:

     /*
      * The ScanTupleSlot holds the (copied) first tuple of each group.
      */
     firsttupleslot = node->ss.ss_ScanTupleSlot;

And node->ss.ss_ScanTupleSlot is expected to be non-NULL. So the initial
assumption that firsttupleslot is NULL is incorrect.

regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: Attempt to consolidate reading of XLOG page
Next
From: Ranier Vilela
Date:
Subject: [PATCH] Tiny optmization or a bug?