Thanks for having a look at this.
On Wed, 25 Feb 2026 at 07:33, Zsolt Parragi <zsolt.parragi@percona.com> wrote:
> + * We expect that 'bits' contains at least one 0 bit somewhere in the mask,
> + * not necessarily < natts.
> + */
>
> Is this precondition really enough?
>
> Let's say we have 20 attributes, only attribute 20 is NULL
> Caller requests natts=8
>
> That sets lastByte = 1, loop only checks bits[0], which is 0xFF, exits
> with bytenum=1, bits[1] is also 0xFF
>
> Then we execute
>
> + res += pg_rightmost_one_pos32(~bits[bytenum]);
>
> where ~0xFF = 0
I think this works ok in v9, but in v10 I've added a cast so that line becomes:
res += pg_rightmost_one_pos32(~((uint32) bits[bytenum]));
For the case you describe, 0xFF becomes 0xFFFFFF00 and
pg_rightmost_one_pos32() returns 8, the lowest bit of the 2nd byte.
> + /* convert the lower 4 bits of null bitmap word into 32 bit int */
> + isnull_8 = (nullbyte & 0xf) * SPREAD_BITS_MULTIPLIER_32;
> +
> + /*
> + * convert the upper 4 bits of null bitmap word into 32 bit int, shift
> + * into the upper 32 bit
> + */
> + isnull_8 |= ((uint64) ((nullbyte >> 4) * SPREAD_BITS_MULTIPLIER_32)) << 32;
> +
> + /* mask out all other bits apart from the lowest bit of each byte */
> + isnull_8 &= UINT64CONST(0x0101010101010101);
> + memcpy(isnull, &isnull_8, sizeof(uint64));
>
>
> Won't this mix up column numbers on big-endian systems?
Yes, I believe you're right. I've added the following before the memcpy().
#ifdef WORDS_BIGENDIAN
/*
* Fix byte order on big-endian machines before copying to the array.
*/
isnull_8 = pg_bswap64(isnull_8);
#endif
>
> Subject: [PATCH v9 1/5] Introduce deform_bench test module
>
> For benchmaring tuple deformation.
> ---
>
> Typo: should be benchmarking
Fixed.
> + * firstNonGuaranteedAttr stores the index to info the compact_attrs array for
>
> to info should be "into"?
Also fixed.
I've attached a revised set of patches.
David
