From: | David Rowley <dgrowleyml(at)gmail(dot)com> |
---|---|
To: | Richard Guo <guofenglinux(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Revise the Asserts added to bimapset manipulation functions |
Date: | 2023-12-29 01:15:13 |
Message-ID: | CAApHDvoOooeebFGv0m_VBH5sCNYD2V8eCR9WxYXfb_cCmP7hAQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, 28 Dec 2023 at 23:21, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> then instead of having to do:
>
> #ifdef REALLOCATE_BITMAPSETS
> a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
> memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
> pfree(tmp);
> #endif
>
> all over the place. Couldn't we do:
>
> #ifdef REALLOCATE_BITMAPSETS
> return bms_clone_and_free(a);
> #else
> return a;
> #endif
I looked into this a bit more and I just can't see why the current
code feels like it must do the reallocation in functions such as
bms_del_members(). We don't repalloc the set there, ever, so why do
we need to do it when building with REALLOCATE_BITMAPSETS? It seems
to me the point of REALLOCATE_BITMAPSETS is to ensure that *if* we
possibly could end up reallocating the set that we *do* reallocate.
There's also a few cases where you could argue that the
REALLOCATE_BITMAPSETS code has introduced bugs. For example,
bms_del_members(), bms_join() and bms_int_members() are meant to
guarantee that their left input (both inputs in the case of
bms_join()) are recycled. Compiling with REALLOCATE_BITMAPSETS
invalidates that, so it seems about as likely that building with
REALLOCATE_BITMAPSETS could cause bugs rather than find bugs.
I've hacked a bit on this to fix these problems and also added some
documentation to try to offer future people modifying bitmapset.c some
guidance on if they need to care about REALLOCATE_BITMAPSETS.
I also don't think "hangling" is a word. So I've adjusted the comment
in pg_config_manual.h to fix that.
David
Attachment | Content-Type | Size |
---|---|---|
bitmapset_fixes_v2.patch | text/plain | 15.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2023-12-29 01:16:08 | Re: Fix Brin Private Spool Initialization (src/backend/access/brin/brin.c) |
Previous Message | Tomas Vondra | 2023-12-28 23:55:08 | Re: Statistics Import and Export |