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
-
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