Re: ExecRTCheckPerms() and many prunable partitions

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: ExecRTCheckPerms() and many prunable partitions
Date: 2021-07-29 08:40:00
Message-ID: CA+HiwqFfiai=qBxPDTjaio_ZcaqUKh+FC=prESrB8ogZgFNNNQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jul 2, 2021 at 9:40 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Fri, Jul 2, 2021 at 12:45 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > 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.
>
> Ah, okay. I'll think about decoupling the permission checking stuff
> from the range table data structure.

I have finished with the attached. Sorry about the delay.

Think I've managed to get the first part done -- getting the
permission-checking info out of the range table -- but have not
seriously attempted the second -- doing away with the OLD/NEW range
table entries in the view/rule action queries, assuming that is what
you meant in the quoted.

One design point I think might need reconsidering is that the list of
the new RelPermissionInfo nodes that holds the permission-checking
info for relations has to be looked up with a linear search using the
relation OID, whereas it was basically free before if a particular of
code had the RTE handy. Maybe I need to check if the overhead of that
is noticeable in some cases.

As there's not much time left in this CF, I've bumped the entry to the next one.

--
Amit Langote
EDB: http://www.enterprisedb.com

Attachment Content-Type Size
v2-0001-Rework-query-relation-permission-checking.patch application/octet-stream 139.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nitin Jadhav 2021-07-29 08:56:53 Re: when the startup process doesn't (logging startup delays)
Previous Message Masahiko Sawada 2021-07-29 08:25:01 Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns