On Thu, Jun 9, 2016 at 5:45 PM, Andres Freund <and...@anarazel.de> wrote: > 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()?
Hmm. Yeah, that helps. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
