Обсуждение: extension_control_path
Hi, Please find attached to this email a patch implementing a new GUC that allows users to setup a list of path where PostgreSQL will search for the extension control files at CREATE EXTENSION time. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Вложения
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > Please find attached to this email a patch implementing a new GUC that > allows users to setup a list of path where PostgreSQL will search for > the extension control files at CREATE EXTENSION time. Why is that a good idea? It's certainly not going to simplify DBAs' lives, more the reverse. ("This dump won't reload." "Uh, where did you get that extension from?" "Ummm...") Assuming that there is some need for loading extensions from nonstandard places, would it be better to just allow a filename specification in CREATE EXTENSION? (I don't know the answer, since the use-case isn't apparent to me in the first place, but it seems worth asking.) regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Why is that a good idea? It's certainly not going to simplify DBAs' > lives, more the reverse. ("This dump won't reload." "Uh, where did > you get that extension from?" "Ummm...") The latest users for the feature are the Red Hat team working on Open Shift where they want to have co-existing per-user PostgreSQL clusters on a machine, each with its own set of extensions. Having extension_control_path also allows to install extension files in a place not owned by root. Lastly, as a developer, you might enjoy being able to have your own non-system-global place to install extensions, as Andres did explain on this list not too long ago. > Assuming that there is some need for loading extensions from nonstandard > places, would it be better to just allow a filename specification in > CREATE EXTENSION? (I don't know the answer, since the use-case isn't > apparent to me in the first place, but it seems worth asking.) In the extension_control_path idea, we still are adressing needs where the people managing the OS and the database are distinct sets. The GUC allows the system admins to setup PostgreSQL the way they want, then the database guy doesn't need to know anything about that at CREATE EXTENSION time. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > > Please find attached to this email a patch implementing a new GUC that > > allows users to setup a list of path where PostgreSQL will search for > > the extension control files at CREATE EXTENSION time. > > Why is that a good idea? It's certainly not going to simplify DBAs' > lives, more the reverse. ("This dump won't reload." "Uh, where did > you get that extension from?" "Ummm...") We *already* have that problem. I don't think this makes it particularly worse- you still need to go back to the old box and look at what came from where. Sure, you *might* be lucky enough to find the right extension by guessing at what packages were installed or searching for one that looks like the correct one, but then, you might discover that the version available isn't the right version for the database you're trying to restore anyway. Indeed, this might allow folks who don't particularly care for package systems to build consistent dumps without having to worry quite as much about what the package system is doing. > Assuming that there is some need for loading extensions from nonstandard > places, would it be better to just allow a filename specification in > CREATE EXTENSION? (I don't know the answer, since the use-case isn't > apparent to me in the first place, but it seems worth asking.) For my 2c, I could absolutely see it as being worthwhile to have an independent directory to install not-from-package extensions. That would keep things which are "managed by the package system" and things which are installed independent separate, which is absolutely a good thing, imv. Thanks, Stephen
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > Tom Lane <tgl@sss.pgh.pa.us> writes: >> Why is that a good idea? It's certainly not going to simplify DBAs' >> lives, more the reverse. ("This dump won't reload." "Uh, where did >> you get that extension from?" "Ummm...") > The latest users for the feature are the Red Hat team working on Open > Shift where they want to have co-existing per-user PostgreSQL clusters > on a machine, each with its own set of extensions. Um ... "own set of installed extensions" doesn't need to mean "own set of available extensions", any more than those clusters need to have their own Postgres executables. If the clusters *do* have their own executables, eg because they're different PG versions, then they can certainly also have their own $SHAREDIR trees too. So this example is totally without value for your case. > Having extension_control_path also allows to install extension files in > a place not owned by root. As far as the control files go, there's nothing saying that $SHAREDIR/extension has to be root-owned. If there are .so's involved, I do not believe the Red Hat crew is asking you to support loading .so's from non-root-owned dirs, because that'd be against their own corporate security policies. (But in any case, where we find the control and SQL files need not have anything to do with where the .so's are.) > Lastly, as a developer, you might enjoy being able to have your own > non-system-global place to install extensions, as Andres did explain on > this list not too long ago. And again, if you're working on a development version, $SHAREDIR/extension is probably owned by you anyway. I don't see that any of these scenarios create a need to install extension files anywhere but $SHAREDIR/extension. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: >> Tom Lane <tgl@sss.pgh.pa.us> writes: >>> Why is that a good idea? It's certainly not going to simplify DBAs' >>> lives, more the reverse. ("This dump won't reload." "Uh, where did >>> you get that extension from?" "Ummm...") > >> The latest users for the feature are the Red Hat team working on Open >> Shift where they want to have co-existing per-user PostgreSQL clusters >> on a machine, each with its own set of extensions. > > Um ... "own set of installed extensions" doesn't need to mean "own set of > available extensions", any more than those clusters need to have their > own Postgres executables. If the clusters *do* have their own > executables, eg because they're different PG versions, then they can > certainly also have their own $SHAREDIR trees too. So this example > is totally without value for your case. They have several clusters as in `initdb` running standard packaged binaries, each user having its own set of processes running with only his privileges. So when applying your idea (well, my understanding of it), they would be happy with a $SHAREDIR per initdb. >> Having extension_control_path also allows to install extension files in >> a place not owned by root. > > As far as the control files go, there's nothing saying that > $SHAREDIR/extension has to be root-owned. If there are .so's involved, > I do not believe the Red Hat crew is asking you to support loading .so's > from non-root-owned dirs, because that'd be against their own corporate > security policies. (But in any case, where we find the control and SQL > files need not have anything to do with where the .so's are.) But you can have a single $SHAREDIR per set of executables, right? Please read the following email to know what they asked for and how they do operate OpenShift: http://www.postgresql.org/message-id/341087492.2585530.1376776393038.JavaMail.root@redhat.com Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
> But you can have a single $SHAREDIR per set of executables, right? > > Please read the following email to know what they asked for and how they > do operate OpenShift: > > http://www.postgresql.org/message-id/341087492.2585530.1376776393038.JavaMail.root@redhat.com FWIW, I'm talking with Amazon later this week and checking how they're handling their tenant-loadable extensions. I'd like to come up with one solution here which covers all cloud providers. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 15 January 2014 03:07, Stephen Frost <sfrost@snowman.net> wrote: > For my 2c, I could absolutely see it as being worthwhile to have an > independent directory to install not-from-package extensions. That > would keep things which are "managed by the package system" and things > which are installed independent separate, which is absolutely a good > thing, imv. Another use case previously mentioned is that it makes user-installable extensions for developer-targeted bundles like Postgres.app possible. Postgres.app ships with contrib and a few other extensions by default, but if you want to install more, you have to chuck them into extensions dir inside the app bundle itself, so minor updates which replace the bundle will then lose your installed extensions. A nicer approach would be to allow it to also look for extensions under ~/Library/ as well as in the bundled distribution, but that's not possible if postgres only looks in one place, short of hand hacking fs links in the extension dir. Cheers Tom
Hi.
I can't apply the patch.
$ git apply --stat ~/Downloads/extension_control_path.v0.patch
--
fatal: unrecognized input
2014/1/14 Dimitri Fontaine <dimitri@2ndquadrant.fr>
Hi,
Please find attached to this email a patch implementing a new GUC that
allows users to setup a list of path where PostgreSQL will search for
the extension control files at CREATE EXTENSION time.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Best regards,
Sergey MuraviovSergey Muraviov <sergey.k.muraviov@gmail.com> writes: > I can't apply the patch. Did you try using the `patch`(1) command? The PostgreSQL project policy is to not use the git format when sending patches to the mailing list, prefering the context diff format. So you need to resort to using the basic patch commands rather than the modern git tooling. See also: http://wiki.postgresql.org/wiki/Submitting_a_Patch Patches must be in a format which provides context (eg: Context Diff); 'normal' or 'plain' diff formats are not acceptable. The following email might be useful for you: http://www.postgresql.org/message-id/CAOR=d=0q0DaL0BNZTsDdNWPgM5EjkXUykj7m+QsQbR728EOKCA@mail.gmail.com Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
<div dir="ltr"><div class="gmail_extra"><br />On Fri, Jan 24, 2014 at 6:57 AM, Dimitri Fontaine <<a href="mailto:dimitri@2ndquadrant.fr">dimitri@2ndquadrant.fr</a>>wrote:<br />><br />> Sergey Muraviov <<a href="mailto:sergey.k.muraviov@gmail.com">sergey.k.muraviov@gmail.com</a>>writes:<br /> > > I can't apply the patch.<br/>><br />> Did you try using the `patch`(1) command?<br />><br />> The PostgreSQL project policy isto not use the git format when sending<br />> patches to the mailing list, prefering the context diff format. So you<br/> > need to resort to using the basic patch commands rather than the modern<br />> git tooling. See also:<br/>><br />> <a href="http://wiki.postgresql.org/wiki/Submitting_a_Patch">http://wiki.postgresql.org/wiki/Submitting_a_Patch</a><br/> ><br/>> Patches must be in a format which provides context (eg: Context<br />> Diff); 'normal' or 'plain'diff formats are not acceptable.<br />><br /><br />Would be nice if we can use "git apply" command...<br /><br/>:-)<br /><br />--<br />Fabrízio de Royes Mello<br />Consultoria/Coaching PostgreSQL<br />>> Timbira: <a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/>>> Blog sobre TI: <a href="http://fabriziomello.blogspot.com">http://fabriziomello.blogspot.com</a><br/> >> Perfil Linkedin: <a href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a></div></div>
Hi
Now patch applies cleanly and works. :-)
But I have some notes:
1. There is an odd underscore character in functions find_in_extension_control_path and list_extension_control_paths:
\"extension_control__path\""
2. If we have several versions of one extension in different directories (which are listed in extension_control_path parameter) then we
get strange output from pg_available_extensions and pg_available_extension_versions views (Information about extension, whose path is at the beginning of the list, is duplicated). And only one version of the extension can be created.
See examples:
/extensions/
├── postgis-2.0.4
│ ├── postgis--2.0.4.sql
│ └── postgis.control
└── postgis-2.1.1
├── postgis--2.1.1.sql
└── postgis.control
=================================================
postgresql.conf:
extension_control_path = '/extensions/postgis-2.0.4:/extensions/postgis-2.1.1'
postgres=# table pg_catalog.pg_available_extensions;
name | default_version | installed_version | comment
---------+-----------------+-------------------+---------------------------------------------------------------------
postgis | 2.0.4 | | PostGIS geometry, geography, and raster spatial types and functions
postgis | 2.0.4 | | PostGIS geometry, geography, and raster spatial types and functions
(2 rows)
postgres=# table pg_catalog.pg_available_extension_versions;
name | version | installed | superuser | relocatable | schema | requires | comment
---------+---------+-----------+-----------+-------------+--------+----------+---------------------------------------------------------------------
postgis | 2.0.4 | f | t | t | | | PostGIS geometry, geography, and raster spatial types and functions
postgis | 2.0.4 | f | t | t | | | PostGIS geometry, geography, and raster spatial types and functions
(2 rows)
=================================================
postgresql.conf:
extension_control_path = '/extensions/postgis-2.1.1:/extensions/postgis-2.0.4'
postgres=# table pg_catalog.pg_available_extensions;
name | default_version | installed_version | comment
---------+-----------------+-------------------+---------------------------------------------------------------------
postgis | 2.1.1 | | PostGIS geometry, geography, and raster spatial types and functions
postgis | 2.1.1 | | PostGIS geometry, geography, and raster spatial types and functions
(2 rows)
postgres=# create extension postgis;
CREATE EXTENSION
postgres=# SELECT PostGIS_version();
postgis_version
---------------------------------------
2.1 USE_GEOS=1 USE_PROJ=1 USE_STATS=1
(1 row)
postgres=# table pg_catalog.pg_available_extensions;
name | default_version | installed_version | comment
---------+-----------------+-------------------+---------------------------------------------------------------------
postgis | 2.1.1 | 2.1.1 | PostGIS geometry, geography, and raster spatial types and functions
postgis | 2.1.1 | 2.1.1 | PostGIS geometry, geography, and raster spatial types and functions
(2 rows)
3. It would be fine to see an extension control path in pg_available_extensions and pg_available_extension_versions views (in separate column or within of extension name).
4. Perhaps the CREATE EXTENSION command should be improved to allow creation of the required version of the extension.
So we can use different versions of extensions in different databases.
PS
Sorry for my English.
2014/1/24 Fabrízio de Royes Mello <fabriziomello@gmail.com>
Would be nice if we can use "git apply" command...
On Fri, Jan 24, 2014 at 6:57 AM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:
>
> Sergey Muraviov <sergey.k.muraviov@gmail.com> writes:
> > I can't apply the patch.
>
> Did you try using the `patch`(1) command?
>
> The PostgreSQL project policy is to not use the git format when sending
> patches to the mailing list, prefering the context diff format. So you
> need to resort to using the basic patch commands rather than the modern
> git tooling. See also:
>
> http://wiki.postgresql.org/wiki/Submitting_a_Patch
>
> Patches must be in a format which provides context (eg: Context
> Diff); 'normal' or 'plain' diff formats are not acceptable.
>
:-)
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Best regards,
Sergey MuraviovOn Fri, Jan 24, 2014 at 6:57 AM, Fabrízio de Royes Mello <fabriziomello@gmail.com> wrote: > Would be nice if we can use "git apply" command... "git apply" seems to have raised pedantry to an art form. Not only won't it apply patches in any format other than the one it likes, it'll fail to apply any part of the patch if there are any failing hunks; I don't think it tolerates fuzz, either. You can override some of these behaviors but not all of them. It seems like somebody designed this tool more with the idea of preventing people from applying patches than actually doing it. "patch", on the other hand, makes the very reasonable assumption that if you didn't want to apply the patch, you wouldn't have run the "patch" command in the first place. It does its best to make sense of whatever you feed it, and if it can't apply the whole thing, it still applies as much as it can. I find this much more desirable behavior. It may be the policy of other projects to reject patches for trivial formatting mistakes or minor fuzz, but it's not the policy here, and I think that's a good thing. We typically bounce things for rebasing if there are actual rejects, but not otherwise. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
I haven't actually looked at the patch itself, but I noted this from the other review:
On Fri, Jan 24, 2014 at 6:43 PM, Sergey Muraviov <sergey.k.muraviov@gmail.com> wrote:
=================================================postgresql.conf:extension_control_path = '/extensions/postgis-2.0.4:/extensions/postgis-2.1.1'
Using colon as the path separator is going to break on windows. The patch notices this and uses semicolon on Windows instead. Do we really want to go down that path - that means that everybody who writes any sorts of installation instructions including this will have to make them separate for different platforms. Shouldn't we just use semicolon on all platforms, for consistency?
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes: > Using colon as the path separator is going to break on windows. The patch > notices this and uses semicolon on Windows instead. Do we really want to go > down that path - that means that everybody who writes any sorts of > installation instructions including this will have to make them separate > for different platforms. Shouldn't we just use semicolon on all platforms, > for consistency? Semicolon, being a valid filename character on most platforms (dunno about Windows), isn't a terribly good choice either. Since I disagree with the goal of this patch in the first place, I'm disinclined to spend brain cells on inventing a more robust format for a list of path names. I'm sure there is one though, if you're giving up on being consistent with traditional PATH format. regards, tom lane
Magnus Hagander <magnus@hagander.net> writes: > Using colon as the path separator is going to break on windows. The patch > notices this and uses semicolon on Windows instead. Do we really want to go > down that path - that means that everybody who writes any sorts of > installation instructions including this will have to make them separate > for different platforms. Shouldn't we just use semicolon on all platforms, > for consistency? Well, I've been considering that what I found already in the backend to solve the same problem was a valid model to build against. Pick any reasonnable choice you want to, fix dynamic_library_path along the new lines or maybe ask me to, and then let's apply the same design to the new GUC doing about exactly the same thing? Tom Lane <tgl@sss.pgh.pa.us> writes: > Since I disagree with the goal of this patch in the first place, I'm Should we remove dynamic_library_path? If not, why do we keep it? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Sat, Jan 25, 2014 at 6:07 PM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:
Magnus Hagander <magnus@hagander.net> writes:Well, I've been considering that what I found already in the backend to
> Using colon as the path separator is going to break on windows. The patch
> notices this and uses semicolon on Windows instead. Do we really want to go
> down that path - that means that everybody who writes any sorts of
> installation instructions including this will have to make them separate
> for different platforms. Shouldn't we just use semicolon on all platforms,
> for consistency?
solve the same problem was a valid model to build against.
Pick any reasonnable choice you want to, fix dynamic_library_path along
the new lines or maybe ask me to, and then let's apply the same design
to the new GUC doing about exactly the same thing?
Ha, I didn't realize dynamic_library_paty had the same problem. In fact, I have to admit I didn't realize I could put more than one path in there - I don't think I've ever used that :D
So based on the previous behaviour there, I withdraw my comment - being consistent with the existing behaviour of that parameter makes perfect sense.
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Hi, Sergey Muraviov <sergey.k.muraviov@gmail.com> writes: > Now patch applies cleanly and works. :-) Cool ;-) > But I have some notes: > > 1. There is an odd underscore character in functions > find_in_extension_control_path and list_extension_control_paths: > \"extension_control__path\"" Fixed in the new version of the patch, attached. > 2. If we have several versions of one extension in different directories > (which are listed in extension_control_path parameter) then we > get strange output from pg_available_extensions and > pg_available_extension_versions views (Information about extension, whose > path is at the beginning of the list, is duplicated). And only one version > of the extension can be created. Fixed. > 3. It would be fine to see an extension control path > in pg_available_extensions and pg_available_extension_versions views (in > separate column or within of extension name). I think the on-disk location is an implementation detail and decided in the attached version not to change those system view definitions. > 4. Perhaps the CREATE EXTENSION command should be improved to allow > creation of the required version of the extension. > So we can use different versions of extensions in different databases. Fixed in the attached. I also fixed ALTER EXTENSION UPDATE to search for udpate scripts in the same directory where the main control file is found, but I suspect this part requires more thinking. When we ALTER EXTENSION UPDATE we might now have several places where we find extname.control files, with possibly differents default_version properties. In the attached, we select the directory containing the control file where default_version matches the already installed extension version. That matches with a model where the new version of the extension changes the default_version in an auxiliary file. We might want to instead match on the default_version in the control file to match with the new version we are asked to upgrade to. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Вложения
Hi.
Now it looks fine for me.
--
2014-01-28 Dimitri Fontaine <dimitri@2ndquadrant.fr>:
Hi,
Cool ;-)Fixed in the new version of the patch, attached.
> But I have some notes:
>
> 1. There is an odd underscore character in functions
> find_in_extension_control_path and list_extension_control_paths:
> \"extension_control__path\""Fixed.
> 2. If we have several versions of one extension in different directories
> (which are listed in extension_control_path parameter) then we
> get strange output from pg_available_extensions and
> pg_available_extension_versions views (Information about extension, whose
> path is at the beginning of the list, is duplicated). And only one version
> of the extension can be created.I think the on-disk location is an implementation detail and decided in
> 3. It would be fine to see an extension control path
> in pg_available_extensions and pg_available_extension_versions views (in
> separate column or within of extension name).
the attached version not to change those system view definitions.Fixed in the attached.
> 4. Perhaps the CREATE EXTENSION command should be improved to allow
> creation of the required version of the extension.
> So we can use different versions of extensions in different databases.
I also fixed ALTER EXTENSION UPDATE to search for udpate scripts in the
same directory where the main control file is found, but I suspect this
part requires more thinking.
When we ALTER EXTENSION UPDATE we might now have several places where we
find extname.control files, with possibly differents default_version
properties.
In the attached, we select the directory containing the control file
where default_version matches the already installed extension version.
That matches with a model where the new version of the extension changes
the default_version in an auxiliary file.
We might want to instead match on the default_version in the control
file to match with the new version we are asked to upgrade to.
Best regards,
Sergey MuraviovOn Jan 30, 2014, at 10:06 AM, Sergey Muraviov <sergey.k.muraviov@gmail.com> wrote: > Now it looks fine for me. Just as another data point, I recently submitted pgTAP to the Homebrew project This is the build-from-source system for OSX, used by a lot of web developers. In my build script, I originally had depends_on :postgresql Which means, “require any version of PostgreSQL.” But then tests failed on OS X Server, which includes a system-distributedPostgreSQL. Homebrew installs everything in /usr/local, and not only does it disallow installing anythingoutside of that directory, it doesn’t have any permissions to do so. The install failed, of course, because extensionswant to install in $PGROOT/share/extensions. For now, I had to change it to depends_on 'postgresql' A subtle difference that means, “require the latest version of the Homebrew-built PostgreSQL in /usr/local.” However, if extension_control_path was supported, I could change it back to requiring any Postgres and install pgTAP somewhereunder /usr/local, as required for Homebrew. Then all the user would have to do to use it with their preferred Postgreswould be to set extension_control_path. In other words, I am strongly in favor of this patch, as it gives distribution systems a lot more flexibility (for betterand for worse) in determining where extensions should be installed. My $0.02. Best, David
On Tue, Feb 4, 2014 at 6:07 PM, David E. Wheeler <david@justatheory.com> wrote: > The install failed, of course, because extensions want to install in $PGROOT/share/extensions. Homebrew sounds kind of confused. Having a non-root user have access to make global system changes sounds like privilege escalation vulnerability by design. However putting that aside, it is fairly standard for software to provide two directories for extensions/modules/plugins/etc. One for distribution-built software such as /usr/share/emacs/site-lisp/ and another for sysadmin customizations such as /usr/local/share/emacs/site-lisp. The same idea as /usr/share/perl and /usr/local/share/perl or with Python or anything else. -- greg
* Greg Stark (stark@mit.edu) wrote: > On Tue, Feb 4, 2014 at 6:07 PM, David E. Wheeler <david@justatheory.com> wrote: > > The install failed, of course, because extensions want to install in $PGROOT/share/extensions. > > Homebrew sounds kind of confused. Having a non-root user have access > to make global system changes sounds like privilege escalation > vulnerability by design. I've not played w/ Homebrew myself, but it's installing into /usr/local and presumably that includes installing things into /usr/local/bin, so the notion that installing something from Homebrew isn't already (and intended to be) making global system changes doesn't quite line up. The end-admin would have to modify the system-installed postgresql.conf anyway to enable this other directory. David wasn't suggesting that Homebrew *should* be able to do so, he was pointing out that it *can't*, which all makes sense imv. > However putting that aside, it is fairly standard for software to > provide two directories for extensions/modules/plugins/etc. One for > distribution-built software such as /usr/share/emacs/site-lisp/ and > another for sysadmin customizations such as > /usr/local/share/emacs/site-lisp. The same idea as /usr/share/perl and > /usr/local/share/perl or with Python or anything else. Agreed. Thanks, Stephen
On Feb 6, 2014, at 6:51 AM, Greg Stark <stark@mit.edu> wrote: > Homebrew sounds kind of confused. Having a non-root user have access > to make global system changes sounds like privilege escalation > vulnerability by design. Well, the point is that it *doesn’t* make global system changes. I got an error on OS X Server with my original formula,because there was no permission to install in $PGROOT/share/extensions. > However putting that aside, it is fairly standard for software to > provide two directories for extensions/modules/plugins/etc. One for > distribution-built software such as /usr/share/emacs/site-lisp/ and > another for sysadmin customizations such as > /usr/local/share/emacs/site-lisp. The same idea as /usr/share/perl and > /usr/local/share/perl or with Python or anything else. Right. And you can also add additional paths for those applications to search. Best, David
On Feb 6, 2014, at 7:32 AM, Stephen Frost <sfrost@snowman.net> wrote: > The end-admin would have to modify the system-installed postgresql.conf > anyway to enable this other directory. David wasn't suggesting that > Homebrew *should* be able to do so, he was pointing out that it *can't*, > which all makes sense imv. Yeah, or be able to add a directory as a Postgres super user at runtime. David
On Thu, Feb 6, 2014 at 5:49 PM, David E. Wheeler <david@justatheory.com> wrote: > On Feb 6, 2014, at 6:51 AM, Greg Stark <stark@mit.edu> wrote: > >> Homebrew sounds kind of confused. Having a non-root user have access >> to make global system changes sounds like privilege escalation >> vulnerability by design. > > Well, the point is that it *doesn't* make global system changes. I got an error on OS X Server with my original formula,because there was no permission to install in $PGROOT/share/extensions. Installing into /usr/local is a global system change. Only root should be able to do that and any user that can do that can easily acquire root privileges. >> However putting that aside, it is fairly standard for software to >> provide two directories for extensions/modules/plugins/etc. One for >> distribution-built software such as /usr/share/emacs/site-lisp/ and >> another for sysadmin customizations such as >> /usr/local/share/emacs/site-lisp. The same idea as /usr/share/perl and >> /usr/local/share/perl or with Python or anything else. > > Right. And you can also add additional paths for those applications to search. Well, users can do whatever they want at run-time but there are blessed paths that are the correct place to install things that these systems are configured to search automatically. My point was just that there are generally two such blessed paths, one for the distribution and one for the local sysadmin. What you do not want is to have a different path for each piece of software. That way lies the /usr/local/kde/bin:/usr/local/gnome/bin:/usr/local/myfavouritehack/bin:... madness. You can do this with Python or Perl but they won't do it automatically and everyone who does this with environment variables or command line flags eventually realizes what a mess it is. (Except Java programmers) -- greg
On Feb 6, 2014, at 9:14 AM, Greg Stark <stark@mit.edu> wrote: > Installing into /usr/local is a global system change. Only root should > be able to do that and any user that can do that can easily acquire > root privileges. I agree with you, but I don’t think the Homebrew folks do. Or at least their current implementation doesn’t. OT though. > Well, users can do whatever they want at run-time but there are > blessed paths that are the correct place to install things that these > systems are configured to search automatically. My point was just that > there are generally two such blessed paths, one for the distribution > and one for the local sysadmin. Yeah, two blessed would be very useful, but I think the ability to add any number of paths would be even better. > What you do not want is to have a different path for each piece of > software. That way lies the > /usr/local/kde/bin:/usr/local/gnome/bin:/usr/local/myfavouritehack/bin:... > madness. You can do this with Python or Perl but they won't do it > automatically and everyone who does this with environment variables or > command line flags eventually realizes what a mess it is. (Except Java > programmers) Agreed. Best, David
Hi, On 06/02/14 18:14, Greg Stark wrote: > Installing into /usr/local is a global system change. Only root should > be able to do that and any user that can do that can easily acquire > root privileges. The idea behind Homebrew is copied from FreeBSD, where you also install 3rd party software to /usr/local. This is felt as cleaner and nicer by these guys. Homebrew goes one step further: with Homebrew you are able to completely remove all 3rd party software installed via Homebrew as well as Homebrew itself by simply removing /usr/local. And since most of the time OS X is used as a desktop software, they simplified things for users by chown-ing /usr/local (which, in a clean OS X installation, is either empty or does not exist, depending on the version) at installation time to the user installing Homebrew. Of course you can avoid this by installing Homebrew as root, but using the root user is not very popular in OS X land. Best regards, -- Christian Kruse http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
I'm massively in favor of this feature. (I had started writing it about three times myself.) The problem I see, however, is that most extensions, by recommendation, set module_pathname = '$libdir/pgfoo', and so relocating the control files will still end up pointing to a not relocated library file. We would need to remove that and then ask users to keep their dynamic_library_path in sync with extension_control_path. That's error prone, of course. In order to address this properly, we need a new directory structure that keeps library files and control files together, similar to how Python, Ruby, etc. install things, and then just one path for everything. Now a few technical problems. When an extension is not found, I get the error message ERROR: could not open extension control file "(null)": Bad address Something is broken there. In the documentation, order extension_control_path after dynamic_library_path. Also, the documentation states that this controls the location of the control file, but it of course controls the location of the script files also. That should be made clearer. (It becomes clearer if we just have one path for everything. ;-) )
* Peter Eisentraut (peter_e@gmx.net) wrote: > I'm massively in favor of this feature. (I had started writing it about > three times myself.) Agreed. > The problem I see, however, is that most extensions, by recommendation, > set module_pathname = '$libdir/pgfoo', and so relocating the control > files will still end up pointing to a not relocated library file. I was wondering how that was dealt with- I simply have not had time to get to looking at this in more detail. I thought the answer I got from Dimitri was that $libdir would actually end up being resolved to any of the available directories due to his other patch...? Perhaps we just need to add in to that list the alternate directory for the control files? Or, what I had been thinking at one point, was making $libdir actually be "where this control file lives" instead. There's risk there though, I suppose, as today only one thing means $libdir. Another thought that I had was dealing with possible overlaps and clarifying things, perhaps such as having a mapping of 'name' to 'directory' which remaps $libdir for that 'name' and then extensions would be created using the 'name' space. eg: set module_paths = 'mymodulepath:/path/to/whatever;nextmodulepath:/other'; CREATE EXTENSION mymodulepath:my_extension; > We would need to remove that and then ask users to keep their > dynamic_library_path in sync with extension_control_path. That's error > prone, of course. I'm starting to regret that dynamic_library_path exists, tbh. Still, I don't think it'd be too bad to automatically add to that path (or to what's searched) the module_paths. > In order to address this properly, we need a new directory structure > that keeps library files and control files together, similar to how > Python, Ruby, etc. install things, and then just one path for everything. Right, that's more-or-less what I was thinking module_path would be. > Now a few technical problems. Agree with all these. Thanks, Stephen
Hi, Peter Eisentraut <peter_e@gmx.net> writes: > I'm massively in favor of this feature. (I had started writing it about > three times myself.) Thanks! > The problem I see, however, is that most extensions, by recommendation, > set module_pathname = '$libdir/pgfoo', and so relocating the control > files will still end up pointing to a not relocated library file. It's kind of true. Is the phrasing “typically” followed by an example really a recommendation though? I though it was more a detailed explanation of the way it works. We still have several other ways to tell PostgreSQL which lib to use for each and every LANGUAGE C function: - $libdir/soname - absolute/path - MODULE_PATHNAME - any/relative/path which is to be solved in dynamic_library_path Also, editing the AS '$libdir/foo' occurences from an SQL script is a quite very easy thing to do programmatically. > We would need to remove that and then ask users to keep their > dynamic_library_path in sync with extension_control_path. That's error > prone, of course. I don't see any pressure in changing the way things currently work after adding this new GUC in. As you say, when extension_control_path is used then some extra work *might need* to be done in order to ensure that the right library is getting loaded. I mainly see that as a distribution/distributor problem tho. > In order to address this properly, we need a new directory structure > that keeps library files and control files together, similar to how > Python, Ruby, etc. install things, and then just one path for everything. It might be true, be it reads to me like you're omiting the directory parameter from the control file: the scripts and auxilliary control files might be found anywhere else on the file system already. Again, my view is that if you want to do things in a non-standard way then you need to tweak the control file and maybe the script files. It's a distribution problem, and I'm solving it in an extra software layer. PostgreSQL is very flexible about where to organise extension files currently, *except* for the control file. This patch is providing the same level of flexibility to this part. Of course flexibility can be seen as creating a mess, but I don't think it's this patch nor PostgreSQL core to solve that mess. > Now a few technical problems. Will see about fixing those later, by friday given my current schedule, thanks. > Also, the documentation states that this controls the location of the > control file, but it of course controls the location of the script files > also. That should be made clearer. (It becomes clearer if we just have > one path for everything. ;-) ) Again, we have directory = 'whatever' in the control file to control where to find the script files. I'm not sure your "of course" follows. Will still edit docs. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri, * Dimitri Fontaine (dimitri@2ndQuadrant.fr) wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > The problem I see, however, is that most extensions, by recommendation, > > set module_pathname = '$libdir/pgfoo', and so relocating the control > > files will still end up pointing to a not relocated library file. > > It's kind of true. Is the phrasing “typically” followed by an example > really a recommendation though? I though it was more a detailed > explanation of the way it works. It probably sufficient to say that nearly all the ones out there are this way, so I don't know that it really matters if the userbase consideres it a recommendation or just documentation- that's what they're doing. I admit, I've not gone and looked through PGXN explicitly to check this, but I'm pretty sure I'm right because every one I've ever seen has done it this way. > Also, editing the AS '$libdir/foo' occurences from an SQL script is a > quite very easy thing to do programmatically. Sure, but I tend to agree with Peter about it not being a terribly great solution to the problem. > I mainly see that as a distribution/distributor problem tho. This comment doesn't really make any sense to me. My hope is to have some kind of automated process to build .debs for projects on PGXN but people are still going to be able to download them directly. If what's in PGXN matches what the debs have, then people downloading directly will have to modify everything. Perhaps PGXN could be modified to make it something different for people downloading directly and then the debs could switch it to $libdir- except, what would they set it to..? And we'd be asking every single author to make a change to their module, which is a pretty terrible thing to do. > > In order to address this properly, we need a new directory structure > > that keeps library files and control files together, similar to how > > Python, Ruby, etc. install things, and then just one path for everything. > > It might be true, be it reads to me like you're omiting the directory > parameter from the control file: the scripts and auxilliary control > files might be found anywhere else on the file system already. This is true and Debian puts the control/sql files into a different directory than the .so files, of course. Still, the issue here is how we find the .so files- the user *has* to tell us where the control file is, if it isn't in the default location, and the assumption (default?) is then that the .sql files are co-located with them. It's at that point when we get to the point of trying to figure out what $libdir is and it strikes me that when users use this approach the expectation will be that it's in the same directory as the control and .sql files. Making that 'just work' has some definite advantages, though I also share the concern of others (who I don't think have chimed in on this particular topic thus far, so I'm really just guessing and it might be just me) that we probably don't want to just start having the ability to load .so files from anywhere- except, well, the superuser can already do that anyway using C functions, so perhaps that isn't really a concern? I'm still worired about the *conflicts* issue, where you might have a .so from a -contrib install in $libdir, with .sql/.control files from downloading the extension itself, hence my suggestion to actually use a different namespace for extensions which exist outside of the normal directories. I'm thinking that would also help people doing dump/restore to realize when they forget to pull in the 'updated' module from PGXN rather than using the shipped-with-distro version. Of course, you still run the risk that such a dump might not work upon restore (same as we have today w/ C functions whose .so's have vanished- try to call them and you get an error back), though perhaps in this case we'd end up with a dump that won't restore, right..? That's not ideal either, of course, but going back to the C functions again, it's possible you could have a functional index using a custom C functions and it's the same issue all over again. > Again, my view is that if you want to do things in a non-standard way > then you need to tweak the control file and maybe the script files. It's > a distribution problem, and I'm solving it in an extra software layer. That's a pretty reasonable option for this specific issue, but it doesn't address the duplication problem, which worries me. > PostgreSQL is very flexible about where to organise extension files > currently, *except* for the control file. This patch is providing the > same level of flexibility to this part. Of course flexibility can be > seen as creating a mess, but I don't think it's this patch nor > PostgreSQL core to solve that mess. Yeah, but we should be trying to avoid practices and configurations which encourage mess creation. :) > > Also, the documentation states that this controls the location of the > > control file, but it of course controls the location of the script files > > also. That should be made clearer. (It becomes clearer if we just have > > one path for everything. ;-) ) > > Again, we have directory = 'whatever' in the control file to control > where to find the script files. I'm not sure your "of course" follows. > Will still edit docs. Yeah, but it seems to be pretty rarely used and the expectation is that the .sql files resides in the same directory. I think what we're looking for here, in some ways, is for that default for .so's to work out the same- except that right now, the users seem to all default to sticking in $libdir. Anyhow, I'd like to hear about how the duplication issue might be addressed (or your thoughts on my proposal) rather than worry overmuch about a relatively small tweak which we could make later (but before release) to address the $libdir problem; and, as you say, you could solve that with a distribution framework for PGXN anyway. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > This is true and Debian puts the control/sql files into a different > directory than the .so files, of course. Still, the issue here is how > we find the .so files- the user *has* to tell us where the control file > is, if it isn't in the default location, and the assumption (default?) > is then that the .sql files are co-located with them. It's at that > point when we get to the point of trying to figure out what $libdir is Ok you're migthy confused. The rules that PostgreSQL follows to know where to load the library from are not changed *at all* by this patch. In my book, it makes the whole topic irrelevant to the review. Futhermore, the rules in question come from two different elements: - the object file name in the AS clause, available *separately* for each and every function definition, to be found inthe script files: src/backend/commands/functioncmds.c:744 * For a dynamically linked C language object, the form of the clause is * * AS <object file name> [, <link symbol name> ] - the dynamic_library_path GUC that helps interpreting the object file name when it's not absolute or when it contains$libdir as its first characters. If you want to change the rules and provide a way to resolve the object file name to use on a per-extension level, fee free to propose a patch. > Yeah, but it seems to be pretty rarely used and the expectation is that > the .sql files resides in the same directory. I think what we're > looking for here, in some ways, is for that default for .so's to work > out the same- except that right now, the users seem to all default to > sticking in $libdir. It used to be a script.sql.in containing AS 'MODULE_PATHNAME', which would then be replaced with $libdir by pgxs.mk (the rule is still here in the file). Nowadays we have the replacement facility in the backend, driven by the module_pathname property in the extension's control file. Contrib modules are still using the AS 'MODULE_PATHNAME' spelling with the extension control file spelling module_pathname = '$libdir/xxx'. Nothing changes with this patch other than where to find the extension control file. How to resolve the <object file name> on the file system is still the distribution and local admin problem. That the controlling of where to find the dynamic libs is convoluted and involves other people than just the PostgreSQL backend packager might be seen as a problem or a great flexibility, in any case I don't see what it has to do with reviewing the extension_control_path patch. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
* Dimitri Fontaine (dimitri@2ndQuadrant.fr) wrote: > The rules that PostgreSQL follows to know where to load the library from > are not changed *at all* by this patch. In my book, it makes the whole > topic irrelevant to the review. I'm really quite tired of the constant dismissal of anything brought up by anyone regarding any changes about anything. I didn't suggest anywhere that the proposed patch changed the rules at all- instead I was trying to point out that by adding this functionality and *not* changing the way that lookup is done *is going to cause confusion*. [... quotes from the docs which aren't relevant ...] > If you want to change the rules and provide a way to resolve the object > file name to use on a per-extension level, fee free to propose a patch. Or, I could simply voice my opinion that this patch *should not go in* without such a change, or at *least* some thought and discussion about what the right answer is here. I'm evidently not alone with this concern either as it's exactly (as I understand it at least; I don't mean to put words into his mouth) what Peter *just* brought up too. I'd really appreciate it if you would stop trying to seperate every other possible thing to do with anything from this patch except the one little thing you want. This patch touches code related to extensions and it's necessary for us to consider it in that broader light. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > I didn't suggest anywhere that the proposed patch changed the rules at > all- instead I was trying to point out that by adding this functionality > and *not* changing the way that lookup is done *is going to cause > confusion*. I don't see any confusion about dynamic library name resolving added from the extension_control_path, I'm sorry. Simply because I don't expect people to use the facility without a third party software designed to fill-in the gap. You're saying that the backend should fill the gap, I'm saying that it should not. Or maybe within another patch entirely. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
* Dimitri Fontaine (dimitri@2ndQuadrant.fr) wrote: > I don't see any confusion about dynamic library name resolving added > from the extension_control_path, I'm sorry. Simply because I don't > expect people to use the facility without a third party software > designed to fill-in the gap. > > You're saying that the backend should fill the gap, I'm saying that it > should not. Or maybe within another patch entirely. I find this role reversal to be quite bizarre. Is this third-party software going to be modifying postgresql.conf too? Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > I find this role reversal to be quite bizarre. Who do you think should have a say about where to load the dynamic librairies from? hackers, packagers, system admins, dbas or users? Who do you think is currently editing the setup that decides where to load the dynamic librairies from, which is spread into SQL scripts, extension control file, postgresql.conf and pg_config --pkglibdir? What exactly are you calling bizarre in the idea that the PostgreSQL source code is maybe not the best way where to solve that problem from? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
* Dimitri Fontaine (dimitri@2ndQuadrant.fr) wrote: > Who do you think should have a say about where to load the dynamic > librairies from? hackers, packagers, system admins, dbas or users? My gut feeling on this is packages and sysadmins. Do you see it differently? I generally *don't* think DBAs-who-aren't-sysadmins should be doing it as they may not have any access to the underlying OS (or perhaps have access only to things like /home), which goes back to my concerns over ALTER SYSTEM, but that ship has obviously sailed. > Who do you think is currently editing the setup that decides where to > load the dynamic librairies from, which is spread into SQL scripts, > extension control file, postgresql.conf and pg_config --pkglibdir? I agree that packagers and sysadmins will be setting this up initially, but it strikes me as a bit silly to ask the sysadmins to go modify the control file path and then also have to modify the dynamic library load path when they're setting them to the same thing. Related to this, as I've asked repeatedly on this thread- what is the plan for dealing with namespace overlaps? As in, the admin happily goes in and sets dynamic_library_path to '$libdir:/path/to/new/hstore' and then tries to CREATE EXTENSION hstore; with the -contrib packages installed? They're going to get cryptic and bizarre error messages, at best during the CREATE EXTENSION and at worst when they actually try to run that one function whose interface changed, perhaps months after the initial install. Part of the reason that I'm pushing for a change here is to try and address that problem. I'd appreciate some feedback on it. > What exactly are you calling bizarre in the idea that the PostgreSQL > source code is maybe not the best way where to solve that problem from? I was referring to the apparent role reversal between us, with me trying to get PG to do more and you pushing to have more in an external tool. It wasn't that long ago that our positions were swapped. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > * Dimitri Fontaine (dimitri@2ndQuadrant.fr) wrote: >> Who do you think should have a say about where to load the dynamic >> librairies from? hackers, packagers, system admins, dbas or users? > > My gut feeling on this is packages and sysadmins. Do you see it +1 >> Who do you think is currently editing the setup that decides where to >> load the dynamic librairies from, which is spread into SQL scripts, >> extension control file, postgresql.conf and pg_config --pkglibdir? > > I agree that packagers and sysadmins will be setting this up initially, Not quite, because of the ability to ship absolute object file names in the SQL script and the extension control files, edited by hackers. The third party tool I'm talking about will have to edit those files at packaging time in order to get the control back to where you want it. > but it strikes me as a bit silly to ask the sysadmins to go modify the > control file path and then also have to modify the dynamic library load > path when they're setting them to the same thing. Well, the point is that if you edit the control file, then you don't have to care about the dynamic_library_path at all, because you're going to setup absolute object file names (or location). > Related to this, as I've asked repeatedly on this thread- what is the > plan for dealing with namespace overlaps? As in, the admin happily goes > in and sets dynamic_library_path to '$libdir:/path/to/new/hstore' and > then tries to CREATE EXTENSION hstore; with the -contrib packages > installed? My proposal is to edit the control file module_pathname property to the right absolute location within the new hstore binary packaging. That responsibility is then given to the new third party tool, aimed at both packagers and system admins. > Part of the reason that I'm pushing for a change here is to try and > address that problem. I'd appreciate some feedback on it. Within the way I see things, this problem just doesn't exist, by design. > I was referring to the apparent role reversal between us, with me trying > to get PG to do more and you pushing to have more in an external tool. > It wasn't that long ago that our positions were swapped. Well you know, I actually read my emails and learn from them. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
* Dimitri Fontaine (dimitri@2ndQuadrant.fr) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > * Dimitri Fontaine (dimitri@2ndQuadrant.fr) wrote: > >> Who do you think should have a say about where to load the dynamic > >> librairies from? hackers, packagers, system admins, dbas or users? > > > > My gut feeling on this is packages and sysadmins. Do you see it > > +1 > > >> Who do you think is currently editing the setup that decides where to > >> load the dynamic librairies from, which is spread into SQL scripts, > >> extension control file, postgresql.conf and pg_config --pkglibdir? > > > > I agree that packagers and sysadmins will be setting this up initially, > > Not quite, because of the ability to ship absolute object file names in > the SQL script and the extension control files, edited by hackers. > > The third party tool I'm talking about will have to edit those files at > packaging time in order to get the control back to where you want it. I'm a bit confused here- above you '+1'd packagers/sysadmins, but then here you are saying that hackers will be setting it? Also, it strikes me as a terrible idea to ship absolute object file names (which I assume you mean to include path, given you say 'absolute') unless you're an actual OS packaging system. I know OSX has some packaging system which lets you install things that aren't-quite-the-OS, can a package completely depend on the install path that the hacker decided upon for the package initially? It's not relocatable at all on a given system? Is there a 'shared' directory for all those packages into which each package can drop files? How is the naming for those files handled? Is there more than one such directory? Presumably, that's what you'd want to set both the control path and the dynamic extension path to- a directory of control files and a directory of .so's, or perhaps one combined directory of both, for the simplest setup. If you're working with a directory-per-package, then wouldn't you want to have everything for that package in that package's directory and then only have to add all those directories to one place in postgresql.conf? I've been trying to follow this thread pretty closely but perhaps I've missed it (or forgotten it) and, if so, my apologies, but how exactly are you envisioning PG, these GUCs, whichever OSX packaging system, and your external tool working together? > Well, the point is that if you edit the control file, then you don't > have to care about the dynamic_library_path at all, because you're going > to setup absolute object file names (or location). My questions about this are mostly covered above, but I did want to get clarification- is this going to be on a per-system basis, as in, when the package is installed through your tool, it's going to go figure out where the package got installed to and rewrite the control file? Seems like a lot of work if you're going to have to add that directory to the postgresql.conf path for the control file anyway to then *also* have to hack up the control file itself. > > Related to this, as I've asked repeatedly on this thread- what is the > > plan for dealing with namespace overlaps? As in, the admin happily goes > > in and sets dynamic_library_path to '$libdir:/path/to/new/hstore' and > > then tries to CREATE EXTENSION hstore; with the -contrib packages > > installed? > > My proposal is to edit the control file module_pathname property to the > right absolute location within the new hstore binary packaging. That > responsibility is then given to the new third party tool, aimed at both > packagers and system admins. I can see some of the simplicity in that, though it strikes me as being more-or-less equivilant to just searching the directory where the control file exists first, if it isn't in the PG default, with the added benefit that moving the base-install location for the modules would only require updating the postgresql.conf rather than having to update it and then also go modify every control file. > > Part of the reason that I'm pushing for a change here is to try and > > address that problem. I'd appreciate some feedback on it. > > Within the way I see things, this problem just doesn't exist, by design. I understand your proposal better now that I understand how you're planning to use it, but I'm still of the opinion that we might be able to do better by our users by not hard-coding paths into every control file. > Well you know, I actually read my emails and learn from them. I think we all aspire to do that. :) Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > I'm a bit confused here- above you '+1'd packagers/sysadmins, but then > here you are saying that hackers will be setting it? Also, it strikes Well I was then talking about how it works today, as in PostgreSQL 9.1, 9.2 and 9.3, and most certainly 9.4 as we're not trying to change anything on that front. > me as a terrible idea to ship absolute object file names (which I assume > you mean to include path, given you say 'absolute') unless you're an I agree, that's why my current design also needs cooperation on the backend side of things, to implement what you're calling here relocation of the files. Now that I read your comments, we might be able to implement something really simple and have something in core… Please see attached patch, tested and documented. doc/src/sgml/extend.sgml | 7 ++++++ src/backend/commands/extension.c | 39 +++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) > Presumably, that's what you'd want to set both the control path and the > dynamic extension path to- a directory of control files and a directory > of .so's, or perhaps one combined directory of both, for the simplest > setup. If you're working with a directory-per-package, then wouldn't > you want to have everything for that package in that package's directory > and then only have to add all those directories to one place in > postgresql.conf? That's a fair-enough observation, that targets a use case where you're using the feature without the extra software. I also note that it could simplify said software a little bit. What about allowing a control file like this: # hstore extension comment = 'data type for storing sets of (key, value) pairs' default_version = '1.3' directory = 'local/hstore-new' module_pathname = '$directory/hstore' relocatable = true The current way directory is parsed, relative pathnames are allowed and will be resolved in SHAREDIR, which is where we find the extension/ main directory, where currently live extension control files. With such a feature, we would allow module_pathname to reuse the same location as where we're going to find auxilliary control files and scripts. > My questions about this are mostly covered above, but I did want to get > clarification- is this going to be on a per-system basis, as in, when > the package is installed through your tool, it's going to go figure out > where the package got installed to and rewrite the control file? Seems > like a lot of work if you're going to have to add that directory to the > postgresql.conf path for the control file anyway to then *also* have to > hack up the control file itself. Given module_pathname = '$directory/xxx' the extension is now fully relocatable and the tool doesn't need to put in any other effort than hacking the control file *at build time*. See the attached patch that implements the idea. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Вложения
* Dimitri Fontaine (dimitri@2ndQuadrant.fr) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > me as a terrible idea to ship absolute object file names (which I assume > > you mean to include path, given you say 'absolute') unless you're an > > I agree, that's why my current design also needs cooperation on the > backend side of things, to implement what you're calling here relocation > of the files. Now that I read your comments, we might be able to > implement something really simple and have something in core… I didn't really expect this to be a huge issue or hard problem to implement.. :) > Please see attached patch, tested and documented. On a quick glance-over, it looks like a reasonable implementation to me. > What about allowing a control file like this: > > # hstore extension > comment = 'data type for storing sets of (key, value) pairs' > default_version = '1.3' > directory = 'local/hstore-new' > module_pathname = '$directory/hstore' > relocatable = true Interesting idea. I'm a *little* concerned that re-useing '$directory' there might confuse people into thking that any values in the control file could be substituted in a similar way though. Would there really be much difference between that and '$ctldir' or something? > The current way directory is parsed, relative pathnames are allowed and > will be resolved in SHAREDIR, which is where we find the extension/ main > directory, where currently live extension control files. Sure, though it's unlikely to be used much there, as it's managed by the packagers. > With such a feature, we would allow module_pathname to reuse the same > location as where we're going to find auxilliary control files and > scripts. Right- they'd be able to have everything in a single directory, presumably one where they're doing development or where some other installers installs to. > Given module_pathname = '$directory/xxx' the extension is now fully > relocatable and the tool doesn't need to put in any other effort than > hacking the control file *at build time*. Right. Peter, any thoughts on this approach..? Thanks, Stephen
On 2/27/14, 6:04 AM, Dimitri Fontaine wrote: > What about allowing a control file like this: > > # hstore extension > comment = 'data type for storing sets of (key, value) pairs' > default_version = '1.3' > directory = 'local/hstore-new' > module_pathname = '$directory/hstore' > relocatable = true > > The current way directory is parsed, relative pathnames are allowed and > will be resolved in SHAREDIR, which is where we find the extension/ main > directory, where currently live extension control files. > > With such a feature, we would allow module_pathname to reuse the same > location as where we're going to find auxilliary control files and > scripts. If I understand this correctly, then installing an extension in a nonstandard directory would require editing (or otherwise changing) the control file. That doesn't seem very attractive. In fact, it would fail my main use case for all of this, which is being able to test extensions before installing them. I think we should get rid of the module_pathname business, and extensions' SQL files should just refer to the base file name and rely on the dynamic library path to find the files. What would we lose if we did that?
* Peter Eisentraut (peter_e@gmx.net) wrote: > On 2/27/14, 6:04 AM, Dimitri Fontaine wrote: > > What about allowing a control file like this: > > > > # hstore extension > > comment = 'data type for storing sets of (key, value) pairs' > > default_version = '1.3' > > directory = 'local/hstore-new' > > module_pathname = '$directory/hstore' > > relocatable = true > > > > The current way directory is parsed, relative pathnames are allowed and > > will be resolved in SHAREDIR, which is where we find the extension/ main > > directory, where currently live extension control files. > > > > With such a feature, we would allow module_pathname to reuse the same > > location as where we're going to find auxilliary control files and > > scripts. > > If I understand this correctly, then installing an extension in a > nonstandard directory would require editing (or otherwise changing) the > control file. It depends on what's in the control file. What this would do is give developers another option for where the .so resides that means "a directory relative to the control file", which could be quite handy. > That doesn't seem very attractive. In fact, it would fail my main use > case for all of this, which is being able to test extensions before > installing them. Not sure why that wouldn't work...? > I think we should get rid of the module_pathname business, and > extensions' SQL files should just refer to the base file name and rely > on the dynamic library path to find the files. What would we lose if we > did that? As I pointed out up-thread, you'd have to keep adding more and more directories to both the control-filed-paths-to-search plus the dynamic-shared-libraries Thanks, Stephen
Peter Eisentraut <peter_e@gmx.net> writes: > I think we should get rid of the module_pathname business, and > extensions' SQL files should just refer to the base file name and rely > on the dynamic library path to find the files. What would we lose if we > did that? Control over *which* mylib.so file gets loaded for a specific sql script. That's the whole namespace issue Stephen is worried about. If you're testing the new version of an extension before installing it properly, then you will have the current and the new versions of the .so, with the exact same name, at different places. Note that when using base file name only, then you could also have a clash with a dynamic library of the same name installed on the system, even if not made to be loaded by PostgreSQL. Some extensions are using way too generic names. Hint: prefix.so. Regards, -- Dimitri http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Stephen Frost <sfrost@snowman.net> writes: >> # hstore extension >> comment = 'data type for storing sets of (key, value) pairs' >> default_version = '1.3' >> directory = 'local/hstore-new' >> module_pathname = '$directory/hstore' >> relocatable = true > > Interesting idea. I'm a *little* concerned that re-useing '$directory' > there might confuse people into thking that any values in the control > file could be substituted in a similar way though. Would there really > be much difference between that and '$ctldir' or something? Well, using $directory makes the feature auto-documented and very easy to read even without the reference documentation handy. It's also a very known way to setup things in .ini files. Now, what other parameters would you possibly use that way, other than $directory? I can see a use for $default_version, but that's about it. Would you rather add support for $default_version in the patch, for all of the parameters just in case, for a different set of control parameters, or rename the $directory macro? My vote goes for adding $default_version only. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
* Dimitri Fontaine (dimitri@2ndQuadrant.fr) wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > I think we should get rid of the module_pathname business, and > > extensions' SQL files should just refer to the base file name and rely > > on the dynamic library path to find the files. What would we lose if we > > did that? > > Control over *which* mylib.so file gets loaded for a specific sql > script. That's the whole namespace issue Stephen is worried about. Indeed. > If you're testing the new version of an extension before installing it > properly, then you will have the current and the new versions of the > .so, with the exact same name, at different places. Hrm. This makes me wonder if there was a way we could check a .so against the definition of what it "should" be in the control file. As in, somehow include the extension name and version in the PG_MODULE_MAGIC. That could be good on a couple of levels.. > Note that when using base file name only, then you could also have a > clash with a dynamic library of the same name installed on the system, > even if not made to be loaded by PostgreSQL. Such as addressing this- perhaps with a GUC that says "only load .so's that have a PG_MODULE_MAGIC and whose extension names/versions match what is in the associated control file". > Some extensions are using way too generic names. Hint: prefix.so. Agreed. Thanks! Stephen
* Dimitri Fontaine (dimitri@2ndQuadrant.fr) wrote: > Stephen Frost <sfrost@snowman.net> writes: > >> # hstore extension > >> comment = 'data type for storing sets of (key, value) pairs' > >> default_version = '1.3' > >> directory = 'local/hstore-new' > >> module_pathname = '$directory/hstore' > >> relocatable = true > > > > Interesting idea. I'm a *little* concerned that re-useing '$directory' > > there might confuse people into thking that any values in the control > > file could be substituted in a similar way though. Would there really > > be much difference between that and '$ctldir' or something? > > Well, using $directory makes the feature auto-documented and very easy > to read even without the reference documentation handy. It's also a very > known way to setup things in .ini files. > > Now, what other parameters would you possibly use that way, other than > $directory? I can see a use for $default_version, but that's about it. Yeah, default_version was the other one that looked like it might be possible to include, but folks might decide to try and use 'comment' in that way too. Basically, there's a chance that they'd want to use any string in there. > Would you rather add support for $default_version in the patch, for all > of the parameters just in case, for a different set of control > parameters, or rename the $directory macro? > My vote goes for adding $default_version only. It certainly seems to me that people will natuarlly want to use $default_version. I'm trying to figure out if that's actually something we should encourage or what. Of course, including it and/or the PG_MODULE_MAGIC that I mentioned in my previous email run afoul of our current 'upgrade-all-the-things' strategy when changing major versions. I continue to wonder if that was really the best idea. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > Yeah, default_version was the other one that looked like it might be > possible to include, but folks might decide to try and use 'comment' in > that way too. Basically, there's a chance that they'd want to use any > string in there. Actually, I think that $default_value is the only other serious enough candidate that we should support, and I think we should support it both from the directory and module_pathname parameters. Also, it seems to me that while the $directory macro should still be found only at the beginning of the module_pathname value, the $default_value should be substituted from wherever it is found. Please find attached a v1 version of the patch implementing that. doc/src/sgml/extend.sgml | 18 ++++++++ src/backend/commands/extension.c | 79 +++++++++++++++++++++++++++++--- 2 files changed, 91 insertions(+), 6 deletions(-) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Вложения
On 2/27/14, 6:04 AM, Dimitri Fontaine wrote: > What about allowing a control file like this: > > # hstore extension > comment = 'data type for storing sets of (key, value) pairs' > default_version = '1.3' > directory = 'local/hstore-new' > module_pathname = '$directory/hstore' > relocatable = true I think your previously proposed patch to add extension_control_path plus my suggestion to update existing de facto best practices to not include $libdir into the module path name (thus allowing the use of dynamic_library_path) will address all desired use cases just fine. Moreover, going that way would reuse existing facilities and concepts, remove indirections and reduce overall complexity. This new proposal, on the other hand, would go the other way, introducing new concepts, adding more indirections, and increasing overall complexity, while actually achieving less. I see an analogy here. What we are currently doing is similar to hardcoding absolute rpaths into all libraries. Your proposal is effectively to (1) add the $ORIGIN mechanism and (2) make people use chrpath when they want to install somewhere else. My proposal is to get rid of all rpaths and just set a search path. Yes, on technical level, this is less powerful, but it's simpler and gets the job done and is harder to misuse. A problem with features like these is that they get rarely used but offer infinite flexibility, so they are not used consistently and you can't rely on anything. This is already the case for the module_pathname setting in the control file. It has, AFAICT, no actual use, and because of that no one uses it, and because of that, there is no guarantee that extensions use it sensibly, and because of that no one can ever make sensible use of it in the future, because there is no guarantee that extensions have it set sensibly. In fact, I would propose deprecating module_pathname.
* Peter Eisentraut (peter_e@gmx.net) wrote: > On 2/27/14, 6:04 AM, Dimitri Fontaine wrote: > > What about allowing a control file like this: > > > > # hstore extension > > comment = 'data type for storing sets of (key, value) pairs' > > default_version = '1.3' > > directory = 'local/hstore-new' > > module_pathname = '$directory/hstore' > > relocatable = true > > I think your previously proposed patch to add extension_control_path > plus my suggestion to update existing de facto best practices to not > include $libdir into the module path name (thus allowing the use of > dynamic_library_path) will address all desired use cases just fine. You still haven't addressed how to deal with the case of multiple .so's with the same name ending up in the dynamic_library_path. I don't see that as unlikely to end up happening either. > Moreover, going that way would reuse existing facilities and concepts, > remove indirections and reduce overall complexity. This new proposal, > on the other hand, would go the other way, introducing new concepts, > adding more indirections, and increasing overall complexity, while > actually achieving less. Being able to have a self-contained module which requires a minimum of modification to postgresql.conf is a reduction in complexity, imv. Having to maintain two config options which will end up being overly long and mostly duplicated doesn't make things easier for people. This has made me wonder if we could allow a control file to be explicitly referred to from CREATE EXTENSION itself, dropping the need for any of this postgresql.conf/GUC maintenance. There are downsides to that approach as well, of course, but it's definitely got a certain appeal. > I see an analogy here. What we are currently doing is similar to > hardcoding absolute rpaths into all libraries. Your proposal is > effectively to (1) add the $ORIGIN mechanism and (2) make people use > chrpath when they want to install somewhere else. My proposal is to get > rid of all rpaths and just set a search path. Yes, on technical level, > this is less powerful, but it's simpler and gets the job done and is > harder to misuse. I don't buy off on this analogy. For starters, you can change the control file without needing to rebuild the library, but the main difference is that, in practice, there are no library transistions happening and instead what we're likely to have are independent and *incompatible* libraries living with the same name in our path. > A problem with features like these is that they get rarely used but > offer infinite flexibility, so they are not used consistently and you > can't rely on anything. This is already the case for the > module_pathname setting in the control file. It has, AFAICT, no actual > use, and because of that no one uses it, and because of that, there is > no guarantee that extensions use it sensibly, and because of that no one > can ever make sensible use of it in the future, because there is no > guarantee that extensions have it set sensibly. In fact, I would > propose deprecating module_pathname. This makes sense when you have complete control over where things are installed to and can drop the control file into the "one true directory of control files" and similairly with the .so. Indeed, that works already today for certain platforms, but from what I understand, on the OSX platform you don't really get to just dump files anywhere on the filesystem that you want and instead end up forced into a specific directory tree. Thanks, Stephen
Hi, Peter Eisentraut <peter_e@gmx.net> writes: > On 2/27/14, 6:04 AM, Dimitri Fontaine wrote: >> directory = 'local/hstore-new' >> module_pathname = '$directory/hstore' > > I think your previously proposed patch to add extension_control_path > plus my suggestion to update existing de facto best practices to not > include $libdir into the module path name (thus allowing the use of > dynamic_library_path) will address all desired use cases just fine. My opinion is that we have two choices: refactoring the current API or incrementally improving it. In both cases we should make it possible for the packager to control where any individual module file is loaded from, with maybe options for the sysadmin to override the packager's choice. In your proposal, the control moves away from the developer, and that's a good thing, so you get a +1 from me. Just please make sure that it's still possible to use full absolute path for the module path name so that the packager can have control too. > Moreover, going that way would reuse existing facilities and concepts, > remove indirections and reduce overall complexity. This new proposal, > on the other hand, would go the other way, introducing new concepts, > adding more indirections, and increasing overall complexity, while > actually achieving less. What the $directory proposal achieves is allowing a fully relocatable extension layout, where you just have to drop a directory anywhere in the file system and it just works (*). It just work and allows to easily control which module is loaded and without having to setup either LD_LIBRARY_PATH, ld.so.conf nor our own dynamic_library_path. * providing that said directory is part of extension_control_path, or that you copy or move the .control file to sharedir/extension. That said, I don't intend to be using it myself, so I won't try and save that patch in any ways. My position is that Stephen's concern is real and his idea simple enough while effective, so worth pursuing. > I see an analogy here. What we are currently doing is similar to > hardcoding absolute rpaths into all libraries. Your proposal is > effectively to (1) add the $ORIGIN mechanism and (2) make people use > chrpath when they want to install somewhere else. My proposal is to get > rid of all rpaths and just set a search path. Yes, on technical level, > this is less powerful, but it's simpler and gets the job done and is > harder to misuse. What happens if you have more than one 'prefix.so' file in your path? > A problem with features like these is that they get rarely used but > offer infinite flexibility, so they are not used consistently and you > can't rely on anything. This is already the case for the > module_pathname setting in the control file. It has, AFAICT, no actual > use, and because of that no one uses it, and because of that, there is > no guarantee that extensions use it sensibly, and because of that no one > can ever make sensible use of it in the future, because there is no > guarantee that extensions have it set sensibly. In fact, I would > propose deprecating module_pathname. The module_pathname facility allows the packager to decide where the library module file gets installed and the extension author not to concern himself with that choice. I agree that using $libdir as the extension developer isn't the right thing to do. Having to choose the installation path as a developer, either in the SQL script or in the control file, is not the right thing. Now, the practical answer I have to that point is to have the packager rewrite the control file as part of its build system. My vote goes against deprecating module_pathname, because I didn't see in your proposal any ways to offer the control back to the packager, only to the sysadmin, and I don't want to have the sysadmin involved if we can avoid it (as you say, too much flexibility is a killer). In practical term, though, given the current situation, the build system I'm woking on already has to edit the SQL scripts and control files anyways… Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On 3/7/14, 11:39 AM, Dimitri Fontaine wrote: > Just please make sure that it's still possible to use full absolute path > for the module path name so that the packager can have control too. I can't think of any package system where absolute paths are part of a recommended workflow. There is always a search path, that search path contains a series of some kinds of system, non-system, per-user directories. A packager or installer chooses one of those directories as installation target, according to convention or user choice. OK, you can install a C library in some obscure place and then to #include </some/absolute/path/file.h>, but that's not sane practice. Similarly, you will still be able to hardcode paths into CREATE FUNCTION statements or other places, but why would you want to? > What the $directory proposal achieves is allowing a fully relocatable > extension layout, where you just have to drop a directory anywhere in > the file system and it just works (*). If that's what you want (and it sounds attractive), why couldn't we just locate libraries using extension_control_path as well (which presumably contains the control file we are just processing). No need for another indirection. Libraries and control files being in separate directory hierarchies is a historical artifact, but it's not something that can't be changed if it's not what we want. The problem I have with this $directory proposal, if I understand it correctly, is that it requires editing of the control file at installation time. I don't like this for three reasons: it's not clear how this should actually be done, creating more broken extension build scripts (a big problem already); control files are part of the "code", so to speak, not a configuration file, and so munging it in a site-specific way creates a hybrid type of file that will be difficult to properly manage; it doesn't allow running an extension before installation (unless I overwrite the control file in my own source tree), which is my main use case for this. > What happens if you have more than one 'prefix.so' file in your path? The same thing that happens if you have more than one prefix.control in your path. You take the first one you find. Yes, if those are actually two different path settings, you need to keep those aligned. But that would be a one-time setting by the database administrator, not a per-extension-and-packager setting, so it's manageable. If it still bothers you, put them both in the same path, as I suggested above. > The module_pathname facility allows the packager to decide where the > library module file gets installed and the extension author not to > concern himself with that choice. Again, that would only work if they patch the control file during installation, which is dubious. That's like patching paths in include files or shared libraries. People do that, but it's not a preferred method. And secondly, why would a packager care? Has any packager ever cared to relocate the library file and no other file? Aside from those details, it seems clear that any reasonably complete move-extensions-elsewhere feature will need some kind of build system support. I have various ideas on that and would gladly contribute some of them, but it's not going to happen within two weeks. At this point I suggest that we work toward the minimum viable product: the extension_control_path feature as originally proposed (plus the crash fixes), and let the field work out best practices. As you describe, you can work around all the other issues by patching various text files, but you currently cannot move the extension control file in any way, and that's a real deficiency. (I once experimented with bind mounts to work around that -- a real mess ;-) )
[I answered most of these concerns in more detail in the reply to Dimitri.] On 3/7/14, 9:16 AM, Stephen Frost wrote: > Being able to have a self-contained module which requires a minimum of > modification to postgresql.conf is a reduction in complexity, imv. > Having to maintain two config options which will end up being overly > long and mostly duplicated doesn't make things easier for people. Then we can make it one path. > This > has made me wonder if we could allow a control file to be explicitly > referred to from CREATE EXTENSION itself, dropping the need for any of > this postgresql.conf/GUC maintenance. There are downsides to that > approach as well, of course, but it's definitely got a certain appeal. That might be useful as a separate feature, but it reeks of #include </absolute/path/file.h>, which isn't a sane practice. No programming language other than ancient or poorly designed ones allows that sort of thing. > I don't buy off on this analogy. For starters, you can change the > control file without needing to rebuild the library, (You can also change the rpath without rebuilding the library.) > but the main > difference is that, in practice, there are no library transistions > happening and instead what we're likely to have are independent and > *incompatible* libraries living with the same name in our path. I understand this concern. The question is, how big is it relative to the other ones. As side idea I just had, how about embedding the extension version number into the library somehow? Food for thought. > This makes sense when you have complete control over where things are > installed to and can drop the control file into the "one true directory > of control files" and similairly with the .so. Indeed, that works > already today for certain platforms, but from what I understand, on the > OSX platform you don't really get to just dump files anywhere on the > filesystem that you want and instead end up forced into a specific > directory tree. That is incorrect. If someone has a general use for module_pathname, I'd be interested to hear it, but that's not one of them.
Peter Eisentraut <peter_e@gmx.net> writes: > Aside from those details, it seems clear that any reasonably complete > move-extensions-elsewhere feature will need some kind of build system > support. I have various ideas on that and would gladly contribute some > of them, but it's not going to happen within two weeks. +1 Note that I am currently working on such a build system, so feel free to send me off-list emails about your thoughs, I'm interested and could integrate them into what I'm building. > At this point I suggest that we work toward the minimum viable product: > the extension_control_path feature as originally proposed (plus the > crash fixes), and let the field work out best practices. As you > describe, you can work around all the other issues by patching various > text files, but you currently cannot move the extension control file in > any way, and that's a real deficiency. (I once experimented with bind > mounts to work around that -- a real mess ;-) ) Please find attached the v2 version of the patch, including fixes for the crash and documentation aspects you've listed before. Regards, -- Dimitri Fontaine 06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *************** *** 6008,6013 **** dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' --- 6008,6068 ---- </listitem> </varlistentry> + <varlistentry id="guc-extension-control-path" xreflabel="extension_control_path"> + <term><varname>extension_control_path</varname> (<type>string</type>)</term> + <indexterm> + <primary><varname>extension_control_path</> configuration parameter</primary> + </indexterm> + <indexterm><primary>extension packaging</></> + <listitem> + <para> + The command <command>CREATE EXTENSION</> searches for the extension + control file in order to install it. The value + for <varname>extension_control_path</varname> is used to search for + the <literal>name.control</literal> files. + </para> + + <para> + Note that unless using the <literal>directory</literal> control file + parameter, the extension scripts and auxilliary files are searched in + the <varname>extension_control_path</varname> too. + </para> + + <para> + The value for <varname>extension_control_path</varname> must be a list + of absolute directory paths separated by colons (or semi-colons on + Windows). If a list element starts with the special + string <literal>$extdir</literal>, the + compiled-in <productname>PostgreSQL</productname> package extension + directory is substituted for <literal>$extdir</literal>; this is where + the extensions provided by the standard + <productname>PostgreSQL</productname> distribution are installed. + (Use <literal>pg_config --extdir</literal> to find out the name of + this directory.) For example: + <programlisting> + extension_control_path = '/usr/local/postgresql/extension:/home/my_project:$extdir' + </programlisting> + or, in a Windows environment: + <programlisting> + extension_control_path = 'C:\tools\postgresql;H:\my_project\lib;$extdir' + </programlisting> + </para> + + <para> + The default value for this parameter is <literal>'$extdir'</literal>. + </para> + + <para> + This parameter can be changed at run time by superusers, but a + setting done that way will only persist until the end of the + client connection, so this method should be reserved for + development purposes. The recommended way to set this parameter + is in the <filename>postgresql.conf</filename> configuration + file. + </para> + </listitem> + </varlistentry> + <varlistentry id="guc-gin-fuzzy-search-limit" xreflabel="gin_fuzzy_search_limit"> <term><varname>gin_fuzzy_search_limit</varname> (<type>integer</type>)</term> <indexterm> *** a/src/backend/commands/extension.c --- b/src/backend/commands/extension.c *************** *** 25,30 **** --- 25,31 ---- #include <dirent.h> #include <limits.h> + #include <sys/stat.h> #include <unistd.h> #include "access/htup_details.h" *************** *** 60,71 **** --- 61,76 ---- bool creating_extension = false; Oid CurrentExtensionObject = InvalidOid; + /* GUC extension_control_path */ + char *Extension_control_path; + /* * Internal data structure to hold the results of parsing a control file */ typedef struct ExtensionControlFile { char *name; /* name of the extension */ + char *filename; /* full path of the extension control file */ char *directory; /* directory for script files */ char *default_version; /* default install target version, if any */ char *module_pathname; /* string to substitute for MODULE_PATHNAME */ *************** *** 342,397 **** is_extension_script_filename(const char *filename) return (extension != NULL) && (strcmp(extension, ".sql") == 0); } static char * ! get_extension_control_directory(void) { char sharepath[MAXPGPATH]; ! char *result; get_share_path(my_exec_path, sharepath); ! result = (char *) palloc(MAXPGPATH); ! snprintf(result, MAXPGPATH, "%s/extension", sharepath); ! return result; } static char * ! get_extension_control_filename(const char *extname) { - char sharepath[MAXPGPATH]; char *result; ! get_share_path(my_exec_path, sharepath); result = (char *) palloc(MAXPGPATH); ! snprintf(result, MAXPGPATH, "%s/extension/%s.control", ! sharepath, extname); return result; } static char * get_extension_script_directory(ExtensionControlFile *control) { ! char sharepath[MAXPGPATH]; char *result; /* * The directory parameter can be omitted, absolute, or relative to the ! * installation's share directory. */ if (!control->directory) ! return get_extension_control_directory(); if (is_absolute_path(control->directory)) return pstrdup(control->directory); ! get_share_path(my_exec_path, sharepath); result = (char *) palloc(MAXPGPATH); ! snprintf(result, MAXPGPATH, "%s/%s", sharepath, control->directory); return result; } static char * get_extension_aux_control_filename(ExtensionControlFile *control, const char *version) --- 347,604 ---- return (extension != NULL) && (strcmp(extension, ".sql") == 0); } + /* + * Substitute for any macros appearing in the given string. + * Result is always freshly palloc'd. + */ static char * ! substitute_extension_control_path_macro(const char *name) { char sharepath[MAXPGPATH]; ! const char *sep_ptr; ! ! AssertArg(name != NULL); ! ! /* Currently, we only recognize $extdir at the start of the string */ ! if (name[0] != '$') ! return pstrdup(name); ! ! if ((sep_ptr = first_dir_separator(name)) == NULL) ! sep_ptr = name + strlen(name); ! ! if (strlen("$extdir") != sep_ptr - name || ! strncmp(name, "$extdir", strlen("$extdir")) != 0) ! ereport(ERROR, ! (errcode(ERRCODE_INVALID_NAME), ! errmsg("invalid macro name in extension control path: %s", ! name))); get_share_path(my_exec_path, sharepath); ! return psprintf("%s/extension%s", sharepath, sep_ptr); ! } ! /* ! * XXX: move that function to somewhere both src/backend/utils/fmgr/dfmgr.c ! * and this file can use it. ! */ ! static bool ! file_exists(const char *name) ! { ! struct stat st; ! ! AssertArg(name != NULL); ! ! if (stat(name, &st) == 0) ! return S_ISDIR(st.st_mode) ? false : true; ! else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES)) ! ereport(ERROR, ! (errcode_for_file_access(), ! errmsg("could not access file \"%s\": %m", name))); ! ! return false; } + /* + * Search for a file called 'basename' in the colon-separated search + * path Extension_control_path. If the file is found, the full file name + * is returned in freshly palloc'd memory. If the file is not found, + * return NULL. + */ static char * ! find_in_extension_control_path(const char *basename) ! { ! const char *p; ! size_t baselen; ! ! AssertArg(basename != NULL); ! AssertArg(first_dir_separator(basename) == NULL); ! AssertState(Extension_control_path != NULL); ! ! p = Extension_control_path; ! if (strlen(p) == 0) ! return NULL; ! ! baselen = strlen(basename); ! ! for (;;) ! { ! size_t len; ! char *piece; ! char *mangled; ! char *full; ! ! piece = first_path_var_separator(p); ! if (piece == p) ! ereport(ERROR, ! (errcode(ERRCODE_INVALID_NAME), ! errmsg("zero-length component in parameter \"extension_control_path\""))); ! ! if (piece == NULL) ! len = strlen(p); ! else ! len = piece - p; ! ! piece = palloc(len + 1); ! strlcpy(piece, p, len + 1); ! ! mangled = substitute_extension_control_path_macro(piece); ! pfree(piece); ! ! canonicalize_path(mangled); ! ! /* only absolute paths */ ! if (!is_absolute_path(mangled)) ! ereport(ERROR, ! (errcode(ERRCODE_INVALID_NAME), ! errmsg("component in parameter \"extension_control_path\" is not an absolute path"))); ! ! /* don't forget to count for ".control", 8 chars */ ! full = palloc(strlen(mangled) + 1 + baselen + 8 + 1); ! sprintf(full, "%s/%s.control", mangled, basename); ! pfree(mangled); ! ! elog(DEBUG3, "find_in_extension_control_path: trying \"%s\"", full); ! ! if (file_exists(full)) ! return full; ! ! pfree(full); ! ! if (p[len] == '\0') ! break; ! else ! p += len + 1; ! } ! ! return NULL; ! } ! ! /* ! * Return the current list of extension_control_path directories, with $extdir ! * macro expanded. ! */ ! static List * ! list_extension_control_paths() ! { ! List *paths = NIL; ! const char *p; ! ! AssertState(Extension_control_path != NULL); ! ! p = Extension_control_path; ! if (strlen(p) == 0) ! return NULL; ! ! for (;;) ! { ! size_t len; ! char *piece; ! char *mangled; ! ! piece = first_path_var_separator(p); ! if (piece == p) ! ereport(ERROR, ! (errcode(ERRCODE_INVALID_NAME), ! errmsg("zero-length component in parameter \"extension_control_path\""))); ! ! if (piece == NULL) ! len = strlen(p); ! else ! len = piece - p; ! ! piece = palloc(len + 1); ! strlcpy(piece, p, len + 1); ! ! mangled = substitute_extension_control_path_macro(piece); ! pfree(piece); ! ! canonicalize_path(mangled); ! ! /* only absolute paths */ ! if (!is_absolute_path(mangled)) ! ereport(ERROR, ! (errcode(ERRCODE_INVALID_NAME), ! errmsg("component in parameter \"extension_control_path\" is not an absolute path"))); ! ! paths = lappend(paths, mangled); ! ! if (p[len] == '\0') ! break; ! else ! p += len + 1; ! } ! ! return paths; ! } ! ! /* ! * When this function is called, the control file has already been opened and ! * its true name is registered in control->filename. We don't need to find the ! * control file in extension_control_path anymore. ! */ ! static char * ! get_extension_control_directory(ExtensionControlFile *control) ! { ! char *filename = pstrdup(control->filename); ! ! get_parent_directory(filename); ! ! return filename; ! } ! ! /* ! * We need to either open directory/extname.control or go find the control ! * file for extname in extension_control_path, depending on being given a ! * directory where to look into. ! */ ! static char * ! get_extension_control_filename(const char *extname, const char *directory) { char *result; ! if (directory == NULL) ! return find_in_extension_control_path(extname); ! result = (char *) palloc(MAXPGPATH); ! snprintf(result, MAXPGPATH, "%s/%s.control", directory, extname); return result; } + /* + * When this function is called, the control file has already been opened and + * its true name is registered in control->filename. We don't need to find the + * control file in extension_control_path anymore. + */ static char * get_extension_script_directory(ExtensionControlFile *control) { ! char *directory; char *result; /* * The directory parameter can be omitted, absolute, or relative to the ! * extension's main control file's parent directory. */ if (!control->directory) ! return get_extension_control_directory(control); if (is_absolute_path(control->directory)) return pstrdup(control->directory); ! /* control->directory is relative to control->filename parent's directory */ ! directory = get_extension_control_directory(control); result = (char *) palloc(MAXPGPATH); ! snprintf(result, MAXPGPATH, "%s/%s", directory, control->directory); return result; } + /* + * When this function is called, the control file has already been opened and + * its true name is registered in control->filename. We don't need to find the + * control file in extension_control_path anymore. + */ static char * get_extension_aux_control_filename(ExtensionControlFile *control, const char *version) *************** *** 410,415 **** get_extension_aux_control_filename(ExtensionControlFile *control, --- 617,627 ---- return result; } + /* + * When this function is called, the control file has already been opened and + * its true name is registered in control->filename. We don't need to find the + * control file in extension_control_path anymore. + */ static char * get_extension_script_filename(ExtensionControlFile *control, const char *from_version, const char *version) *************** *** 444,450 **** get_extension_script_filename(ExtensionControlFile *control, */ static void parse_extension_control_file(ExtensionControlFile *control, ! const char *version) { char *filename; FILE *file; --- 656,663 ---- */ static void parse_extension_control_file(ExtensionControlFile *control, ! const char *version, ! const char *directory) { char *filename; FILE *file; *************** *** 458,464 **** parse_extension_control_file(ExtensionControlFile *control, if (version) filename = get_extension_aux_control_filename(control, version); else ! filename = get_extension_control_filename(control->name); if ((file = AllocateFile(filename, "r")) == NULL) { --- 671,684 ---- if (version) filename = get_extension_aux_control_filename(control, version); else ! filename = get_extension_control_filename(control->name, directory); ! ! if (filename == NULL) ! ereport(ERROR, ! (errcode(ERRCODE_INVALID_PARAMETER_VALUE), ! errmsg("extension \"%s\" is not available", control->name), ! errdetail("Couldn't find \"%s.control\" anywhere in extension_control_path", ! control->name))); if ((file = AllocateFile(filename, "r")) == NULL) { *************** *** 482,487 **** parse_extension_control_file(ExtensionControlFile *control, --- 702,709 ---- FreeFile(file); + control->filename = pstrdup(filename); + /* * Convert the ConfigVariable list into ExtensionControlFile entries. */ *************** *** 578,586 **** parse_extension_control_file(ExtensionControlFile *control, /* * Read the primary control file for the specified extension. */ static ExtensionControlFile * ! read_extension_control_file(const char *extname) { ExtensionControlFile *control; --- 800,811 ---- /* * Read the primary control file for the specified extension. + * + * When directory is not NULL, then instead of trying to find the extension + * control file in extension_control_path, we just open the file in directory. */ static ExtensionControlFile * ! read_extension_control_file(const char *extname, const char *directory) { ExtensionControlFile *control; *************** *** 596,602 **** read_extension_control_file(const char *extname) /* * Parse the primary control file. */ ! parse_extension_control_file(control, NULL); return control; } --- 821,864 ---- /* * Parse the primary control file. */ ! parse_extension_control_file(control, NULL, directory); ! ! return control; ! } ! ! /* ! * Find the control file where default_version is the same as given version. ! * To be able to check that we've found the right control file, we need to ! * parse it. ! */ ! static ExtensionControlFile * ! find_extension_control_file_for_version(const char *extname, const char *version) ! { ! ExtensionControlFile *control = NULL; ! List *extension_control_paths = list_extension_control_paths(); ! ListCell *lc; ! ! foreach(lc, extension_control_paths) ! { ! char *location = (char *) lfirst(lc); ! char *path = get_extension_control_filename(extname, location); ! ! if (path && access(path, R_OK) == 0) ! { ! control = read_extension_control_file(extname, location); ! ! if (strcmp(control->default_version, version) == 0) ! break; ! else ! control = NULL; ! } ! } ! if (control == NULL) ! ereport(ERROR, ! (errcode(ERRCODE_INVALID_PARAMETER_VALUE), ! errmsg("invalid extension version: \"%s\"", version), ! errdetail("Extension '%s' has no control file for version '%s'.", ! extname, version))); return control; } *************** *** 622,628 **** read_extension_aux_control_file(const ExtensionControlFile *pcontrol, /* * Parse the auxiliary control file, overwriting struct fields */ ! parse_extension_control_file(acontrol, version); return acontrol; } --- 884,890 ---- /* * Parse the auxiliary control file, overwriting struct fields */ ! parse_extension_control_file(acontrol, version, NULL); return acontrol; } *************** *** 1229,1235 **** CreateExtension(CreateExtensionStmt *stmt) * any non-ASCII data, so there is no need to worry about encoding at this * point. */ ! pcontrol = read_extension_control_file(stmt->extname); /* * Read the statement option list --- 1491,1497 ---- * any non-ASCII data, so there is no need to worry about encoding at this * point. */ ! pcontrol = read_extension_control_file(stmt->extname, NULL); /* * Read the statement option list *************** *** 1283,1288 **** CreateExtension(CreateExtensionStmt *stmt) --- 1545,1558 ---- check_valid_version_name(versionName); /* + * We might need to read another control file given a version number that + * is not the default one. + */ + if (strcmp(versionName, pcontrol->default_version) != 0) + pcontrol = + find_extension_control_file_for_version(stmt->extname, versionName); + + /* * Determine the (unpackaged) version to update from, if any, and then * figure out what sequence of update scripts we need to apply. */ *************** *** 1619,1671 **** RemoveExtensionById(Oid extId) } /* ! * This function lists the available extensions (one row per primary control ! * file in the control directory). We parse each control file and report the ! * interesting fields. ! * ! * The system view pg_available_extensions provides a user interface to this ! * SRF, adding information about whether the extensions are installed in the ! * current DB. */ ! Datum ! pg_available_extensions(PG_FUNCTION_ARGS) { - ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; - TupleDesc tupdesc; - Tuplestorestate *tupstore; - MemoryContext per_query_ctx; - MemoryContext oldcontext; - char *location; DIR *dir; struct dirent *de; - /* check to see if caller supports us returning a tuplestore */ - if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("set-valued function called in context that cannot accept a set"))); - if (!(rsinfo->allowedModes & SFRM_Materialize)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("materialize mode required, but it is not " \ - "allowed in this context"))); - - /* Build a tuple descriptor for our result type */ - if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) - elog(ERROR, "return type must be a row type"); - - /* Build tuplestore to hold the result rows */ - per_query_ctx = rsinfo->econtext->ecxt_per_query_memory; - oldcontext = MemoryContextSwitchTo(per_query_ctx); - - tupstore = tuplestore_begin_heap(true, false, work_mem); - rsinfo->returnMode = SFRM_Materialize; - rsinfo->setResult = tupstore; - rsinfo->setDesc = tupdesc; - - MemoryContextSwitchTo(oldcontext); - - location = get_extension_control_directory(); dir = AllocateDir(location); /* --- 1889,1904 ---- } /* ! * Add available extensions informations (from the control files found in ! * location) to the pg_available_extension tuple store. */ ! static void ! list_available_extensions(TupleDesc tupdesc, Tuplestorestate *tupstore, ! const char *location) { DIR *dir; struct dirent *de; dir = AllocateDir(location); /* *************** *** 1696,1702 **** pg_available_extensions(PG_FUNCTION_ARGS) if (strstr(extname, "--")) continue; ! control = read_extension_control_file(extname); memset(values, 0, sizeof(values)); memset(nulls, 0, sizeof(nulls)); --- 1929,1935 ---- if (strstr(extname, "--")) continue; ! control = read_extension_control_file(extname, location); memset(values, 0, sizeof(values)); memset(nulls, 0, sizeof(nulls)); *************** *** 1717,1725 **** pg_available_extensions(PG_FUNCTION_ARGS) tuplestore_putvalues(tupstore, tupdesc, values, nulls); } - FreeDir(dir); } /* clean up and return the tuplestore */ tuplestore_donestoring(tupstore); --- 1950,2011 ---- tuplestore_putvalues(tupstore, tupdesc, values, nulls); } FreeDir(dir); } + } + + /* + * This function lists the available extensions (one row per primary control + * file in the control directory). We parse each control file and report the + * interesting fields. + * + * The system view pg_available_extensions provides a user interface to this + * SRF, adding information about whether the extensions are installed in the + * current DB. + */ + Datum + pg_available_extensions(PG_FUNCTION_ARGS) + { + ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; + TupleDesc tupdesc; + Tuplestorestate *tupstore; + MemoryContext per_query_ctx; + MemoryContext oldcontext; + List *extension_control_paths = list_extension_control_paths(); + ListCell *lc; + + /* check to see if caller supports us returning a tuplestore */ + if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("set-valued function called in context that cannot accept a set"))); + if (!(rsinfo->allowedModes & SFRM_Materialize)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("materialize mode required, but it is not " \ + "allowed in this context"))); + + /* Build a tuple descriptor for our result type */ + if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) + elog(ERROR, "return type must be a row type"); + + /* Build tuplestore to hold the result rows */ + per_query_ctx = rsinfo->econtext->ecxt_per_query_memory; + oldcontext = MemoryContextSwitchTo(per_query_ctx); + + tupstore = tuplestore_begin_heap(true, false, work_mem); + rsinfo->returnMode = SFRM_Materialize; + rsinfo->setResult = tupstore; + rsinfo->setDesc = tupdesc; + + MemoryContextSwitchTo(oldcontext); + + foreach(lc, extension_control_paths) + { + char *location = (char *) lfirst(lc); + + list_available_extensions(tupdesc, tupstore, location); + } /* clean up and return the tuplestore */ tuplestore_donestoring(tupstore); *************** *** 1744,1750 **** pg_available_extension_versions(PG_FUNCTION_ARGS) Tuplestorestate *tupstore; MemoryContext per_query_ctx; MemoryContext oldcontext; ! char *location; DIR *dir; struct dirent *de; --- 2030,2037 ---- Tuplestorestate *tupstore; MemoryContext per_query_ctx; MemoryContext oldcontext; ! List *extension_control_paths = list_extension_control_paths(); ! ListCell *lc; DIR *dir; struct dirent *de; *************** *** 1774,1816 **** pg_available_extension_versions(PG_FUNCTION_ARGS) MemoryContextSwitchTo(oldcontext); ! location = get_extension_control_directory(); ! dir = AllocateDir(location); ! ! /* ! * If the control directory doesn't exist, we want to silently return an ! * empty set. Any other error will be reported by ReadDir. ! */ ! if (dir == NULL && errno == ENOENT) ! { ! /* do nothing */ ! } ! else { ! while ((de = ReadDir(dir, location)) != NULL) { ! ExtensionControlFile *control; ! char *extname; ! if (!is_extension_control_filename(de->d_name)) ! continue; ! /* extract extension name from 'name.control' filename */ ! extname = pstrdup(de->d_name); ! *strrchr(extname, '.') = '\0'; ! /* ignore it if it's an auxiliary control file */ ! if (strstr(extname, "--")) ! continue; ! /* read the control file */ ! control = read_extension_control_file(extname); ! /* scan extension's script directory for install scripts */ ! get_available_versions_for_extension(control, tupstore, tupdesc); ! } ! FreeDir(dir); } /* clean up and return the tuplestore */ --- 2061,2107 ---- MemoryContextSwitchTo(oldcontext); ! foreach(lc, extension_control_paths) { ! char *location = (char *) lfirst(lc); ! ! dir = AllocateDir(location); ! ! /* ! * If the control directory doesn't exist, we want to silently return an ! * empty set. Any other error will be reported by ReadDir. ! */ ! if (dir == NULL && errno == ENOENT) { ! /* do nothing */ ! } ! else ! { ! while ((de = ReadDir(dir, location)) != NULL) ! { ! ExtensionControlFile *control; ! char *extname; ! if (!is_extension_control_filename(de->d_name)) ! continue; ! /* extract extension name from 'name.control' filename */ ! extname = pstrdup(de->d_name); ! *strrchr(extname, '.') = '\0'; ! /* ignore it if it's an auxiliary control file */ ! if (strstr(extname, "--")) ! continue; ! /* read the control file */ ! control = read_extension_control_file(extname, location); ! /* scan extension's script directory for install scripts */ ! get_available_versions_for_extension(control, tupstore, tupdesc); ! } ! FreeDir(dir); ! } } /* clean up and return the tuplestore */ *************** *** 1968,1974 **** pg_extension_update_paths(PG_FUNCTION_ARGS) MemoryContextSwitchTo(oldcontext); /* Read the extension's control file */ ! control = read_extension_control_file(NameStr(*extname)); /* Extract the version update graph from the script directory */ evi_list = get_ext_ver_list(control); --- 2259,2265 ---- MemoryContextSwitchTo(oldcontext); /* Read the extension's control file */ ! control = read_extension_control_file(NameStr(*extname), NULL); /* Extract the version update graph from the script directory */ evi_list = get_ext_ver_list(control); *************** *** 2653,2659 **** ExecAlterExtensionStmt(AlterExtensionStmt *stmt) * any non-ASCII data, so there is no need to worry about encoding at this * point. */ ! control = read_extension_control_file(stmt->extname); /* * Read the statement option list --- 2944,2951 ---- * any non-ASCII data, so there is no need to worry about encoding at this * point. */ ! control = find_extension_control_file_for_version(stmt->extname, ! oldVersionName); /* * Read the statement option list *** a/src/backend/utils/misc/guc.c --- b/src/backend/utils/misc/guc.c *************** *** 32,37 **** --- 32,38 ---- #include "access/xact.h" #include "catalog/namespace.h" #include "commands/async.h" + #include "commands/extension.h" #include "commands/prepare.h" #include "commands/vacuum.h" #include "commands/variable.h" *************** *** 2792,2797 **** static struct config_string ConfigureNamesString[] = --- 2793,2811 ---- }, { + {"extension_control_path", PGC_SUSET, CLIENT_CONN_OTHER, + gettext_noop("Sets the path for extension control files."), + gettext_noop("If an extension control file needs to be opened " + "the system will search this path for " + "the specified file."), + GUC_SUPERUSER_ONLY + }, + &Extension_control_path, + "$extdir", + NULL, NULL, NULL + }, + + { {"krb_server_keyfile", PGC_SIGHUP, CONN_AUTH_SECURITY, gettext_noop("Sets the location of the Kerberos server key file."), NULL, *** a/src/backend/utils/misc/postgresql.conf.sample --- b/src/backend/utils/misc/postgresql.conf.sample *************** *** 547,552 **** --- 547,553 ---- # - Other Defaults - #dynamic_library_path = '$libdir' + #extension_control_path = '$extdir' #local_preload_libraries = '' #session_preload_libraries = '' *** a/src/bin/pg_config/pg_config.c --- b/src/bin/pg_config/pg_config.c *************** *** 146,151 **** show_includedir_server(bool all) --- 146,163 ---- } static void + show_extdir(bool all) + { + char path[MAXPGPATH]; + + if (all) + printf("EXTDIR = "); + get_share_path(mypath, path); + cleanup_path(path); + printf("%s/extension\n", path); + } + + static void show_libdir(bool all) { char path[MAXPGPATH]; *************** *** 402,407 **** static const InfoItem info_items[] = { --- 414,420 ---- {"--pkgincludedir", show_pkgincludedir}, {"--includedir-server", show_includedir_server}, {"--libdir", show_libdir}, + {"--extdir", show_extdir}, {"--pkglibdir", show_pkglibdir}, {"--localedir", show_localedir}, {"--mandir", show_mandir}, *************** *** 436,441 **** help(void) --- 449,455 ---- " interfaces\n")); printf(_(" --pkgincludedir show location of other C header files\n")); printf(_(" --includedir-server show location of C header files for the server\n")); + printf(_(" --extdir show location of extension control files\n")); printf(_(" --libdir show location of object code libraries\n")); printf(_(" --pkglibdir show location of dynamically loadable modules\n")); printf(_(" --localedir show location of locale support files\n")); *** a/src/include/commands/extension.h --- b/src/include/commands/extension.h *************** *** 26,31 **** --- 26,33 ---- extern bool creating_extension; extern Oid CurrentExtensionObject; + /* GUC extension_control_path */ + extern char *Extension_control_path; extern Oid CreateExtension(CreateExtensionStmt *stmt);
On Fri, Mar 7, 2014 at 10:19 PM, Peter Eisentraut <peter_e@gmx.net> wrote: >> What the $directory proposal achieves is allowing a fully relocatable >> extension layout, where you just have to drop a directory anywhere in >> the file system and it just works (*). > > If that's what you want (and it sounds attractive), why couldn't we just > locate libraries using extension_control_path as well (which presumably > contains the control file we are just processing). No need for another > indirection. Libraries and control files being in separate directory > hierarchies is a historical artifact, but it's not something that can't > be changed if it's not what we want. > > The problem I have with this $directory proposal, if I understand it > correctly, is that it requires editing of the control file at > installation time. I don't like this for three reasons: it's not clear > how this should actually be done, creating more broken extension build > scripts (a big problem already); control files are part of the "code", > so to speak, not a configuration file, and so munging it in a > site-specific way creates a hybrid type of file that will be difficult > to properly manage; it doesn't allow running an extension before > installation (unless I overwrite the control file in my own source > tree), which is my main use case for this. +1. >> What happens if you have more than one 'prefix.so' file in your path? > > The same thing that happens if you have more than one prefix.control in > your path. You take the first one you find. > > Yes, if those are actually two different path settings, you need to keep > those aligned. But that would be a one-time setting by the database > administrator, not a per-extension-and-packager setting, so it's > manageable. If it still bothers you, put them both in the same path, as > I suggested above. It's tempting to think that when you install an extension, we should search the directory where the control file was found for the .so first - and if so, use THAT library every time, not any other one. Of course the problem with that is that we'd then need to remember that in the system catalogs, which might pose a problem in terms of letting people reorganize the filesystem hierarchy without getting too bothered about what PostgreSQL is remembering internally. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
* Robert Haas (robertmhaas@gmail.com) wrote: > On Fri, Mar 7, 2014 at 10:19 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > >> What the $directory proposal achieves is allowing a fully relocatable > >> extension layout, where you just have to drop a directory anywhere in > >> the file system and it just works (*). > > > > If that's what you want (and it sounds attractive), why couldn't we just > > locate libraries using extension_control_path as well (which presumably > > contains the control file we are just processing). No need for another > > indirection. Libraries and control files being in separate directory > > hierarchies is a historical artifact, but it's not something that can't > > be changed if it's not what we want. > > > > The problem I have with this $directory proposal, if I understand it > > correctly, is that it requires editing of the control file at > > installation time. I don't like this for three reasons: it's not clear > > how this should actually be done, creating more broken extension build > > scripts (a big problem already); control files are part of the "code", > > so to speak, not a configuration file, and so munging it in a > > site-specific way creates a hybrid type of file that will be difficult > > to properly manage; it doesn't allow running an extension before > > installation (unless I overwrite the control file in my own source > > tree), which is my main use case for this. > > +1. I agree with this- it wasn't my intent to require any hacking of the control file on install. At least my recollection (and it might be wrong- I'm feeling a bit under-the-weather at the moment..) was that I was looking for a way to explicitly say "look for the .so in the same directory as the control file" and then had another thought of "allow a fully-qualified path to be used for the control file @ CREATE EXTENSION time". > >> What happens if you have more than one 'prefix.so' file in your path? > > > > The same thing that happens if you have more than one prefix.control in > > your path. You take the first one you find. > > > > Yes, if those are actually two different path settings, you need to keep > > those aligned. But that would be a one-time setting by the database > > administrator, not a per-extension-and-packager setting, so it's > > manageable. If it still bothers you, put them both in the same path, as > > I suggested above. > > It's tempting to think that when you install an extension, we should > search the directory where the control file was found for the .so > first - and if so, use THAT library every time, not any other one. Of > course the problem with that is that we'd then need to remember that > in the system catalogs, which might pose a problem in terms of letting > people reorganize the filesystem hierarchy without getting too > bothered about what PostgreSQL is remembering internally. I'd like to be able to specify "same directory as the control file" somehow since that's what I expect non-packaged extensions to mostly want. I also don't like having to hack the control file. Nor do I particularly like having to hack the postgresql.conf every time you add an extension (made doubly worse if you have to hand-edit two paths for every additional extension). I agree that it also presents challenges for how we store that information internally. Thanks, Stephen
On Mon, 2014-03-10 at 09:36 +0100, Dimitri Fontaine wrote: > Please find attached the v2 version of the patch, including fixes for > the crash and documentation aspects you've listed before. Do we want to get this version committed (will need some small tweaks), or do we want to wait for the next release for a more comprehensive solution?
* Peter Eisentraut (peter_e@gmx.net) wrote: > On Mon, 2014-03-10 at 09:36 +0100, Dimitri Fontaine wrote: > > Please find attached the v2 version of the patch, including fixes for > > the crash and documentation aspects you've listed before. > > Do we want to get this version committed (will need some small tweaks), > or do we want to wait for the next release for a more comprehensive > solution? I'm alright with this and it's in line with the other parameters that we support. I had hoped for a different approach, but I seem to be in the minority camp and this is more in line with what we've got already anyway. I do think it's better than what we've got today, and the ability to set it runtime makes it useable for a lot of things. Perhaps we could make that not require superuser at some point, provided the extension which gets installed from the path set by the user doesn't have anything in it which requires superuser. Thanks, Stephen
Peter Eisentraut <peter_e@gmx.net> writes: > On Mon, 2014-03-10 at 09:36 +0100, Dimitri Fontaine wrote: >> Please find attached the v2 version of the patch, including fixes for >> the crash and documentation aspects you've listed before. > Do we want to get this version committed (will need some small tweaks), > or do we want to wait for the next release for a more comprehensive > solution? I think it's missed the window for 9.4, especially since the change was still controversial IIRC. regards, tom lane