Re: Avoid mix char with bool type in comparisons

Em sex., 7 de out. de 2022 às 12:40, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> escreveu:

> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Thu, Oct 6, 2022 at 8:35 PM Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
> wrote:
> >> No Tom, unfortunately I don't have the knowledge to create a test with
> GIN_MAYBE values.
>
> > Well then don't post.
>
> > Basically what you're saying is that you suspect there might be a
> > problem with this code but you don't really know that and you can't
> > test it. That's not a good enough reason to take up the time of
> > everyone on this list.
>
> FWIW, I did take a look at this code, and I don't see any bug.
> The entryRes[] array entries are indeed GinTernaryValue, but it's
> obvious by inspection that matchPartialInPendingList only returns
> true or false, therefore collectMatchesForHeapRow also only deals
> in true or false, never maybe.

Thanks for spending your time with this.

Anyway, it's not *true* that collectMatchesForHeapRow deals
only "true" and "false", once that key->entryRes is initialized with
GIN_FALSE not false.

/*
* Reset all entryRes and hasMatchKey flags
*/
for (i = 0; i < so->nkeys; i++)
{
GinScanKey key = so->keys + i;
memset(key->entryRes, GIN_FALSE, key->nentries);
}

Maybe only typo, that doesn't matter to anyone but some static analysis
tools that alarm about these stupid things.

/*
* Reset all entryRes and hasMatchKey flags
*/
for (i = 0; i < so->nkeys; i++)
{
GinScanKey key = so->keys + i;
memset(key->entryRes, false, key->nentries);
}

I do not think changing
> matchPartialInPendingList to return ternary would be an improvement,
> because then it'd be less obvious that it doesn't deal in maybe.
>
On this point I don't quite agree with you, since for anyone wanting to
read the code in gin.h,
they will think in terms of GIN_FALSE, GIN_TRUE and GIN_MAYBE,
and will have to spend some time thinking about why they are mixing char
and bool types.

Besides that, it remains a trap, just waiting for someone to fall in the
future.
if (key->entryRes[j]) be TRUE when GIN_MAYBE.

regards,
Ranier Vilela