Thread: Extensions, patch 22 (cleanup, review, cleanup)
Hi, >From last round of review from Robert and Álvaro, here's the patch version 22. Changes: - cleanup contrib/ and 'Adjust search_path' comments - remove contrib/*/uninstall* scripts - add some documentation to the NO USER DATA option - remove objects locking in the pg_extension_objects() SRF, per Robert - propose split patches It so happens that git allows to prepare diffs for subdirs (or files) of a tree, so I've been using that to separate away contrib changes from the others. As a result, 3 patches are attached: full, contrib only, doc and src only. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachment
On Mon, December 20, 2010 22:35, Dimitri Fontaine wrote:
> Hi,
>
> From last round of review from Robert and �lvaro, here's the patch
> version 22. Changes:
>
> - cleanup contrib/ and 'Adjust search_path' comments
> - remove contrib/*/uninstall* scripts
> - add some documentation to the NO USER DATA option
> - remove objects locking in the pg_extension_objects() SRF, per Robert
> - propose split patches
>
I used the 'full' patch.
During configure I spotted this:
[...]
checking for bison... /usr/bin/bison
configure: using bison (GNU Bison) 2.3
gawk: { if ($4 < 1.875-extension) exit 0; else exit 1;}
gawk: ^ syntax error
gawk: { if ($4 < 1.875-extension) exit 0; else exit 1;}
gawk: ^ syntax error
checking for flex... /usr/local/bin/flex
configure: using flex 2.5.3
[...]
Otherwise the patch applies, and compiles, checks, installs and runs OK. (obviously I haven't
tested anything yet)
Linux Centos 5.4
Erik Rijkers
On Mon, December 20, 2010 22:55, Erik Rijkers wrote:
> On Mon, December 20, 2010 22:35, Dimitri Fontaine wrote:
>
> During configure I spotted this:
>
> [...]
> checking for bison... /usr/bin/bison
> configure: using bison (GNU Bison) 2.3
> gawk: { if ($4 < 1.875-extension) exit 0; else exit 1;}
> gawk: ^ syntax error
> gawk: { if ($4 < 1.875-extension) exit 0; else exit 1;}
> gawk: ^ syntax error
> checking for flex... /usr/local/bin/flex
> configure: using flex 2.5.3
> [...]
>
Apologies - please ignore the above ...
It turns out this is an artifact of my build script hacking of the configure file.
> Otherwise the patch applies, and compiles, checks, installs and runs OK. (obviously I haven't
> tested anything yet)
>
> Linux Centos 5.4
>
> Erik Rijkers
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
Excerpts from Dimitri Fontaine's message of lun dic 20 18:35:44 -0300 2010:
> Hi,
>
> From last round of review from Robert and Álvaro, here's the patch
> version 22. Changes:
I noticed this bit in the docs:
The admin
function <link linkend="functions-extension">pg_extension_flag_dump</link>
can be used to revert the default <literal>pg_dump</literal> policy
about objects that belong to an extension and force the flagged objects
to be part of the backups.
However, the code to that function contains this bit:
/*
* CREATE EXTENSION is superuser only and we check we're in that code
* path, so we don't add another explicit check for superuser here.
*/
if (!create_extension)
ereport(ERROR,
(errmsg("this function can only be used from CREATE EXTENSION")));
So presumably this shouldn't be documented because it cannot be called
anyway?
To be honest I don't understand the purpose of this part of the patch.
I attach some minor fixes while reading it over. I compiled but didn't
run it :-)
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Attachment
>> On Mon, December 20, 2010 22:35, Dimitri Fontaine wrote: > > I might be mistaken but it looks like a doc/src/sgml/ref/alter_extension.sgml is missing?
"Erik Rijkers" <er@xs4all.nl> writes: > I might be mistaken but it looks like a doc/src/sgml/ref/alter_extension.sgml is missing? Mmm, it seems that git was agreeing with you, so here's it: git ls-files doc/src/sgml/ref/alter_extension.sgml http://git.postgresql.org/gitweb?p=postgresql-extension.git;a=commitdiff;h=9371a9763651df2636cb6c20dced7cd67398c477 It was already online for readers of the HTML version of the docs: http://pgsql.tapoueh.org/extensions/doc/html/sql-alterextension.html And it will appear in next revision of the patch. Thanks! -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Alvaro Herrera <alvherre@commandprompt.com> writes: > function <link linkend="functions-extension">pg_extension_flag_dump</link> [...] > So presumably this shouldn't be documented because it cannot be called > anyway? It can be called but only from an extension's script. > To be honest I don't understand the purpose of this part of the patch. So the problem we're offering a solution for, here, is the extension with user data problem: the extension infrastructure is only there so that pg_dump knows to filter OUT sql objects from its dump, prefering a single create extension command. Some extension allows users to control the data in some of they're objects: now you want to have those in the backup again. From the docs: http://pgsql.tapoueh.org/extensions/doc/html/functions-admin.html#FUNCTIONS-EXTENSION pg_extension_with_user_data allows extension's author to prepare installation scripts that will work well for initial installationand when restoring from a pg_dump backup, which issues CREATE EXTENSION foo WITH NO USER DATA;. See CREATE EXTENSIONfor details. One way to use it is as following: DO $$ BEGIN IF pg_extension_with_user_data() THEN create schema foo; create table foo.bar(id serial primary key); perform pg_extension_flag_dump('foo.bar_id_seq'::regclass); perform pg_extension_flag_dump('foo.bar::regclass); END IF; END; $$; I don't really know how to improve the docs, you seem to have been surprised by reading the CREATE EXTENSION docs but you didn't follow the link to the function's doc with the above details, did you? I'm open to improving things here, but I'm not seeing how yet. > I attach some minor fixes while reading it over. I compiled but didn't > run it :-) Thanks a lot, that's applied in my git repo, and I did run it successfully! It will be part of the next patch revision. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Tue, December 21, 2010 09:57, Dimitri Fontaine wrote: > "Erik Rijkers" <er@xs4all.nl> writes: >> I might be mistaken but it looks like a doc/src/sgml/ref/alter_extension.sgml is missing? > > Mmm, it seems that git was agreeing with you, so here's it: > > git ls-files doc/src/sgml/ref/alter_extension.sgml > http://git.postgresql.org/gitweb?p=postgresql-extension.git;a=commitdiff;h=9371a9763651df2636cb6c20dced7cd67398c477 > > It was already online for readers of the HTML version of the docs: > > http://pgsql.tapoueh.org/extensions/doc/html/sql-alterextension.html > > And it will appear in next revision of the patch. Thanks! > -- > Dimitri Fontaine > http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support > Two changes to sql-alterextension.sgml: ALTER EXTENSION name SET EXTENSION new_schema should be: ALTER EXTENSION name SET SCHEMA new_schema And in the 'Description' there are (I think) old copy/paste remnants: ALTER EXTENSION changes the definition of an existing type. There are only one subforms: SET SCHEMA it should be (something like): ALTER EXTENSION changes an existing extension. There is only one form: ALTER EXTENSION set schema new_schema Erik Rijkers
"Erik Rijkers" <er@xs4all.nl> writes: >> http://pgsql.tapoueh.org/extensions/doc/html/sql-alterextension.html [...] > Two changes to sql-alterextension.sgml: Fixed and uploaded on the URL above, will be in the next patch revision, thanks! Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support