Re: Minimal logical decoding on standbys

Hi,

On 12/21/22 10:06 AM, Drouvot, Bertrand wrote:
> Hi,
>
> On 12/20/22 10:41 PM, Robert Haas wrote:
>> On Tue, Dec 20, 2022 at 3:39 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> I guess whatever else we
>> do here, we should fix the comments.
>>
>
> Agree, please find attached a patch proposal doing so.
>
>
>> Bottom line is that I think the two cases that have alignment issues
>> as coded are xl_hash_vacuum_one_page and gistxlogDelete. Everything
>> else is OK, as far as I can tell right now.
>>
>
> Thanks a lot for the repro(s) and explanations! That's very useful/helpful.
>
> Based on your discovery about the wrong comments above, I'm now tempted to fix those 2 alignment issues
> by using a FLEXIBLE_ARRAY_MEMBER within those structs (as you proposed in [1]) (as that should also prevent
> any possible wrong comments about where the array is located).
>
> What do you think?

As mentioned above, It looks to me that making use of a FLEXIBLE_ARRAY_MEMBER is a good choice.
So, please find attached v35 making use of a FLEXIBLE_ARRAY_MEMBER in xl_hash_vacuum_one_page and gistxlogDelete (your 2 repros are not failing anymore).
I've also added a few words in the commit message in 0001 about it.

So, we end up with:

(gdb) ptype /o struct xl_hash_vacuum_one_page
/* offset | size */ type = struct xl_hash_vacuum_one_page {
/* 0 | 4 */ TransactionId snapshotConflictHorizon;
/* 4 | 4 */ int ntuples;
/* 8 | 1 */ _Bool isCatalogRel;
/* XXX 1-byte hole */
/* 10 | 0 */ OffsetNumber offsets[];
/* XXX 2-byte padding */

/* total size (bytes): 12 */
}

(gdb) ptype /o struct gistxlogDelete
/* offset | size */ type = struct gistxlogDelete {
/* 0 | 4 */ TransactionId snapshotConflictHorizon;
/* 4 | 2 */ uint16 ntodelete;
/* 6 | 1 */ _Bool isCatalogRel;
/* XXX 1-byte hole */
/* 8 | 0 */ OffsetNumber offsets[];

/* total size (bytes): 8 */
}

While looking at it, I've a question: xl_hash_vacuum_one_page.ntuples is an int, do you see any reason why it is not an uint16? (we would get rid of 4 bytes in the struct).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com