Re: Converting contrib SQL functions to new style - Mailing list pgsql-hackers

I wrote:
> I see that the cfbot is unhappy because it doesn't understand
> that some of the patches have been applied already.  I am going
> to go ahead and get the earthdistance one done, because we have
> a live problem report about that [1].  I'll rebase and repost
> the remainder afterwards.

Pushed the earthdistance patch, backpatching to v16 as already
discussed upthread.  While it's still fresh in mind, a few
comments about that:

The search path during execution of earthdistance's scripts
is going to be pg_catalog (implicitly first), then earthdistance's
schema, then cube's schema (if different), then pg_temp (to make
sure it's not implicitly before the others).  So that means the
ground rules for object references are:

* References to pg_catalog objects are unconditionally secure,
with or without qualification, if they have simple names (such
as datatypes).  Calls of functions and operators in pg_catalog
are unconditionally secure if the argument data types exactly match
the function or operator.  If implicit casting is required, there's
a hazard of the reference being captured by a better match further
down the search path.  That can be fixed by adding explicit schema
qualification, but I think usually it's better to make the argument
data types match by explicitly casting them.  In particular the
syntax for schema-qualifying an operator is painful enough that
nobody wants to do that.

* References to earthdistance's own objects are again unconditionally
secure for simple names (given no conflicts with pg_catalog).  For
calls of functions and operators, the only safe answer is to make the
argument data types match: schema qualification isn't enough, since
there might be hostile objects in earthdistance's installation schema.

* References to cube's objects require an @extschema:cube@
qualification to ensure they aren't captured by hostile objects
in earthdistance's schema, *and* we have to be careful about
exact argument data type matches to prevent capture by
hostile objects in cube's schema.

Ronan had added some "pg_catalog." qualifications to the previous
version of the patch, but I found all of them to be unnecessary
per these ground rules, and probably not good style because they
convey an impression of schema safety that is entirely false if
the operators aren't likewise qualified.

Also, I realized while looking at the patch that fixing the
functions wasn't sufficient, because we also had

CREATE DOMAIN earth AS cube
  CONSTRAINT not_point check(cube_is_point(value))
  CONSTRAINT not_3d check(cube_dim(value) <= 3)
  CONSTRAINT on_surface check(abs(cube_distance(value, '(0)'::cube) /
  earth() - '1'::float8) < '10e-7'::float8);

The reference to type cube is potentially subvertible, and so are the
cube function calls.  We could fix the constraint expressions (at
great cost) in the 1.2 update script by dropping and re-adding the
domain constraints, but there's nothing the update script can do to
change the domain's base type.

What I did about this was to add @extschema:cube@ decoration in this
command in the 1.1 base script.  I don't think this is a violation of
the normal rule that extension scripts mustn't change post-release,
because the intended results are the same.  (That is, really the rule
is "the results of installation mustn't change".)  This fixes things
for fresh installations including dump/restore updates.  It won't do
anything for you if you're trying to pg_upgrade an already-trojaned
earthdistance extension, but it's better than not addressing the issue
at all.

I think we could now mark earthdistance as trusted (probably only
in HEAD), but it'd be good for someone else to verify it.

            regards, tom lane