Re: ExecRTCheckPerms() and many prunable partitions

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <amitlangote09@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-07-01T15:45:03Z
Lists: pgsql-hackers
Amit Langote <amitlangote09@gmail.com> writes:
> The problem is that it loops over the entire range table even though
> only one or handful of those entries actually need their permissions
> checked.  Most entries, especially those of partition child tables
> have their requiredPerms set to 0, which David pointed out to me in
> [2], so what ExecCheckRTPerms() does in their case is pure overhead.

> An idea to fix that is to store the RT indexes of the entries that
> have non-0 requiredPerms into a separate list or a bitmapset in
> PlannedStmt.

I think perhaps we ought to be more ambitious than that, and consider
separating the list of permissions-to-check from the rtable entirely.
Your patch hardly qualifies as non-invasive, plus it seems to invite
errors of omission, while if we changed the data structure altogether
then the compiler would help find any not-updated code.

But the main reason that this strikes me as possibly a good idea
is that I was just taking another look at the complaint in [1],
where I wrote

>> I think it's impossible to avoid less-than-O(N^2) growth on this sort
>> of case.  For example, the v2 subquery initially has RTEs for v2 itself
>> plus v1.  When we flatten v1 into v2, v2 acquires the RTEs from v1,
>> namely v1 itself plus foo.  Similarly, once vK-1 is pulled up into vK,
>> there are going to be order-of-K entries in vK's rtable, and that stacking
>> makes for O(N^2) work overall just in manipulating the rtable.
>> 
>> We can't get rid of these rtable entries altogether, since all of them
>> represent table privilege checks that the executor will need to do.

Perhaps, if we separated the rtable from the required-permissions data
structure, then we could avoid pulling up otherwise-useless RTEs when
flattening a view (or even better, not make the extra RTEs in the
first place??), and thus possibly avoid that exponential planning-time
growth for nested views.

Or maybe not.  But I think we should take a hard look at whether
separating these data structures could solve both of these problems
at once.

			regards, tom lane

[1] https://www.postgresql.org/message-id/flat/797aff54-b49b-4914-9ff9-aa42564a4d7d%40www.fastmail.com



Commits

  1. Add a test case for a316a3bc

  2. Correctly set userid of subquery relations' child rels

  3. Fix buggy recursion in flatten_rtes_walker().

  4. Remove some dead code in selfuncs.c

  5. Update outdated comment in ApplyRetrieveRule

  6. Rework query relation permission checking

  7. Generalize ri_RootToPartitionMap to use for non-partition children

  8. Stop accessing checkAsUser via RTE in some cases

  9. Add 'missing_ok' argument to build_attrmap_by_name