Re: Extension pg_trgm, permissions and pg_dump order

On Tue, Jun 21, 2022 at 10:56:16AM -0700, Nathan Bossart wrote:
> On Wed, Jun 15, 2022 at 10:42:18PM -0700, Noah Misch wrote:
> > -REGRESS = citext citext_utf8
> > +REGRESS = create_index_acl citext citext_utf8
>
> It's a little unfortunate that these tests are located within the citext
> module, but I can't claim to have a better idea.

A little, yes. I did consider creating an operator class in the main
regression suite, then testing that. If I had used that approach, it probably
would have been a my_text that behaves as much like text as possible. There's
little difference in value between a contrib/ test and an equivalent
src/test/regress/ test, so I picked this for the increase in realism and the
decrease in test code bulk.

> > + * Identify the opclass to use. Use of ddl_userid is necessary due to
> > + * ACL checks therein. This is safe despite opclasses containing
> > + * opaque expressions (specifically, functions), because only
> > + * superusers can define opclasses.
>
> It's not clear to me why the fact that only superusers can define opclasses
> makes this safe.

classOidP[attn] = ResolveOpClass(attribute->opclass,
atttype,
accessMethodName,
accessMethodId);

To write the comment, I pondered how those four arguments could conceivably
lead ResolveOpClass() to locate Trojan code. Since only superusers can define
opclasses, we can assume the catalog entries of an opclass do not point to
Trojan code. (The superuser could just do the mischief directly, rather than
going to extra trouble to set a trap for later.) If you see a hole in that
thinking, please do share.

> Overall, the patch seems reasonable to me.

Thanks for reviewing. I'll push it sometime in the next seven days.