pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: David Rowley <drowley@postgresql.org>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Fix REALLOCATE_BITMAPSETS code
Date: Tue, 16 Jan 2024 20:31:08 +0000
Message-ID: <E1rPq5U-001k0B-1O@gemulon.postgresql.org> (raw)

Fix REALLOCATE_BITMAPSETS code

7d58f2342 added a compile-time option to have bitmapset.c reallocate the
set before returning when a set is modified.  That commit failed to do
its job in various cases and returned the input set when it shouldn't
have in these cases.  Here we fix those missing cases.

This commit also adds some documentation about what
REALLOCATE_BITMAPSETS is for.  This is important as future functions
that go inside bitmapset.c need to know if they need to do anything
special when this compile-time option is defined.

Also, between 71a3e8c43 and 7d58f2342 some Asserts seem to have become
duplicated.  Tidy these up.  Rather than having the Assert check each
aspect of what makes a set invalid, here we introduce a helper function
which returns false when a set is invalid and have the Asserts use this
instead.

Also, make a pass on improving the comments in bitmapset.c.  Various
comments mentioned the input sets being "recycled".  This could be
interpreted to mean that the output set will always point to the same
memory as the given input parameter.  Here we try to make it clear that
this must not be relied upon and that callers must ensure that all
references to a given set are updated on each modification.

In passing, improve comments for bms_union(), bms_intersect() and
bms_difference() to detail what they do.  I (David) have too often had to
remind myself by reading the code each time to find out if I need, for
example, to use bms_union() or bms_join().  I also removed some
low-value comments that were trying to convey information about "these
operations" without mentioning which operations it was talking about.
It seems better to document these things in the function header comment
instead.

Author: Richard Guo, David Rowley
Discussion: https://postgr.es/m/CAMbWs4-djy9qYux2gZrtmxA0StrYXJjvB-oqLxn-d7J88t=PQQ@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/c7e5e994b2eb07cd0f3d5f0bb320e981bf1aae6e

Modified Files
--------------
src/backend/nodes/bitmapset.c | 329 ++++++++++++++++++++++++------------------
1 file changed, 189 insertions(+), 140 deletions(-)



Message-ID: <E1rPq5U-001k0B-1O@gemulon.postgresql.org>
Permalink:  ../E1rPq5U-001k0B-1O@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1rPq5U-001k0B-1O@gemulon.postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: drowley@postgresql.org, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Fix REALLOCATE_BITMAPSETS code
  In-Reply-To: <E1rPq5U-001k0B-1O@gemulon.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox