Re: [HACKERS] Extensions support for pg_dump, patch v27
Tom Lane t...@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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Tom Lane t...@sss.pgh.pa.us writes: Dimitri Fontaine dimi...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
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 binrRC1pCyMTR.bin Description: extensions.v31.patch.gz -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Itagaki Takahiro itagaki.takah...@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 dimi...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Itagaki Takahiro itagaki.takah...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Itagaki Takahiro itagaki.takah...@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 ON E.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
On Fri, Jan 28, 2011 at 18:03, Dimitri Fontaine dimi...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
On Thu, Jan 27, 2011 at 20:01, Dimitri Fontaine dimi...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Itagaki Takahiro itagaki.takah...@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 d...@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 */
boolreset_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 target schema 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);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
On Thu, Jan 27, 2011 at 22:48, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Itagaki Takahiro itagaki.takah...@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 extension-diff-on.v28a.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Itagaki Takahiro itagaki.takah...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Tom Lane t...@sss.pgh.pa.us writes: Dimitri Fontaine dimi...@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_depend where 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: Dimitri Fontaine dimi...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
On Mon, Jan 24, 2011 at 18:22, Dimitri Fontaine dimi...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Hi, Itagaki Takahiro itagaki.takah...@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.
Re: [HACKERS] Extensions support for pg_dump, patch v27
Itagaki Takahiro itagaki.takah...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
On Tue, Jan 25, 2011 at 04:23:41PM +0100, Dimitri Fontaine wrote: Itagaki Takahiro itagaki.takah...@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 da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
On Wed, Jan 26, 2011 at 02:57, David E. Wheeler da...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
