Re: Extensions support for pg_dump, patch v27

From: Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>
To: Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>
Cc: Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "David E(dot) Wheeler" <david(at)kineticode(dot)com>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-Hackers <pgsql-hackers(at)postgresql(dot)org>, Dave Page <dpage(at)postgresql(dot)org>, Guillaume Lelarge <guillaume(at)lelarge(dot)info>
Subject: Re: Extensions support for pg_dump, patch v27
Date: 2011-01-27 13:48:08
Message-ID: 87aaimcvaf.fsf@hi-media-techno.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)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(at)tapoueh(dot)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 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);

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2011-01-27 13:51:16 Re: Is there a way to build PostgreSQL client libraries with MinGW
Previous Message Xiaobo Gu 2011-01-27 13:27:25 Re: Is there a way to build PostgreSQL client libraries with MinGW