On Oct 8 2025, at 11:47 pm, Michael Paquier <[email protected]> wrote:
> On Thu, Oct 09, 2025 at 04:35:55PM +1300, David Rowley wrote: >> On Thu, 9 Oct 2025 at 15:13, Michael Paquier <[email protected]> wrote: >> > What do you think about the attached? >> >> Thanks. Looks pretty good. >> >> > + members = palloc(sizeof(int) * num_ops); >> >> Any reason to pfree that and allocate that to the same size as it >> already was? > > No reason. We can shortcut that a bit. > >> Wondering if the "members[pos] = members[--num_members];" is worth a >> short comment. Maybe something like: /* zap this member by moving the >> final array member into its place and shrinking the array by 1 */ > > Yes, a comment can be adapted here. Sounds good to me. > -- > Michael Michael, David, This patch LGTM. I appreciate the additional work you've both put into it. You're right, of course, I didn’t need to preserve the order of elements when deleting. This is where I'd started and I don't recall why I changed it and didn't revert to this simpler solution. + members[pos] = members[--num_members]; Phase 2 might record a value to members that was also recorded in phase 1, but that seems harmless. That's why I had: + if (!bms_is_member(member, bms1) && !bms_is_member(member, bms2)) I hadn't thought to optimize this function, but given the amount of time the tests run in aggregate globally, it makes sense. best. -greg
