On 2016-06-09 17:40:24 -0400, Robert Haas wrote:
> On Thu, Jun 9, 2016 at 4:48 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> > Robert Haas <robertmh...@gmail.com> writes:
> >> On Sat, May 21, 2016 at 11:45 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> >>> Yes, let's fix it.  This will also take care of the questions about
> >>> whether the GIN/GIST opclass tweaks I made a few months ago require
> >>> module version bumps.
> >
> >> Tom, there's a patch for this at
> >> https://www.postgresql.org/message-id/574f091a.3050...@proxel.se which
> >> I think you should review, since you were the one who made the tweaks
> >> involved.  Any chance you can do that RSN?
> >
> > I've pushed this with some revisions to make the queries more
> > search-path-safe.  I'm not too happy with the safety of the queries
> > I see already present from the previous patches.  I think stuff
> > like this:
> >
> > UPDATE pg_proc SET proparallel = 's'
> > WHERE oid = 'min(citext)'::regprocedure;
> >
> > needs to be more like
> >
> > UPDATE pg_catalog.pg_proc SET proparallel = 's'
> > WHERE oid = 'min(citext)'::pg_catalog.regprocedure;
> 
> We could do that, but there's no guarantee that "min" or "citext"
> resolve correctly either, is there?  Basically, the search-path-safety
> of many of the scripts already in contrib looks pretty horrendous to
> me.  For example:
> 
> CREATE VIEW pg_buffercache AS
>         SELECT P.* FROM pg_buffercache_pages() AS P
>         (bufferid integer, relfilenode oid, reltablespace oid, reldatabase 
> oid,
>          relforknumber int2, relblocknumber int8, isdirty bool, usagecount 
> int2,
>          pinning_backends int4);
> 
> Well, what guarantee have we that we'll get the right
> pg_buffercache_pages() function?

Aren't both of the above guaranteed due to
        /*
         * Set up the search path to contain the target schema, then the schemas
         * of any prerequisite extensions, and nothing else.  In particular this
         * makes the target schema be the default creation target namespace.
         *
         * Note: it might look tempting to use PushOverrideSearchPath for this,
         * but we cannot do that.  We have to actually set the search_path GUC 
in
         * case the extension script examines or changes it.  In any case, the
         * GUC_ACTION_SAVE method is just as convenient.
         */
        initStringInfo(&pathbuf);
        appendStringInfoString(&pathbuf, quote_identifier(schemaName));
        foreach(lc, requiredSchemas)
        {
                Oid                     reqschema = lfirst_oid(lc);
                char       *reqname = get_namespace_name(reqschema);

                if (reqname)
                        appendStringInfo(&pathbuf, ", %s", 
quote_identifier(reqname));
        }

        (void) set_config_option("search_path", pathbuf.data,
                                                         PGC_USERSET, 
PGC_S_SESSION,
                                                         GUC_ACTION_SAVE, true, 
0, false);
in extension.c:execute_extension_script()?

> CREATE FUNCTION earth() RETURNS float8
> LANGUAGE SQL IMMUTABLE PARALLEL SAFE
> AS 'SELECT ''6378168''::float8';
> 
> What guarantees we'll get the correct float8 type?
> 
> CREATE FUNCTION sec_to_gc(float8)
> RETURNS float8
> LANGUAGE SQL
> IMMUTABLE STRICT
> PARALLEL SAFE
> AS 'SELECT CASE WHEN $1 < 0 THEN 0::float8 WHEN $1/(2*earth()) > 1
> THEN pi()*earth() ELSE 2*earth()*asin($1/(2*earth())) END';

These aren't though.

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to