Re: Extension pg_trgm, permissions and pg_dump order

On Sun, Jun 12, 2022 at 08:45:51PM -0700, Noah Misch wrote:
> On Wed, Jun 08, 2022 at 10:17:08PM -0700, Noah Misch wrote:
> > On Sun, Jun 05, 2022 at 11:30:56PM -0700, Noah Misch wrote:
> > > On Fri, Jun 03, 2022 at 11:17:24AM -0700, Nathan Bossart wrote:
> > > > On Tue, May 31, 2022 at 08:28:55AM -0700, Nathan Bossart wrote:
> > > > > I've spent some time looking at all the code impacted by a117ceb and
> > > > > 0abc1a0, and I've yet to identify any additional problems besides the one
> > > > > with ResolveOpClass().  However, I'm far from confident in this analysis,
> > > > > and I still need to try out the ereport() approach that Noah suggested.  I
> > > > > would welcome any assistance identifying other problem areas.
> > > > 
> > > > Ah, I think I am missing something.  For example, the code that identifies
> > > > the exclusion operator (immediately following the call to ResolveOpClass())
> > > > is likely subject to a similar problem.
> > > 
> > > That would not surprise me.  Do you have what you need to continue?
> > 
> > I'll use that window on Sunday to review what you have.  If you have work in
> > progress, please get it to the mailing list by then.
> 
> I have the patch mostly done.  I will send something late on Wednesday.

I also found post-202205 CREATE INDEX reaching an improper "permission denied"
for the namespace containing a collation.  The attached version fixes and
tests all three improper denials.  Also, I added discards of GUC changes
before switching to the original userid.  Without those, an index expression
changing search_path affects the collation lookup.  Unfortunately, this is
making ComputeIndexAttrs() look like an unplanned mess.  I considered two
alternatives:

1. Move all "List *names -> oid" translation activities from
   ComputeIndexAttrs() to transformIndexStmt().  I ruled this out for a
   back-patched fix, because such a change would tend to break compatibility
   with the PGXN modules that call transformIndexStmt() or DefineIndex().  It
   might be a good long-term direction, though the note in the header of
   transformIndexStmt() gives another obstacle.

2. Move all "List *names -> oid" translation activities from
   ComputeIndexAttrs() to DefineIndex(), before table_open().  There's nothing
   ruling out this option, but I ran out of time to try it.  It would be weird
   to have per-attribute logic in both DefineIndex() and ComputeIndexAttrs(),
   so I'm skeptical about this helping.

For partitioned tables, it would add good defense-in-depth to perform the
"List *names -> oid" translations once per SQL statement, not once per
partition as has been the case.  (1) would move in that direction.

Thanks,
nm