From: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Amit Langote <amitlangote09(at)gmail(dot)com>, Richard Guo <guofenglinux(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, Greg Stark <stark(at)mit(dot)edu>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: List of Bitmapset (was Re: ExecRTCheckPerms() and many prunable partitions) |
Date: | 2022-11-15 09:07:33 |
Message-ID: | 20221115090733.htje4ogq27djdt5e@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2022-Nov-14, Tom Lane wrote:
> For discussion's sake, here's my current version of that 0004 patch,
> rewritten to use list-of-bitmapset as the data structure.
I feel that there should be more commentary that explains what a
multi-bms is. Not sure where, maybe just put it near the function that
first appears in the file.
> * The multi_bms prefix is a bit wordy, so I was thinking of shortening
> the function names to mbms_xxx. Maybe that's too brief.
I don't think the "ulti_" bytes add a lot, and short names are better.
Either you know what a mbms is, or you don't. If the latter, then you
jump to one of these functions in order to find out what the data
structure is; after that, you can read the code and it should be clear
enough.
> * This is a pretty short list of functions so far. I'm not eager
> to build out a bunch of dead code though. Is it OK to leave it
> with just this much functionality until someone needs more?
I agree with not adding dead code.
> * I'm a little hesitant about whether the API actually should be
> List-of-Bitmapset, or some dedicated struct as I had in the previous
> version of 0004. This version is way less invasive in prepjointree.c
> than that was, but the reason is there's ambiguity about what the
> forced_null_vars Lists actually contain, which feels error-prone.
Hmm ... if somebody makes a mistake, does the functionality break in
obvious ways, or is it very hard to pinpoint what happened?
> +/*
> + * multi_bms_add_member
> + * Add a new member to a list of bitmapsets.
> + *
> + * This is like bms_add_member, but for lists of bitmapsets.
> + * The new member is identified by the zero-based index of the List
> + * element it should go into, and the bit number to be set therein.
> + */
> +List *
> +multi_bms_add_member(List *mbms, int index1, int index2)
Maybe s/index1/listidx/ or bitmapidx and s/index2/bitnr/ ?
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"La rebeldía es la virtud original del hombre" (Arthur Schopenhauer)
From | Date | Subject | |
---|---|---|---|
Next Message | Simon Riggs | 2022-11-15 09:13:36 | Re: Allow single table VACUUM in transaction block |
Previous Message | Drouvot, Bertrand | 2022-11-15 09:02:17 | Re: Synchronizing slots from primary to standby |