Re: Missing update of all_hasnulls in BRIN opclasses

Tomas Vondra <tomas.vondra@enterprisedb.com>

From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Matthias van de Meent <boekewurm+postgres@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2023-05-06T22:13:07Z
Lists: pgsql-hackers

On 4/24/23 23:20, Tomas Vondra wrote:
> On 4/24/23 17:36, Alvaro Herrera wrote:
>> On 2023-Apr-23, Tomas Vondra wrote:
>>
>>> here's an updated version of the patch, including a backport version. I
>>> ended up making the code yet a bit closer to master by introducing
>>> add_values_to_range(). The current pre-14 code has the loop adding data
>>> to the BRIN tuple in two places, but with the "fixed" logic handling
>>> NULLs and the empty_range flag the amount of duplicated code got too
>>> high, so this seem reasonable.
>>
>> 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)

b) encoding the info in existing BrinMemTuple flags - e.g. we could use
bt_placeholder to store two bits, not just one. Seems a bit ugly.

c) ignore the issue - AFAICS this would be an issue only for (external)
code accessing BrinMemTuple structs, but I don't think we're aware of
any out-of-core BRIN opclasses or anything like that ...


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Commits

  1. Show empty BRIN ranges in brin_page_items

  2. Fix handling of empty ranges and NULLs in BRIN

  3. Fix handling of NULLs when merging BRIN summaries

  4. Move IS [NOT] NULL handling from BRIN support functions