Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Oct 6, 2022 at 8:35 PM Ranier Vilela <ranier.vf@gmail.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
