Horton

Re: ExecRTCheckPerms() and many prunable partitions

Alvaro Herrera <alvherre@alvh.no-ip.org>

View thread View on postgr.es
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Ian Lawrence Barwick <barwick@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, David Rowley <dgrowleyml@gmail.com>, Greg Stark <stark@mit.edu>, Zhihong Yu <zyu@yugabyte.com>, Julien Rouhaud <rjuju123@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-11-16T11:44:02Z
Lists: pgsql-hackers

Attachments

On 2022-Nov-10, Alvaro Herrera wrote:

> I couldn't find any cross-check that
> some perminfo element that we obtain for a rte does actually match the
> relation we wanted to check.  Maybe we could add a test in some central
> place that perminfo->relid equals rte->relid?

I hadn't looked hard enough.  This is already in GetRTEPermissionInfo().


> A related point is that concatenating lists doesn't seem to worry about
> not processing one element multiple times and ending up with bogus offsets.

> I think the API of ConcatRTEPermissionInfoLists is a bit weird.  Why not
> have the function return the resulting list instead, just like
> list_append?  It is more verbose, but it seems easier to grok.

Another point related to this.  I noticed that everyplace we do
ConcatRTEPermissionInfoLists, it is followed by list_append'ing the RT
list themselves.  This is strange.  Maybe that's the wrong way to look
at this, and instead we should have a function that does both things
together: pass both rtables and rtepermlists and smash them all
together.

I attach your 0001 again with a bunch of other fixups (I don't include
your 0002ff).  I've pushed this to see the CI results, and so far it's
looking good (hasn't finished yet though):
https://cirrus-ci.com/build/5126818977021952

I'll have a look at 0002 now.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

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