Horton

Re: ExecRTCheckPerms() and many prunable partitions

Zhihong Yu <zyu@yugabyte.com>

View thread View on postgr.es
From: Zhihong Yu <zyu@yugabyte.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Julien Rouhaud <rjuju123@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-03-23T21:02:10Z
Lists: pgsql-hackers 
On Wed, Mar 23, 2022 at 12:03 AM Amit Langote <amitlangote09@gmail.com>
wrote:

> On Mon, Mar 14, 2022 at 4:36 PM Amit Langote <amitlangote09@gmail.com>
> wrote:
> > Also needed fixes when rebasing.
>
> Needed another rebase.
>
> As the changes being made with the patch are non-trivial and the patch
> hasn't been reviewed very significantly since Alvaro's comments back
> in Sept 2021 which I've since addressed, I'm thinking of pushing this
> one into the version 16 dev cycle.
>
> --
> Amit Langote
> EDB: http://www.enterprisedb.com

Hi,
For patch 1:

bq. makes permissions-checking needlessly expensive when many inheritance
children are added to the range range

'range' is repeated in the above sentence.

+ExecCheckOneRelPerms(RelPermissionInfo *perminfo)

Since RelPermissionInfo is for one relation, I think the 'One' in func name
can be dropped.

+       else                    /* this isn't a child result rel */
+           resultRelInfo->ri_RootToChildMap = NULL;
...
+       resultRelInfo->ri_RootToChildMapValid = true;

Should the assignment of true value be moved into the if block (in the else
block, ri_RootToChildMap is assigned NULL) ?

+   /* Looks like the RTE doesn't, so try to find it the hard way. */

doesn't -> doesn't know

Cheers

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