From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
Cc: | Justin Pryzby <pryzby(at)telsasoft(dot)com>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Missing update of all_hasnulls in BRIN opclasses |
Date: | 2023-03-28 14:30:45 |
Message-ID: | ac391463-9335-8fe0-c097-2c661d4c5b73@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
It took me a while but I finally got back to reworking this to use the
bt_info bit, as proposed by Alvaro. And it turned out to work great,
because (a) it's a tuple-level flag, i.e. the right place, and (b) it
does not overload existing flags.
This greatly simplified the code in add_values_to_range and (especially)
union_tuples, making it much easier to understand, I think.
One disadvantage is we are unable to see which ranges are empty in
current pageinspect, but 0002 addresses that by adding "empty" column to
the brin_page_items() output. That's a matter for master only, though.
It's a trivial patch and it makes it easier/possible to test this, so we
should consider to squeeze it into PG16.
I did quite a bit of testing - the attached 0003 adds extra tests, but I
don't propose to get this committed as is - it's rather overkill. Maybe
some reduced version of it ...
The hardest thing to test is the union_tuples() part, as it requires
concurrent operations with "correct" timing. Easy to simulate by
breakpoints in GDB, not so much in plain regression/TAP tests.
There's also a stress tests, doing a lot of randomized summarizations,
etc. Without the fix this failed in maybe 30% of runs, now I did ~100
runs without a single failure.
I haven't done any backporting, but I think it should be simpler than
with the earlier approach. I wonder if we need to care about starting to
use the previously unused bit - I don't think so, in the worst case
we'll just ignore it, but maybe I'm missing something (e.g. when using
physical replication).
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachment | Content-Type | Size |
---|---|---|
0001-Fix-handling-of-NULLs-in-BRIN-indexes.patch | text/x-patch | 13.8 KB |
0002-Show-empty-ranges-in-brin_page_items.patch | text/x-patch | 6.9 KB |
0003-extra-tests.patch | text/x-patch | 25.1 KB |
stress-test.sh | application/x-shellscript | 5.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2023-03-28 14:46:29 | Re: "variable not found in subplan target list" |
Previous Message | Stephen Frost | 2023-03-28 14:30:28 | Re: Kerberos delegation support in libpq and postgres_fdw |