Re: ExecRTCheckPerms() and many prunable partitions
Amit Langote <amitlangote09@gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-08-26T09:13:33Z
Lists: pgsql-hackers
Attachments
- v4-0001-Rework-query-relation-permission-checking.patch (application/octet-stream) patch v4-0001
- v4-0002-Do-not-add-OLD-NEW-RTEs-to-stored-view-rule-actio.patch (application/octet-stream) patch v4-0002
On Fri, Aug 20, 2021 at 10:46 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Thu, Jul 29, 2021 at 5:40 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > On Fri, Jul 2, 2021 at 9:40 AM Amit Langote <amitlangote09@gmail.com> wrote:
> > > On Fri, Jul 2, 2021 at 12:45 AM Tom Lane <tgl@sss.pgh.pa.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
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