Re: silence compiler warning in brin.c

On Wed, Jun 1, 2022 at 10:06 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Zhihong Yu <zyu(at)yugabyte(dot)com> writes:
> > On Wed, Jun 1, 2022 at 9:35 AM Nathan Bossart <nathandbossart(at)gmail(dot)com>
> > wrote:
> >> I'm seeing a compiler warning in brin.c with an older version of gcc.
> >> Specifically, it seems worried that a variable might not be initialized.
> >> AFAICT there is no real risk, so I've attached a small patch to silence
> the
> >> warning.
>
> Yeah, I noticed the other day that a couple of older buildfarm members
> (curculio, gaur) are grousing about this too. We don't really have a
> hard-n-fast rule about how old a compiler needs to be before we stop
> worrying about its notions about uninitialized variables, but these are
> kind of old. Still, since this is the only such warning from these
> animals, I'm inclined to silence it.
>
> > It seems the variable can be initialized to the value of GUCNestLevel
> since
> > later in the func:
> > /* Roll back any GUC changes executed by index functions */
> > AtEOXact_GUC(false, save_nestlevel);
>
> That seems pretty inappropriate. If, thanks to some future thinko,
> control were able to reach the AtEOXact_GUC call despite not having
> called NewGUCNestLevel, we'd want that to fail. It looks like
> AtEOXact_GUC asserts nestLevel > 0, so that either 0 or -1 would
> do as an "invalid" value; I'd lean a bit to using 0.
>
> regards, tom lane
>
Hi,

if (heapRel == NULL || heapoid != IndexGetRelation(indexoid, false))
ereport(ERROR,

I wonder why the above check is not placed in the else block:

else
heapRel = NULL;

because heapRel is not modified between the else and the above check.
If the check is placed in the else block, we can potentially save the call
to index_open().

Cheers