Feedback-ID: ia2694551:Fastmail
Date: Mon, 15 May 2023 12:06:07 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
	Matthias van de Meent <boekewurm+postgres@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Missing update of all_hasnulls in BRIN opclasses
Message-ID: <20230515100607.uftic7fsmz7blkfk@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <141202ca-4c92-e294-0bfe-e1709ee2f71b@enterprisedb.com>
Archived-At: <https://www.postgresql.org/message-id/20230515100607.uftic7fsmz7blkfk%40alvherre.pgsql>
Precedence: bulk

On 2023-May-07, Tomas Vondra wrote:

> > Álvaro wrote:
> >> In backbranches, the new field to BrinMemTuple needs to be at the end of
> >> the struct, to avoid ABI breakage.
> 
> Unfortunately, this is not actually possible :-(
> 
> The BrinMemTuple has a FLEXIBLE_ARRAY_MEMBER at the end, so we can't
> place anything after it. I think we have three options:
> 
> a) some other approach? - I really can't see any, except maybe for going
> back to the previous approach (i.e. encoding the info using the existing
> BrinValues allnulls/hasnulls flags)

Actually, mine was quite the stupid suggestion: the BrinMemTuple already
has a 3 byte hole in the place where you originally wanted to add the
flag:

struct BrinMemTuple {
    _Bool                      bt_placeholder;       /*     0     1 */

    /* XXX 3 bytes hole, try to pack */

    BlockNumber                bt_blkno;             /*     4     4 */
    MemoryContext              bt_context;           /*     8     8 */
    Datum *                    bt_values;            /*    16     8 */
    _Bool *                    bt_allnulls;          /*    24     8 */
    _Bool *                    bt_hasnulls;          /*    32     8 */
    BrinValues                 bt_columns[];         /*    40     0 */

    /* size: 40, cachelines: 1, members: 7 */
    /* sum members: 37, holes: 1, sum holes: 3 */
    /* last cacheline: 40 bytes */
};

so putting it there was already not causing any ABI breakage.  So, the
solution to this problem of not being able to put it at the end is just
to return the struct to your original formulation.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"La primera ley de las demostraciones en vivo es: no trate de usar el sistema.
Escriba un guión que no toque nada para no causar daños." (Jakob Nielsen)