Em qui., 1 de set. de 2022 às 21:27, David Rowley <dgrowleyml@gmail.com> escreveu:
On Sat, 27 Aug 2022 at 01:29, Ranier Vilela <ranier.vf@gmail.com> wrote: > At function has_matching_range, if variable ranges->nranges == 0, > we exit quickly with a result equal to false. > > This means that nranges can be zero. > It occurs then that it is possible then to occur an array out of bonds, in the initialization of the variable maxvalue. > So if nranges is equal to zero, there is no need to initialize minvalue and maxvalue.
I think there's more strange coding in the same file that might need addressed, for example, AssertCheckRanges() does:
if (ranges->nranges == 0) break;
from within the first for() loop. Why can't that check be outside of the loop. Nothing seems to make any changes to that field from within the loop.
Also, in the final loop of the same function there's:
if (ranges->nsorted == 0) break;
It's not very obvious to me why we don't only run that loop when ranges->nsorted > 0. Also, isn't it an array overrun to access:
Datum value = ranges->values[2 * ranges->nranges + i];
If there's only 1 range stored in the array, then there should be 2 elements, but that code will try to access the 3rd element with ranges->values[2].
Yeah, it seems to me that both nranges and nsorted are invariant there,
so we can safely avoid loops.
This is not so critical, but I'll note it down anyway. The following looks a bit suboptimal in brin_minmax_multi_summary_out():
StringInfoData str;
initStringInfo(&str);
a = FunctionCall1(&fmgrinfo, ranges_deserialized->values[idx++]);
appendStringInfoString(&str, DatumGetCString(a));
b = cstring_to_text(str.data);
Why do we need a StringInfoData there? Why not just do:
b = cstring_to_text(DatumGetCString(a)); ?
That requires less memcpy()s and pallocs().
I agree that StringInfoData is not needed there.
Is it strange to convert char * to only store a temporary str.data.
