Thread: Extensions support for pg_dump, patch v27
Hi, Following recent commit of the hstore Makefile cleaning, that I included into my extension patch too, I have a nice occasion to push version 27 of the patch. Please find it enclosed. This time I'm not including the contrib-only and doc-src split versions, just ask for them if that's what you need. Of course you can also use my git repository: http://git.postgresql.org/gitweb?p=postgresql-extension.git;a=shortlog;h=refs/heads/extension Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachment
On Mon, Jan 24, 2011 at 18:22, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > Following recent commit of the hstore Makefile cleaning, that I included > into my extension patch too, I have a nice occasion to push version 27 > of the patch. Please find it enclosed. Hi, I read the patch and test it in some degree. It works as expected generally, but still needs a bit more development in edge case. * commands/extension.h needs cleanup. - Missing "extern" declarations for create_extension and create_extension_with_user_data variables. - ExtensionControlFile and ExtensionList can be hidden from the header. - extension_objects_fctx is not used at all. * There are many recordDependencyOn(&myself, &CreateExtensionAddress, ...) in some places, but I'm not sure we forget something -- TRIGGERs? * Will we still need uninstaller scripts? DROP EXTENSION depends on object dependency to drop objects, but we have a few configurations that cannot be described in the dependency. For example, rows inserted into pg_am for user AMs or reverting default settings modified by the extension. * Extensions installed in pg_catalog: If we install an extension into pg_catalog, CREATE EXTENSION xxx WITH SCHEMA pg_catalog pg_dump dumps nothing about the extension. We might need special care for modules that install functions only in pg_catalog. * I can install all extensions successfully, but there is a warning in hstore. The warning was not reported to the client, but the whole contents of hstore.sql.in was recorded in the server log. WARNING: => is deprecated as an operator name We sets client_min_messages for the issue in hstore.sql.in, but I think we also need an additional "SET log_min_messages TO ERROR" around it. * Is it support nested CREATE EXTENSION calls? It's rare case, but we'd have some error checks for it. * It passes all regression test for both backend and contrib modules, but there are a couple of warnings during build and installation. Three compiler warnings found. Please check. pg_proc.c: In function 'ProcedureCreate': pg_proc.c:110:8: warning: 'extensionOid' may be used uninitialized in this function pg_shdepend.c: In function 'shdepReassignOwned': pg_shdepend.c:1335:6: warning: implicit declaration of function 'AlterOperatorOwner_oid' operatorcmds.c:369:1: warning: no previous prototype for 'AlterOperatorOwner_oid' * Five warnings also found during make install in contrib directory. ../../config/install-sh: ./uninstall_btree_gist.sql does not exist. ../../config/install-sh: ./uninstall_pageinspect.sql does not exist. ../../config/install-sh: ./uninstall_pgcrypto.sql does not exist. ../../config/install-sh: ./uninstall_pgrowlocks.sql does not exist. ../../config/install-sh: ./uninstall_pgstattuple.sql does not exist. * You use "const = expr" in some places, ex. if( InvalidOid != ...), but we seem to prefer "expr = const". * [off topic] I found some installer scripts are inconsistent. They uses CREATE FUNCTION for some of functions, but CREATE OR REPLACE FUNCTION for others. We'd better write documentation about how to write installer scripts because CREATE EXTENSION has some implicit assumptions in them. For example, "Don't use transaction", etc. -- Itagaki Takahiro
Hi, Itagaki Takahiro <itagaki.takahiro@gmail.com> writes: > Hi, I read the patch and test it in some degree. It works as expected > generally, but still needs a bit more development in edge case. Thanks for the review! > * commands/extension.h needs cleanup. > - Missing "extern" declarations for create_extension and > create_extension_with_user_data variables. > - ExtensionControlFile and ExtensionList can be hidden from the header. > - extension_objects_fctx is not used at all. Fixed in git. I'm not yet used to the project style where we declare some structures into the c code rather than the headers… and then it's cleanup. > * There are many recordDependencyOn(&myself, &CreateExtensionAddress, ...) > in some places, but I'm not sure we forget something -- TRIGGERs? I think we're good here. The extensions I know that use triggers are installing the functions, it's still the user who CREATE TRIGGER and uses the function. And even if we have an extension that contains some CREATE TRIGGER commands in its script, the trigger will depend on a function and the function on the extension. If there's a use case for CREATE TRIGGER in an extension's script and having the trigger not depend on another extension's object, like a function, then I didn't see it. I'm ok to add the triggers as first class dependency objects in the extension patch if there's a need… > * Will we still need uninstaller scripts? > DROP EXTENSION depends on object dependency to drop objects, > but we have a few configurations that cannot be described in the > dependency. For example, rows inserted into pg_am for user AMs > or reverting default settings modified by the extension. Well I'm unconvinced by index AM extensions. Unfortunately, if you want a crash safe AM, you need to patch core code, it's not an extension any more. About reverting default settings, I'm not following. What I saw is existing extensions that didn't need uninstall script once you can DROP EXTENSION foo; but of course it would be quite easy to add a parameter for that in the control file. Do we need it, though? > * Extensions installed in pg_catalog: > If we install an extension into pg_catalog, > CREATE EXTENSION xxx WITH SCHEMA pg_catalog > pg_dump dumps nothing about the extension. We might need special care > for modules that install functions only in pg_catalog. I didn't spot that, I just didn't think about that use case. On a quick test here it seems like the dependency is not recorded in this case. Will fix. > * I can install all extensions successfully, but there is a warning > in hstore. The warning was not reported to the client, but the whole > contents of hstore.sql.in was recorded in the server log. > > WARNING: => is deprecated as an operator name > > We sets client_min_messages for the issue in hstore.sql.in, but I think > we also need an additional "SET log_min_messages TO ERROR" around it. We can do that, but what's happening now is my understanding of the consensus we reached on the list. > * Is it support nested CREATE EXTENSION calls? > It's rare case, but we'd have some error checks for it. In fact earthdistance could CREATE EXTENSION cube; itself in its install script. As you say, though, it's a rare case and it was agreed that this feature could wait until a later version, so I didn't spend time on it. > * It passes all regression test for both backend and contrib modules, > but there are a couple of warnings during build and installation. > > Three compiler warnings found. Please check. > pg_proc.c: In function 'ProcedureCreate': > pg_proc.c:110:8: warning: 'extensionOid' may be used uninitialized in > this function > pg_shdepend.c: In function 'shdepReassignOwned': > pg_shdepend.c:1335:6: warning: implicit declaration of function > 'AlterOperatorOwner_oid' > operatorcmds.c:369:1: warning: no previous prototype for > 'AlterOperatorOwner_oid' Oops sorry about that, I miss them too easily. What's the trick to turn warnings into errors already please? Fixed in the git repository. > * Five warnings also found during make install in contrib directory. > ../../config/install-sh: ./uninstall_btree_gist.sql does not exist. > ../../config/install-sh: ./uninstall_pageinspect.sql does not exist. > ../../config/install-sh: ./uninstall_pgcrypto.sql does not exist. > ../../config/install-sh: ./uninstall_pgrowlocks.sql does not exist. > ../../config/install-sh: ./uninstall_pgstattuple.sql does not exist. That's the DATA = uninstall_ lines in the Makefile. Removed in the git repository. Cleared. > * You use "const = expr" in some places, ex. if( InvalidOid != ...), > but we seem to prefer "expr = const". It allows to get a compiler error when you mistakenly use = rather than == because the Left Hand Side is a constant, so I got used to writing things this way. Should I review my patch and adapt? Will do after some votes :) > * [off topic] I found some installer scripts are inconsistent. > They uses CREATE FUNCTION for some of functions, but CREATE OR REPLACE > FUNCTION for others. We'd better write documentation about how to write > installer scripts because CREATE EXTENSION has some implicit assumptions > in them. For example, "Don't use transaction", etc. Will add some more documentation, ok. As far as contrib goes, I didn't rework the install scripts. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Itagaki Takahiro <itagaki.takahiro@gmail.com> writes: > * Extensions installed in pg_catalog: > If we install an extension into pg_catalog, > CREATE EXTENSION xxx WITH SCHEMA pg_catalog > pg_dump dumps nothing about the extension. We might need special care > for modules that install functions only in pg_catalog. In src/backend/catalog/pg_depend.c we find the code for recordDependencyOn() which is in fact in recordMultipleDependencies(), and sayth so: /* * If the referenced object is pinned by the system, there's no real * need to record dependencies on it. This saves lots of space in * pg_depend, so it's worth the time taken to check. */ So I'm not sure about what we can do here other than error on using pg_catalog in CREATE EXTENSION? How do we want to manage adminpack? Other than adminpack, I think it makes sense to say that extensions are not going into pg_catalog… Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Tue, Jan 25, 2011 at 04:23:41PM +0100, Dimitri Fontaine wrote: > Itagaki Takahiro <itagaki.takahiro@gmail.com> writes: > > * Extensions installed in pg_catalog: > > If we install an extension into pg_catalog, > > CREATE EXTENSION xxx WITH SCHEMA pg_catalog > > pg_dump dumps nothing about the extension. We might need special care > > for modules that install functions only in pg_catalog. > > In src/backend/catalog/pg_depend.c we find the code for > recordDependencyOn() which is in fact in recordMultipleDependencies(), > and sayth so: > > /* > * If the referenced object is pinned by the system, there's no real > * need to record dependencies on it. This saves lots of space in > * pg_depend, so it's worth the time taken to check. > */ > > So I'm not sure about what we can do here other than error on using > pg_catalog in CREATE EXTENSION? How do we want to manage adminpack? > > Other than adminpack, I think it makes sense to say that extensions > are not going into pg_catalog… Given this, we should maybe see about either making adminpack part of PostgreSQL's core distribution (probably a good idea) or moving it out of pg_catalog so we don't have an exception to the rule. Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On Jan 25, 2011, at 7:27 AM, David Fetter wrote: >> Other than adminpack, I think it makes sense to say that extensions >> are not going into pg_catalog… > > Given this, we should maybe see about either making adminpack part of > PostgreSQL's core distribution (probably a good idea) or moving it out > of pg_catalog so we don't have an exception to the rule. +1 David
On Wed, Jan 26, 2011 at 02:57, David E. Wheeler <david@kineticode.com> wrote: >>> Other than adminpack, I think it makes sense to say that extensions >>> are not going into pg_catalog… >> >> Given this, we should maybe see about either making adminpack part of >> PostgreSQL's core distribution (probably a good idea) or moving it out >> of pg_catalog so we don't have an exception to the rule. > > +1 I doubt it can solve the real problem. It is my understanding that we install them in pg_catalog because they are designed to be installed in template1 for pgAdmin, right? We have a problem in pg_dump when we install extension modules in template1. If we create a database, installed functions are copied from template1. However, they are also dumped with pg_dump unless they are in pg_catalog. So, we encounter "function already exists" errors when pg_restore. Since pg_dump won't dump user objects in pg_catalog, adminpack can avoid the above errors by installing functions in pg_catalog. CREATE EXTENSION might have the same issue -- Can EXTENSION work without errors when we install extensions in template databases? To avoid errors, pg_dump might need to dump extensions as "CREATE OR REPLACE EXTENSION" or "CREATE EXTENSION IF NOT EXISTS" rather than "CREATE EXTENSION". -- Itagaki Takahiro
Itagaki Takahiro <itagaki.takahiro@gmail.com> writes: > Since pg_dump won't dump user objects in pg_catalog, adminpack can > avoid the above errors by installing functions in pg_catalog. > CREATE EXTENSION might have the same issue -- Can EXTENSION work > without errors when we install extensions in template databases? Well in fact the reason why pg_dump will not dump the extension when it's installed in pg_catalog is that the pg_depend entry is not recorded, and the SQL query pg_dump uses gets its data from pg_extension JOIN pg_depend JOIN pg_namespace. The missing entry in pg_depend is the reason why the extension is not part of the dump. We could fix that using a LEFT JOIN here and COALESCE to force the namespace as pg_catalog. Is that not a kludge? If acceptable, I will do that next. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > The missing entry in pg_depend is the reason why the extension is not > part of the dump. We could fix that using a LEFT JOIN here and COALESCE > to force the namespace as pg_catalog. Is that not a kludge? Yes, it is. Why is the pg_depend entry missing? regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: >> The missing entry in pg_depend is the reason why the extension is not >> part of the dump. We could fix that using a LEFT JOIN here and COALESCE >> to force the namespace as pg_catalog. Is that not a kludge? > > Yes, it is. Why is the pg_depend entry missing? See src/backend/catalog/pg_depend.c /* * If the referenced object is pinned by the system, there's no real * need to record dependencies on it. This saves lots of space in * pg_depend, so it's worth the time taken to check. */ Certainly, pg_catalog is pinned. select * from pg_dependwhere refobjid = (select oid from pg_namespace where nspname ='pg_catalog') and refclassid = 'pg_namespace'::regclass; classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype ---------+-------+----------+------------+----------+-------------+--------- 0 | 0 | 0 | 2615 | 11 | 0 | p (1 row) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > Tom Lane <tgl@sss.pgh.pa.us> writes: >> Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: >>> The missing entry in pg_depend is the reason why the extension is not >>> part of the dump. We could fix that using a LEFT JOIN here and COALESCE >>> to force the namespace as pg_catalog. Is that not a kludge? >> Yes, it is. Why is the pg_depend entry missing? > See src/backend/catalog/pg_depend.c > Certainly, pg_catalog is pinned. OK, so I guess I'm missing why the extension code is looking for stuff dependent on the pg_catalog schema. That schema certainly doesn't belong to any extension. In any case, your proposed hack above is effectively assuming that there's only one pinned schema, which is untrue now and is likely to become even less true in the future. So I don't think we can go that way. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > OK, so I guess I'm missing why the extension code is looking for stuff > dependent on the pg_catalog schema. That schema certainly doesn't > belong to any extension. Exactly. We're on the same page here, full agreement. So the extension patch is not considering pg_catalog in any special way here, and the problem comes from contrib/adminpack which installs its functions into pg_catalog, yet is not part of core. So in his tests, Itagaki was tempted to issue the following statement: CREATE EXTENSION adminpack WITH SCHEMA pg_catalog; That's supposed to register a dependency from the extension to its declared hosting schema (adminpack is not relocatable so that entirely depends on its script — which forces pg_catalog). Then you get the same problems as with any other object that lives into this schema, pg_dump will avoid dumping its definition… > In any case, your proposed hack above is effectively assuming that > there's only one pinned schema, which is untrue now and is likely to > become even less true in the future. So I don't think we can go that way. Well I proposed is nothing more than a crock. What I'd like us to do instead is ERRORing out when you want to use pg_catalog for an extension. We could use get_extension_namespace() just after recoding the dependency and error out if we don't find the arguments we gave to recordDependencyOn() so that we're not duplicating code. That will cover any pinned schema. I'm preparing a patch to do that. What do we want to do with adminpack? Including its functions into core, or have it use another schema? I don't think an extension installing its objects into pg_catalog is that good an idea… Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > So in his tests, Itagaki was tempted to issue the following statement: > CREATE EXTENSION adminpack WITH SCHEMA pg_catalog; > That's supposed to register a dependency from the extension to its > declared hosting schema (adminpack is not relocatable so that entirely > depends on its script - which forces pg_catalog). > Then you get the same problems as with any other object that lives into > this schema, pg_dump will avoid dumping its definition ... Mph. Although such an extension should certainly carry a dependency on the schema it's using, I'm not sure that it makes sense to consider that the extension (as opposed to its contained objects) belongs to the schema. If we think that extensions live inside schemas then it's logically difficult for an extension to own objects scattered across multiple schemas, which is surely a restriction we do not want. So I'd have expected that extensions use unqualified names, like languages or tablespaces. That being the case, there is no reason why pg_dump ought to be making any such test. There are places where pg_dump refuses to dump objects contained in pg_catalog on the grounds that they're system objects, but that heuristic ought not apply to extensions IMO, even if you don't agree with the conclusion of the preceding paragraph. Any extension is, by definition, not built-in. > Well I proposed is nothing more than a crock. What I'd like us to do > instead is ERRORing out when you want to use pg_catalog for an > extension. Right offhand I'm not seeing the need for such a restriction, though certainly I'd not cry a lot if we had to impose it. ISTM what we've got here is some bogus what-to-dump rules in pg_dump. Extensions should always be dumped. > What do we want to do with adminpack? Including its functions into > core, or have it use another schema? I don't think an extension > installing its objects into pg_catalog is that good an idea I'm trying to avoid having an opinion on that. The reasons for it might or might not be very good, but I'd rather that the extension mechanism didn't break the ability for people to make such decisions. regards, tom lane
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > We could use get_extension_namespace() just after recoding the > dependency and error out if we don't find the arguments we gave to > recordDependencyOn() so that we're not duplicating code. That will > cover any pinned schema. I'm preparing a patch to do that. Kids are falling asleep and the patch there: http://git.postgresql.org/gitweb?p=postgresql-extension.git;a=commitdiff;h=5d0834a8de54a52c601e4fd04aee2d19d1eef4c6 > What do we want to do with adminpack? Including its functions into > core, or have it use another schema? I don't think an extension > installing its objects into pg_catalog is that good an idea… As we're still waiting on some decision here, and some others in previous mails of this same thread, I'm waiting some more before to produce the next patch in the series. See http://archives.postgresql.org/pgsql-hackers/2011-01/msg02385.php http://archives.postgresql.org/pgsql-hackers/2011-01/msg02392.php Of course the git repository is uptodate should you want to try the newest code without waiting on me for the next patch release. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Tom Lane <tgl@sss.pgh.pa.us> writes: > Mph. Although such an extension should certainly carry a dependency on > the schema it's using, I'm not sure that it makes sense to consider that > the extension (as opposed to its contained objects) belongs to the > schema. If we think that extensions live inside schemas then it's > logically difficult for an extension to own objects scattered across > multiple schemas, which is surely a restriction we do not want. So I'd > have expected that extensions use unqualified names, like languages or > tablespaces. That being the case, there is no reason why pg_dump ought > to be making any such test. Well yes, extension are not living in a schema, we just offer users a way to influence the script as far as where the extension's objects are to be found and register a dependency so that it's easy to remember what the users asked. So that for example we are able to act the same way on pg_restore. Now, pg_dump makes no such test already, but a query is using a JOIN to list extensions and their target schema, where it's possible that the target has not been recorded by recordDependencyOn(): in this case the whole extension's is filtered out of the resultset. Quick and dirty fix, I proposed to LEFT JOIN in the pg_dump query… > There are places where pg_dump refuses to dump objects contained in > pg_catalog on the grounds that they're system objects, but that > heuristic ought not apply to extensions IMO, even if you don't agree > with the conclusion of the preceding paragraph. Any extension is, by > definition, not built-in. Agreed. The problem we're having here is how to implement all that yet fully support adminpack. The design we're talking about is the same. >> Well I proposed is nothing more than a crock. What I'd like us to do >> instead is ERRORing out when you want to use pg_catalog for an >> extension. > > Right offhand I'm not seeing the need for such a restriction, though > certainly I'd not cry a lot if we had to impose it. ISTM what we've got > here is some bogus what-to-dump rules in pg_dump. Extensions should > always be dumped. Agreed. Trying to solve an implementation detail, and that's the easier way to prevent users from shooting themselves in the foot here. >> What do we want to do with adminpack? Including its functions into >> core, or have it use another schema? I don't think an extension >> installing its objects into pg_catalog is that good an idea > > I'm trying to avoid having an opinion on that. The reasons for it might > or might not be very good, but I'd rather that the extension mechanism > didn't break the ability for people to make such decisions. You still can do one of the following commands, where you're not having a say on the real schema that adminpack will use because it's not relocatable and not paying attention to @extschema@, but apart from this lie everything will just work. I'd be happy to ship with such a lie, but I can see why people want something different. CREATE EXTENSION adminpack; CREATE EXTENSION adminpack WITH SCHEMA utils; Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > Tom Lane <tgl@sss.pgh.pa.us> writes: >> Mph. Although such an extension should certainly carry a dependency on >> the schema it's using, I'm not sure that it makes sense to consider that >> the extension (as opposed to its contained objects) belongs to the >> schema. > Well yes, extension are not living in a schema, we just offer users a > way to influence the script as far as where the extension's objects are > to be found and register a dependency so that it's easy to remember what > the users asked. So that for example we are able to act the same way on > pg_restore. Oh: then you're doing it wrong. If you want to remember that WITH SCHEMA was specified, you need to explicitly store that as another column in pg_extension. You should not be depending on the dependency mechanism to remember that for you, any more than we'd use pg_depend to remember a table's relnamespace. The dependency mechanism is there to figure out the consequences of a DROP command, it's not there to remember arbitrary facts. (And yes, I know that we've cheated on that principle a few times before; but you can't do it here.) regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Oh: then you're doing it wrong. If you want to remember that WITH > SCHEMA was specified, you need to explicitly store that as another > column in pg_extension. You should not be depending on the dependency > mechanism to remember that for you, any more than we'd use pg_depend to > remember a table's relnamespace. The dependency mechanism is there > to figure out the consequences of a DROP command, it's not there to > remember arbitrary facts. (And yes, I know that we've cheated on that > principle a few times before; but you can't do it here.) The thinking is that we need to have the dependency registered too, so that DROP SCHEMA will cascade to the extension. So while at it, I also used the dependency for tracking the schema. Even if I get to use a column to track the schema, I will have to maintain registering the dependency. Should I do that? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Tom Lane <tgl@sss.pgh.pa.us> writes: > Oh: then you're doing it wrong. If you want to remember that WITH > SCHEMA was specified, you need to explicitly store that as another > column in pg_extension. Ok, done. Of course, it solves the whole problem Itagaki had with adminpack because we stop relying on dependencies to get it right now. I've also added another parameter in the control file, named "schema". It's only valid to use that when relocatable is false, and it allows to force the schema where to install the extension. When this schema does not already exists, it will be created for the user. Of course the adminpack extension's control file now has relocatable = false and schema = 'pg_catalog'. ~=# create extension lo; CREATE EXTENSION ~=# create extension adminpack; CREATE EXTENSION ~=# \dx List of extensions Schema | Name | Version | Description ------------+-----------+----------+----------------------------------------- pg_catalog | adminpack | 9.1devel | Administrative functions for PostgreSQL utils | lo | 9.1devel | managing Large Objects (2 rows) ~=# drop extension adminpack; DROP EXTENSION ~=# create extension adminpack with schema utils; ERROR: this extension has to be installed in schema "pg_catalog" ~=# create extension adminpack with schema pg_catalog; CREATE EXTENSION ~=# alter extension adminpack set schema utils; ERROR: this extension does not support SET SCHEMA The documentation is updated both in the patch and here: http://pgsql.tapoueh.org/extensions/doc/html/extend-extension.html Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachment
On Thu, Jan 27, 2011 at 20:01, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > Ok, done. Of course, it solves the whole problem Itagaki had with > adminpack because we stop relying on dependencies to get it right now. I found pg_restore with -c option fails when an extension is created in pg_catalog. Since pg_catalog is an implicit search target, so we might need the catalog to search_path explicitly. Note that I can restore adminpack with not errors because it has explicit "pg_catalog." prefix in the installer script. ---- postgres=# CREATE EXTENSION cube WITH SCHEMA pg_catalog; $ pg_dump -Fc postgres > postgres.dump $ pg_restore -d postgres -c postgres.dump pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 1; 3996 16678 EXTENSION cube pg_restore: [archiver (db)] could not execute query: ERROR: no schema has been selected to create in ---- BTW, I have a minor comments for the code. extern bool extension_relocatable_p(Oid ext_oid); What is "_p" ? Also, we could make the function static because it is used only in extension.c. -- Itagaki Takahiro
Itagaki Takahiro <itagaki.takahiro@gmail.com> writes: > I found pg_restore with -c option fails when an extension is created > in pg_catalog. Since pg_catalog is an implicit search target, so we > might need the catalog to search_path explicitly. > Note that I can restore adminpack with not errors because it has > explicit "pg_catalog." prefix in the installer script. Nice catch, thank you very much (again) for finding those :) Please find inline the patch I've just applied that fixes the issue by managing the current search path and creation namespace before executing the script, the same way that schemacmds.c does. http://git.postgresql.org/gitweb?p=postgresql-extension.git;a=commitdiff;h=5c14c989426c0811e5bf8b993ae9a966fbf903c4 > BTW, I have a minor comments for the code. > extern bool extension_relocatable_p(Oid ext_oid); > What is "_p" ? Also, we could make the function static > because it is used only in extension.c. predicate. Maybe I've done too much Emacs Lisp coding at the time I added that function, but it looked natural (enough) to me :) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support commit 5c14c989426c0811e5bf8b993ae9a966fbf903c4 (HEAD, refs/remotes/origin/extension, refs/heads/extension) Author: Dimitri Fontaine <dim@tapoueh.org> Date: Thu Jan 27 14:40:10 2011 +0100 Fully set the creation namespace before to execute the extenions's script. Modified src/backend/commands/extension.c diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index 87310fb..4a8c757 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -415,6 +415,8 @@ execute_extension_script(ExtensionControlFile *control, char *filename = get_extension_absolute_path(control->script); char *old_cmsgs = NULL, *old_lmsgs = NULL; /* silence compiler */ bool reset_cmsgs = false, reset_lmsgs = false; + Oid target_schema; + OverrideSearchPath *overridePath; /* * Set create_extension and create_extension_with_user_data so that the @@ -453,6 +455,21 @@ execute_extension_script(ExtensionControlFile *control, SetConfigOption("log_min_messages", "warning",PGC_SUSET, PGC_S_SESSION); } + if (control->relocatable) + target_schema = LookupCreationNamespace(schema); + else + target_schema = get_namespace_oid(schema, false); + + /* + * Temporarily make the new namespace be the front of the search path, as + * well as the default creation target namespace. This will be undone at + * the end of this routine, or upon error. + */ + overridePath = GetOverrideSearchPath(CurrentMemoryContext); + overridePath->schemas = lcons_oid(target_schema, overridePath->schemas); + /* XXX should we clear overridePath->useTemp? */ + PushOverrideSearchPath(overridePath); + /* * On failure, ensure that create_extension does not remain set. */ @@ -474,7 +491,8 @@ execute_extension_script(ExtensionControlFile *control, if (control->relocatable) { List *search_path = fetch_search_path(false); - Oid first_schema, target_schema; + Oid first_schema = linitial_oid(search_path); + list_free(search_path); if (!execute_sql_string(sql)) ereport(ERROR, @@ -495,10 +513,6 @@ execute_extension_script(ExtensionControlFile *control, * We skip this step if the targetschema happens to be the * first schema of the current search_path. */ - first_schema = linitial_oid(search_path); - target_schema = LookupCreationNamespace(schema); - list_free(search_path); - if (first_schema != target_schema) AlterExtensionNamespace_oid(CreateExtensionAddress.objectId, target_schema); @@ -531,6 +545,9 @@ execute_extension_script(ExtensionControlFile *control, } PG_END_TRY(); + /* Reset search path to normal state */ + PopOverrideSearchPath(); + if (reset_cmsgs) SetConfigOption("client_min_messages", old_cmsgs, PGC_SUSET, PGC_S_SESSION);
On Thu, Jan 27, 2011 at 22:48, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > Itagaki Takahiro <itagaki.takahiro@gmail.com> writes: >> I found pg_restore with -c option fails when an extension is created >> in pg_catalog. > Nice catch, thank you very much (again) for finding those :) Seems good. >> extern bool extension_relocatable_p(Oid ext_oid); > predicate. Maybe I've done too much Emacs Lisp coding at the time I > added that function, but it looked natural (enough) to me :) Hmmm, I like extension_is_relocatable() or so... Including the above, I wrote a patch on your patch for minor cleanup. Please merge reasonable parts in it. * access() is not portable. The pre-checking with access() doesn't seems needed because the same error will be raised in parse_extension_control_file(). * There are some dead code in the patch. For example, you exported ObjectAddresses to public, but it is not used in extension.c actually. I reverted some of changes. * Should we support absolute control file paths? Absolute paths are supported by get_extension_absolute_path(), but I'm not sure actual use-cases. * Each ereport(ERROR) should have a reasonable errcode unless they are an internal logic error, and whether the error message follows our guidline (starting with a lower case character, etc.) * Changed create_extension_with_user_data to a static variable. CreateExtensionAddress and create_extension are still exported. We could have better names for them -- CurrentExtensionAddress and in_create_extension? Or, in_create_extension might be replaced with "CreateExtensionAddress.objectId != InvalidOid". * Added psql tab completion for CREATE/DROP/ALTER EXTENSION. * Use palloc0() instead of palloc() and memset(0). * Several code cleanup. -- Itagaki Takahiro
Attachment
Itagaki Takahiro <itagaki.takahiro@gmail.com> writes: > Hmmm, I like extension_is_relocatable() or so... > Including the above, I wrote a patch on your patch for minor > cleanup. Please merge reasonable parts in it. After review, I included all your proposed changes, thanks a lot! Please find attached a new version of the patch, v29, including your changes and merged against HEAD from this morning (there was no conflict, but still). > * access() is not portable. > The pre-checking with access() doesn't seems needed because > the same error will be raised in parse_extension_control_file(). > > * There are some dead code in the patch. > For example, you exported ObjectAddresses to public, but it is not > used in extension.c actually. I reverted some of changes. Thanks, those are due to late refactoring, removal of features and adjustments etc. Doing that in "free time" rather than full time these busy days, so I've missed some cleaning. > * Should we support absolute control file paths? > Absolute paths are supported by get_extension_absolute_path(), > but I'm not sure actual use-cases. Well I don't see no harm in allowing non-core compliant extension packaging, as this feature is not targeting only BSD compatible Open Source extensions such as contribs. It seems to me to be a case of mechanism versus policy, I don't think our policy here should mean that we alter the mechanism for others. > * Each ereport(ERROR) should have a reasonable errcode unless > they are an internal logic error, and whether the error message > follows our guidline (starting with a lower case character, etc.) Done. > * Changed create_extension_with_user_data to a static variable. > CreateExtensionAddress and create_extension are still exported. > We could have better names for them -- CurrentExtensionAddress > and in_create_extension? Or, in_create_extension might be > replaced with "CreateExtensionAddress.objectId != InvalidOid". Well there has been enough discussion about those names I think, current one where voted by Alvaro and Robert IIRC. I'm open to changes, but would now prefer to include that in the commiter's work :) > * Added psql tab completion for CREATE/DROP/ALTER EXTENSION. > * Use palloc0() instead of palloc() and memset(0). > * Several code cleanup. Thanks a lot! Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachment
On Fri, Jan 28, 2011 at 18:03, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > After review, I included all your proposed changes, thanks a lot! > Please find attached a new version of the patch, v29, Additional questions and discussions: * "relocatable" and "schema" seems to be duplicated options. We could treat an extension is relocatable when schema is not specified instead of relocatable option. Or, If we keep "schema" option, it would be used as the default schema to be installed when WITH SCHEMA is not specified. * "version" field in pg_available_extension might mislead when a newer version of an extension is available but an older version is installed. How about returning installed version for "installed" field instead of booleans? The field will be NULLs if not installed. * I want to remove O(n^2) behavior in pg_extensions(). It scans pg_extension catalog to return whether the extension is installed, but it would be better to change the function to return just whole extensions and JOIN with pg_extension in pg_available_extensions. (it's the same technique used in pg_stat_replication) -- Itagaki Takahiro
Itagaki Takahiro <itagaki.takahiro@gmail.com> writes: > * "relocatable" and "schema" seems to be duplicated options. They are not, really. If you have a relocatable extension, then there's no schema option in the control file (setting it is an ERROR). If you have a non-relocatable extension, then you can either setup the schema to force it as the extension's author (or distributor / packager), or leave it alone and use the @extschema@ facility instead. > * "version" field in pg_available_extension might mislead when > a newer version of an extension is available but an older version > is installed. How about returning installed version for "installed" > field instead of booleans? The field will be NULLs if not installed. Good idea, I've done that in the pg_available_extension system view. > * I want to remove O(n^2) behavior in pg_extensions(). It scans > pg_extension catalog to return whether the extension is installed, > but it would be better to change the function to return just whole > extensions and JOIN with pg_extension in pg_available_extensions. > (it's the same technique used in pg_stat_replication) Well, that allows to get rid of the whole extension's listing internal function. Less code is good :) Here's the new system's view: http://pgsql.tapoueh.org/extensions/doc/html/view-pg-available-extensions.html CREATE VIEW pg_available_extensions AS SELECT n.nspname as "schema", E.name, X.extversion as "installed", E.version, E.relocatable, E.comment FROM pg_available_extensions() AS E LEFT JOIN pg_extension as X ONE.name = X.extname LEFT JOIN pg_namespace as N on N.oid = X.extnamespace; The new code (and documentation) is published in the git repository, I'm waiting a little bit more before (for your comments) to prepare the patch v30, or just tell me and I'll do that. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Hi, the attached is a further cleanup of the latest commit (1db20cdd36cb1c2cc5ef2210a23b3c09f5058690). * Accept paths under $PGSHARE during CREATE EXTENSION "in addition to" normal paths in convert_and_check_filename(). I think we don't have to disallow normal file accesses in the case. * Rewrite pg_available_extensions() to use materialize mode. * Add a protection for nested CREATE EXTENSION calls. * Removed some unneeded changes in the patch: - utils/genfile.h (use DIR directly) - missing merges from master in guc.c - only #include changes in a few files Comments and documentation would need to be checked native English speakers. I cannot help you In this area, sorry. There are last two issues before it goes to ready for committer. On Mon, Jan 31, 2011 at 19:21, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: >> * "relocatable" and "schema" seems to be duplicated options. > They are not, really. I'd suggest to remove "relocatable" option if you won't add additional meanings to the "relocatable but having schema" case. Or, I'm still not sure why only adminpack is not relocatable. Is it possible to make all extensions to relocatable and add "default_schema = pg_catalog" to adminpack for backward-compatibility? >From older mails: >> * Should we support absolute control file paths? > Well I don't see no harm in allowing non-core compliant extension > packaging, If you want to support absolute paths, you also need to adjust convert_and_check_filename() because it only allows to read files in $PGSHARE. So, absolute path support doesn't work actually. I prefer to remove absolute path support from script option because absolute paths are just unportable. -- Itagaki Takahiro
Attachment
Itagaki Takahiro <itagaki.takahiro@gmail.com> writes: > Hi, the attached is a further cleanup of the latest commit > (1db20cdd36cb1c2cc5ef2210a23b3c09f5058690). Thanks! Given that the patch contains some merging from master's branch, I'm not sure if I should apply it to my repository then handle conflicts, or let you manage the patch now? > * Accept paths under $PGSHARE during CREATE EXTENSION > "in addition to" normal paths in convert_and_check_filename(). > I think we don't have to disallow normal file accesses in the case. > * Rewrite pg_available_extensions() to use materialize mode. > * Add a protection for nested CREATE EXTENSION calls. > * Removed some unneeded changes in the patch: > - utils/genfile.h (use DIR directly) > - missing merges from master in guc.c > - only #include changes in a few files > > Comments and documentation would need to be checked native > English speakers. I cannot help you In this area, sorry. Thanks. I don't see the PATH modifications when reading the patch, though. > There are last two issues before it goes to ready for committer. > > On Mon, Jan 31, 2011 at 19:21, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: >>> * "relocatable" and "schema" seems to be duplicated options. >> They are not, really. > > I'd suggest to remove "relocatable" option if you won't add > additional meanings to the "relocatable but having schema" case. The schema option only makes sense when the extension is *NOT* relocatable. > Or, I'm still not sure why only adminpack is not relocatable. > Is it possible to make all extensions to relocatable and add > "default_schema = pg_catalog" to adminpack for backward-compatibility? The adminpack SQL script is hard-coding pg_catalog, so user won't be able to choose where to install it. Technically, they could still move the functions to another schema, but that would break pgAdmin AFAIUI, so the extension's author here *wants* to forbid the user to relocate the extension. And want to prevent to user from installing it where he wants in the first place. The option relocatable is allowing ALTER EXTENSION … SET SCHEMA, when the control files also specify the schema, then you can't choose where to install the extension in the first place. I don't think we can go to only 1 options here. > From older mails: >>> * Should we support absolute control file paths? >> Well I don't see no harm in allowing non-core compliant extension >> packaging, > > If you want to support absolute paths, you also need to adjust > convert_and_check_filename() because it only allows to read files > in $PGSHARE. So, absolute path support doesn't work actually. > I prefer to remove absolute path support from script option > because absolute paths are just unportable. I have no strong opinion here, ok for me. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > Itagaki Takahiro <itagaki.takahiro@gmail.com> writes: >> Hi, the attached is a further cleanup of the latest commit >> (1db20cdd36cb1c2cc5ef2210a23b3c09f5058690). > Thanks! Given that the patch contains some merging from master's > branch, I'm not sure if I should apply it to my repository then handle > conflicts, or let you manage the patch now? Actually, I was about to pick up and start working on the whole extensions patch series, but now I'm confused as to what and where is the latest version. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Actually, I was about to pick up and start working on the whole > extensions patch series, but now I'm confused as to what and where is > the latest version. Ok, here's what I have, attached, as patch v30. What Itagaki just sent applies on top of that. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachment
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > Tom Lane <tgl@sss.pgh.pa.us> writes: >> Actually, I was about to pick up and start working on the whole >> extensions patch series, but now I'm confused as to what and where is >> the latest version. > Ok, here's what I have, attached, as patch v30. What Itagaki just sent > applies on top of that. What are the guc.c changes in this patch? They appear completely unrelated to the topic. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > What are the guc.c changes in this patch? They appear completely > unrelated to the topic. Right. Didn't spot them. Sorry for the noise in the patch, it must be a merge problem in my repository. Do you want me to clean that up or is it already to late? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > Tom Lane <tgl@sss.pgh.pa.us> writes: >> What are the guc.c changes in this patch? They appear completely >> unrelated to the topic. > Right. Didn't spot them. Sorry for the noise in the patch, it must be > a merge problem in my repository. Do you want me to clean that up or is > it already to late? No need, I'll just drop that file from the patch. regards, tom lane
While I'm looking at this ... what is the rationale for treating rewrite rules as members of extensions, ie, why does the patch touch rewriteDefine.c? ISTM a rule is a property of a table and could not sensibly be an independent member of an extension. If there is a use for that, why are table constraints and triggers not given the same treatment? regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > While I'm looking at this ... what is the rationale for treating rewrite > rules as members of extensions, ie, why does the patch touch > rewriteDefine.c? ISTM a rule is a property of a table and could not > sensibly be an independent member of an extension. If there is a use > for that, why are table constraints and triggers not given the same > treatment? I remember thinking I needed to do that for CREATE VIEW support while discovering PostgreSQL internals. Regards. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attached is an updated patch that incorporates all of the review I've done so far on the core code. This omits the contrib changes, which I've not looked at in any detail, and the docs changes since I've not yet updated the docs to match today's code changes. User-visible changes are that WITH NO DATA is gone, and instead there's pg_extension_config_dump(regclass, text) as per today's discussion. The latter is only lightly tested but seems to work as intended. I believe the core code is now in pretty good shape; the only open issue I have at the moment is that pg_dump will fail to suppress dumping of USER MAPPING objects that are in an extension. Which is something I'm not too excited about anyway. (The reason that's an issue is that I reverted most of the changes in pg_dump.c in favor of using pg_dump's already existing dependency mechanisms to suppress dumping unwanted items. The USER MAPPING code tries to bypass the DumpableObject representation, which means it doesn't get filtered.) The documentation still needs a good bit of work, but I hope to have this committed within a day or so. regards, tom lane
Attachment
Tom Lane <tgl@sss.pgh.pa.us> writes: > Attached is an updated patch that incorporates all of the review I've And that looks great, thanks. I've only had time to read the patch, will play with it later on today, hopefully. I've spotted a comment that I think you missed updating. The schema given in the control file is now created in all cases rather than only when the extension is not relocatable, right? + else if (control->schema != NULL) + { + /* + * The extension is not relocatable and the author gave us a schema + * for it. We create the schema here if it does not already exist. + */ I also note that the attached version doesn't contain \dX, is that an happenstance or a choice you did here? > done so far on the core code. This omits the contrib changes, which > I've not looked at in any detail, and the docs changes since I've not > yet updated the docs to match today's code changes. User-visible > changes are that WITH NO DATA is gone, and instead there's > pg_extension_config_dump(regclass, text) as per today's discussion. > The latter is only lightly tested but seems to work as intended. I think I will have to test it and get my head around it, but as we said, it's a good change (simpler, only target user tables). > I believe the core code is now in pretty good shape; the only open issue > I have at the moment is that pg_dump will fail to suppress dumping of > USER MAPPING objects that are in an extension. Which is something I'm > not too excited about anyway. (The reason that's an issue is that I > reverted most of the changes in pg_dump.c in favor of using pg_dump's > already existing dependency mechanisms to suppress dumping unwanted > items. The USER MAPPING code tries to bypass the DumpableObject > representation, which means it doesn't get filtered.) Well the pg_dump support code is very different than the one I did, so I will have to learn about the dependency management there before I can comment. > The documentation still needs a good bit of work, but I hope to have > this committed within a day or so. Great! Again, thanks a lot, I like the way you simplified and cleaned the patch, and I still recognise the code :) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > I've spotted a comment that I think you missed updating. The schema > given in the control file is now created in all cases rather than only > when the extension is not relocatable, right? Hm, no, that logic is the same as before no? > I also note that the attached version doesn't contain \dX, is that an > happenstance or a choice you did here? I removed it --- it wasn't documented and I didn't see much point anyway in a \d command that just duplicates a system view, especially a view only usable by superusers. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: >> I've spotted a comment that I think you missed updating. The schema >> given in the control file is now created in all cases rather than only >> when the extension is not relocatable, right? > > Hm, no, that logic is the same as before no? Well I had if (!control->relocatable && control->schema != NULL) And you have + else if (control->schema != NULL) So you're considering the schema option independently of the relocatable option, which is ok because you changed those options defaults and conflicts in the control file parsing. Only the comment needs adjusting. > I removed it --- it wasn't documented and I didn't see much point anyway > in a \d command that just duplicates a system view, especially a view > only usable by superusers. It used to not be a straight select from system view, and I wanted to offer something as simple as clicking a check box in pgadmin for people who want to see what's available and install it. But well, the system view is good enough now I guess, we're talking about DBA here after all. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > Tom Lane <tgl@sss.pgh.pa.us> writes: >> Hm, no, that logic is the same as before no? > Well I had > if (!control->relocatable && control->schema != NULL) > And you have > + else if (control->schema != NULL) Yeah, I deleted that relocatable test because it's redundant: control->schema cannot be set for a relocatable extension, cf the test in read_extension_control_file. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Yeah, I deleted that relocatable test because it's redundant: > control->schema cannot be set for a relocatable extension, > cf the test in read_extension_control_file. I missed that you kept this test in your version of the patch. Sorry for noise. Regardsm -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
I have gone ahead and committed the core and documentation parts of this patch, but not as yet any of the contrib changes; that is, all the contrib modules are still building old-style. I had intended to do it in two steps all along, so as to get some buildfarm proof for the thesis that we haven't broken old-style add-on modules. However, there is now an additional motivation for delay: so long as we haven't pulled the trigger on changing the contrib modules, this is an experimental feature that nothing else depends on. Worst case, if we can't get to a satisfactory resolution of the pg_upgrade and ALTER EXTENSION UPGRADE issues, we can ship 9.1 as-is and just label the EXTENSION commands as subject to change. I will however now go work on those issues. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > I have gone ahead and committed the core and documentation parts of this Thank you! And I'd like to take the time to thank all of you who helped me reach this goal, but that ranges to about everyone here who I talked to at any conference those last two or three years (I still remember talking about that at PGDay 2008 in Prato, but the ball was already rolling if only in my head). You might also be interested to know that the research leading to these results has received funding from the European Union's Seventh Framework Programme (FP7/2007-2013) under grant agreement 258862. > patch, but not as yet any of the contrib changes; that is, all the > contrib modules are still building old-style. I had intended to do it > in two steps all along, so as to get some buildfarm proof for the thesis > that we haven't broken old-style add-on modules. However, there is now > an additional motivation for delay: so long as we haven't pulled the > trigger on changing the contrib modules, this is an experimental feature > that nothing else depends on. Worst case, if we can't get to a > satisfactory resolution of the pg_upgrade and ALTER EXTENSION UPGRADE > issues, we can ship 9.1 as-is and just label the EXTENSION commands as > subject to change. I will however now go work on those issues. Wise move. Again :) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support