Re: Extension pg_trgm, permissions and pg_dump order

On Sat, May 28, 2022 at 08:30:33PM -0700, Nathan Bossart wrote:
> On Fri, May 27, 2022 at 09:51:22PM -0700, Noah Misch wrote:
> > On Wed, May 25, 2022 at 10:50:47PM -0700, Noah Misch wrote:
> >> BEGIN;
> >> CREATE ROLE limitedrole;
> >> CREATE SCHEMA ext_trgm;
> >> CREATE EXTENSION pg_trgm SCHEMA ext_trgm;
> >> CREATE TABLE x(y text);
> >> ALTER TABLE x OWNER TO limitedrole;
> >> CREATE INDEX ON x USING gist(y ext_trgm.gist_trgm_ops);
> >> ROLLBACK;
> > 
> >> Not too simple, no.  The parts of DefineIndex() that execute user code
> >> (e.g. DefineIndex->ComputeIndexAttrs->CheckMutability) are interspersed with
> >> the parts that do permissions checks, like the one yielding $SUBJECT at
> >> DefineIndex->ComputeIndexAttrs->ResolveOpClass->LookupExplicitNamespace.  My
> >> first candidate is to undo the userid switch before the ResolveOpClass() call
> >> and restore it after.  My second candidate is to pass down the userid we want
> >> used for this sort of permissions check.  Depending on the full list of call
> >> stacks reaching permissions checks, this could get hairy.
> > 
> > While I'd value the opportunity to work on this, there only a 50% chance I
> > could get it done by 2022-08-01.  I've set aside four hours on 2022-06-12 to
> > see how far I get.  For a 95% chance, the date would be 2023-02-01.  (I've
> > already canceled a 2022-07 vacation for the thing taking my time instead;
> > there's nothing clearly left to cut.)  If anyone would like it done faster
> > than that, I welcome that person taking over.
> 
> I'd like to help.

Thanks.

> I started looking at the problem and hacked together a
> proof-of-concept based on your first candidate that seems to fix the
> reported issue.  However, from the upthread discussion, it is not clear
> whether there is agreement on the approach.  IIUC there are still many
> other code paths that would require a similar treatment, so perhaps
> identifying all of those would be a good next step.

Agreed.  To identify them, perhaps put an ereport(..., errbacktrace()) in
aclmask(), then write some index-creating DDL that refers to the
largest-possible number of objects.