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