22.03.2023 06:07, Richard Guo wrote:
Alexander Lakhin <exclusion@gmail.com> writes:
> I'm afraid that zeroing only bytes behind nitems bits is not enough, as outDatum() doesn't bother to calculate the exact
> size of nulls bitmap, it just outputs all bytes of a datum (40 bytes in that case):
In that case, won't padding bytes between array elements also create
issues? Seems like we have to just zero the whole array area, like
those other functions do.
I came to a conclusion that the padding is not affected by
palloc/palloc0 there.
There we have:
subdata[outer_nelems] = ARR_DATA_PTR(array);
subbytes[outer_nelems] = ARR_SIZE(array) - ARR_DATA_OFFSET(array);
...
memcpy(dat, subdata[i], subbytes[i]);
dat += subbytes[i];
So the result array elements come packed one after another from the input
array and are not padded here.
Yeah, this should be the right fix, to use palloc0 instead here. FWIW
currently in the codes there are 14 places that explicitly allocate
ArrayType, 13 of them using palloc0, the only exception is the one
discussed here.
BTW, in array_set_slice() and array_set_element() we explicitly zero the
null bitmap although the whole array area is allocated with palloc0. Is
this necessary?
Yeah, I see two instances of MemSet(newnullbitmap, 0, ...):
1) In array_set_element():
/* Zero the bitmap to take care of marking inserted positions null */
MemSet(newnullbitmap, 0, (newnitems + 7) / 8);
It came with cecb60755 (2005-11-17).
2) In array_set_slice():
/* Zero the bitmap to handle marking inserted positions null */
MemSet(newnullbitmap, 0, (nitems + 7) / 8);
It came with 352a56ba6 (2006-09-29).
These MemSets were meaningful before 18c0b4ecc (2011-04-27).
The commit 18c0b4ecc changed palloc() to palloc0() almost everywhere, so
that the MemSets became redundant, but at the same time the discussed
palloc() (residing in execQual.c:ExecEvalArray() back then) somehow evaded
the change.
So maybe the fix for the bug can be seen as a supplement for 18c0b4ecc.
Best regards,
Alexander
