Re: ExecRTCheckPerms() and many prunable partitions
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Amit Langote <amitlangote09@gmail.com>,
David Rowley <dgrowleyml@gmail.com>, Greg Stark <stark@mit.edu>,
Zhihong Yu <zyu@yugabyte.com>, Julien Rouhaud <rjuju123@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-11-11T16:46:16Z
Lists: pgsql-hackers
Alvaro Herrera <alvherre@alvh.no-ip.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
Commits
-
Add a test case for a316a3bc
- 054ff3b33a85 17.0 landed
- 0a14bca662a4 16.0 landed
-
Correctly set userid of subquery relations' child rels
- a316a3bc6d3f 16.0 landed
-
Fix buggy recursion in flatten_rtes_walker().
- c7468c73f7b6 16.0 landed
-
Remove some dead code in selfuncs.c
- 438e6b724090 16.0 landed
-
Update outdated comment in ApplyRetrieveRule
- 29861e228a47 16.0 landed
-
Rework query relation permission checking
- a61b1f74823c 16.0 landed
-
Generalize ri_RootToPartitionMap to use for non-partition children
- fb958b5da86d 16.0 landed
-
Stop accessing checkAsUser via RTE in some cases
- 599b33b9492d 16.0 landed
-
Add 'missing_ok' argument to build_attrmap_by_name
- ad86d159b6ab 16.0 landed