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-08-26 09:13:33
Message-ID: CA+HiwqGwYbrSOH1FLNVr=bp=oZB81X7d8pJzpB52Z19Zs=otow@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Aug 20, 2021 at 10:46 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Thu, Jul 29, 2021 at 5:40 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > 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:
> > > > 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.
> >
> > 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.
>
> I took a stab at the 2nd part, implemented in the attached 0002.
>
> The patch removes UpdateRangeTableOfViewParse() which would add the
> dummy OLD/NEW entries to a view rule's action query's rtable
>
> I haven't yet checked how this further improves the performance for
> the case discussed at [1] that prompted this.
>
> [1] https://www.postgresql.org/message-id/flat/797aff54-b49b-4914-9ff9-aa42564a4d7d%40www.fastmail.com

I checked the time required to do explain select * from v512 (worst
case), using the setup described at the above link and I get the
following numbers:

HEAD: 119.774 ms
0001 : 129.802 ms
0002 : 109.456 ms

So it appears that applying only 0001 makes things a bit worse for
this case. That seems to have to do with the following addition in
pull_up_simple_subquery():

@@ -1131,6 +1131,9 @@ pull_up_simple_subquery(PlannerInfo *root, Node
*jtnode, RangeTblEntry *rte,
*/
parse->rtable = list_concat(parse->rtable, subquery->rtable);

+ parse->relpermlist = MergeRelPermissionInfos(parse->relpermlist,
+ subquery->relpermlist);
+

What it does is pull up the RelPermissionInfo nodes in the subquery
being pulled up into the parent query and it's not a simple
list_concat(), because I decided that it's better to de-duplicate the
entries for a given relation OID even across subqueries.

Things get better than HEAD with 0002, because less work needs to be
done in the rewriter when copying the subqueries into the main query,
especially the range table, which only has 1 entry now, not 3 per
view.

Attached updated patches. I wrote a longer commit message for 0002 this time.

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

Attachment Content-Type Size
v4-0001-Rework-query-relation-permission-checking.patch application/octet-stream 140.3 KB
v4-0002-Do-not-add-OLD-NEW-RTEs-to-stored-view-rule-actio.patch application/octet-stream 114.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2021-08-26 09:20:45 Re: list of acknowledgments for PG14
Previous Message Kyotaro Horiguchi 2021-08-26 08:48:34 Re: prevent immature WAL streaming