Re: ExecRTCheckPerms() and many prunable partitions

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Amit Langote <amitlangote09(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, Greg Stark <stark(at)mit(dot)edu>, Zhihong Yu <zyu(at)yugabyte(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: ExecRTCheckPerms() and many prunable partitions
Date: 2022-11-11 16:46:16
Message-ID: 545349.1668185176@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
> On 2022-Oct-06, Amit Langote wrote:
>> Actually, List of Bitmapsets turned out to be something that doesn't
>> just-work with our Node infrastructure, which I found out thanks to
>> -DWRITE_READ_PARSE_PLAN_TREES. So, I had to go ahead and add
>> first-class support for copy/equal/write/read support for Bitmapsets,

> Hmm, is this related to what Tom posted as part of his 0004 in
> https://postgr.es/m/2901865.1667685211@sss.pgh.pa.us

It could be. For some reason I thought that Amit had withdrawn
his proposal to make Bitmapsets be Nodes. But if it's still live,
then the data structure I invented in my 0004 could plausibly be
replaced by a List of Bitmapsets.

The code I was using that for would rather have fixed-size arrays
of Bitmapsets than variable-size Lists, mainly because it always
knows ab initio what the max length of the array will be. But
I don't think that the preference is so strong that it justifies
a private data structure.

The main thing I was wondering about in connection with that
was whether to assume that there could be other future applications
of the logic to perform multi-bitmapset union, intersection,
etc. If so, then I'd be inclined to choose different naming and
put those functions in or near to bitmapset.c. It doesn't look
like Amit's code needs anything like that, but maybe somebody
has an idea about other applications?

Anyway, I concur with Peter's upthread comment that making
Bitmapsets be Nodes is probably justifiable all by itself.
The lack of a Node tag in them now is just because in a 32-bit
world it seemed like unnecessary bloat. But on 64-bit machines
it's free, and we aren't optimizing for 32-bit any more.

I do not like the details of v24-0003 at all though, because
it seems to envision that a "node Bitmapset" is a different
thing from a raw Bitmapset. That can only lead to bugs ---
why would we not make it the case that every Bitmapset is
properly labeled with the node tag?

Also, although I'm on board with making Bitmapsets be Nodes,
I don't think I'm on board with changing their dump format.
Planner node dumps would get enormously bulkier and less
readable if we changed things like

:relids (b 1 2)

to

:relids
{BITMAPSET
(b 1 2)
}

or whatever the output would look like as the patch stands.
So that needs a bit more effort, but it's surely manageable.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2022-11-11 17:35:06 Re: Add LSN along with offset to error messages reported for WAL file read/write/validate header failures
Previous Message Önder Kalacı 2022-11-11 16:16:36 Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher