Extension Templates S03E11
Hi,
Please find attached to this email the latest and greatest version of
in-line SQL only extensions support, known as "Extension Templates" and
which could be renamed "In-Catalog Extension Templates".
I've included a high-level description of the patch in a style that
targets the detailed commit messages for features of that source code
impact level.
The attached patch is known to address all points raised in the previous
reviews and to implement the best design we could come up with, thanks
to immense helping from Tom, Heikki and Markus. Of course, bugs are all
my precious.
I'm going to register that patch to the next commitfest. It's not the
only patch I intend to register for september though, as I want to get
to a usable situation with Event Triggers, so you can expect a series of
patches for that, covering what couldn't make it previously.
As I think this WIP is about as ready-for-committer as it will ever be,
it would be fantastic if we could do a single committer review before
CF2013-09 so that I know that it's going to be accepted… or not. Well at
least it's in the queue already, we'll see what can be done.
Regards,
---
Implement in-catalog Extension Template facility.
Previously, the only way to CREATE EXTENSION involved installing file
system templates in a place generally owned by root: creation scripts,
upgrade scripts, main control file and auxilliary control files. This
patch implements a way to upload all those resources into the catalogs,
so that a PostgreSQL connection is all you need to make an extension
available.
By design and for security concerns the current Extension Template
facility is not able to deal with extensions that need to load a DSO
module into the backend. Using any other PL is supported though.
An extension created from a template depends on it, and the templates
are part of any backup script taken with pg_dump. So that at pg_restore
time, when CREATE EXTENSION is executed the templates are already in
place.
To be able to do that, though, we need a difference in behavior in
between the classic file system level templates and the catalog
templates: there's no dependency tracking happening at all with file
system templates and those can be changed at will even if an extension
has been already instanciated from the templates, or even removed.
Apart from the dependency tracking, the only other difference between
file system templates and catalog templates for extensions is that the
later are managed per-database. The file system level templates being
managed per major version of PostgreSQL is considered a drawback of that
method and not to be immitated by the in-catalog system, more flexible
by design.
At CREATE EXTENSION time, the file system templates are always prefered
to the catalog templates. Also, it's prohibited to make available an
extension in the catalogs if an extension of the same name is already
available from file system templates. That said, some "race conditions"
make it still possible to have the same extension name available as a
file system template and a catalog template. Even if only the former
will ever get installed, it's been deemed prudent to restrict the
in-catalog templates for extensions to superusers only.
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachments:
On 1 August 2013 18:01, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:
Hi,
Please find attached to this email the latest and greatest version of
in-line SQL only extensions support, known as "Extension Templates" and
which could be renamed "In-Catalog Extension Templates".I've included a high-level description of the patch in a style that
targets the detailed commit messages for features of that source code
impact level.The attached patch is known to address all points raised in the previous
reviews and to implement the best design we could come up with, thanks
to immense helping from Tom, Heikki and Markus. Of course, bugs are all
my precious.I'm going to register that patch to the next commitfest. It's not the
only patch I intend to register for september though, as I want to get
to a usable situation with Event Triggers, so you can expect a series of
patches for that, covering what couldn't make it previously.As I think this WIP is about as ready-for-committer as it will ever be,
it would be fantastic if we could do a single committer review before
CF2013-09 so that I know that it's going to be accepted… or not. Well at
least it's in the queue already, we'll see what can be done.Regards,
---
Implement in-catalog Extension Template facility.Previously, the only way to CREATE EXTENSION involved installing file
system templates in a place generally owned by root: creation scripts,
upgrade scripts, main control file and auxilliary control files. This
patch implements a way to upload all those resources into the catalogs,
so that a PostgreSQL connection is all you need to make an extension
available.By design and for security concerns the current Extension Template
facility is not able to deal with extensions that need to load a DSO
module into the backend. Using any other PL is supported though.An extension created from a template depends on it, and the templates
are part of any backup script taken with pg_dump. So that at pg_restore
time, when CREATE EXTENSION is executed the templates are already in
place.To be able to do that, though, we need a difference in behavior in
between the classic file system level templates and the catalog
templates: there's no dependency tracking happening at all with file
system templates and those can be changed at will even if an extension
has been already instanciated from the templates, or even removed.Apart from the dependency tracking, the only other difference between
file system templates and catalog templates for extensions is that the
later are managed per-database. The file system level templates being
managed per major version of PostgreSQL is considered a drawback of that
method and not to be immitated by the in-catalog system, more flexible
by design.At CREATE EXTENSION time, the file system templates are always prefered
to the catalog templates. Also, it's prohibited to make available an
extension in the catalogs if an extension of the same name is already
available from file system templates. That said, some "race conditions"
make it still possible to have the same extension name available as a
file system template and a catalog template. Even if only the former
will ever get installed, it's been deemed prudent to restrict the
in-catalog templates for extensions to superusers only.
Could you please resubmit this without using SnapshotNow as it's no longer
supported?
Thanks
Thom
Thom Brown <thom@linux.com> writes:
Could you please resubmit this without using SnapshotNow as it's no longer
supported?
Sure, sorry that I missed that, please find v12 attached.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachments:
Hi,
2013-08-04 15:20 keltez�ssel, Dimitri Fontaine �rta:
Thom Brown <thom@linux.com> writes:
Could you please resubmit this without using SnapshotNow as it's no longer
supported?Sure, sorry that I missed that, please find v12 attached.
Here's a review for this patch.
* Is the patch in a patch format which has context? (eg: context diff format)
Yes.
* Does it apply cleanly to the current git master?
No, it has one reject in src/bin/pg_dump/pg_backup_archiver.c.
It was obvious to fix, version 12a is attached.
* Does it include reasonable tests, necessary doc patches, etc?
It has extended the SQL reference nicely but the reference to
<xref linkend="extend-extensions"> was not obvious enough
regarding the list of control parameters.
The SQL syntax has them in allcaps, while the referenced section
has them in lower case. It's easy to miss them while just browsing
for e.g. RELOCATABLE. I had to go back twice to find the proper
part of the text.
I would like to see the control parameters documented in allcaps
in CREATE EXTENSION TEMPLATE. Then ALTER EXTENSION
TEMPLATE should reference the CREATE instead of the longer
text in <xref linkend="extend-extensions">. This xref can still
be there for reference in all three of CREATE/ALTER/DROP
EXTENSION TEMPLATE.
* Does the patch actually implement what it's supposed to do?
Yes.
* Do we want that?
Yes.
* Do we already have it?
No.
* Does it follow SQL spec, or the community-agreed behavior?
There's no such provision in the spec.
As far as I can tell, it follows the community-agreed behavior.
* Does it include pg_dump support (if applicable)?
Yes.
But the version check is already wrong in src/bin/pg_dump/pg_dump.c
since this patch missed 9.3.
+ /*
+ * Before 9.3, there are no extension templates.
+ */
+ if (fout->remoteVersion < 90300)
+ {
+ *numExtensionTemplates = 0;
+ return NULL;
+ }
+
* Are there dangers?
I don't think so.
* Have all the bases been covered?
It seems so.
* Does the feature work as advertised?
Yes.
* Are there corner cases the author has failed to consider?
I don't know.
* Are there any assertion failures or crashes?
No.
* Does the patch slow down simple tests?
No.
* If it claims to improve performance, does it?
n/a
* Does it slow down other things?
No.
* Does it follow the project coding guidelines?
Yes.
Nitpicking. This chunk has an extra unnecessary space:
diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile
index c4d3f3c..689dc37 100644
--- a/src/backend/catalog/Makefile
+++ b/src/backend/catalog/Makefile
@@ -38,6 +38,7 @@ POSTGRES_BKI_SRCS = $(addprefix $(top_srcdir)/src/include/catalog/,\
pg_authid.h pg_auth_members.h pg_shdepend.h pg_shdescription.h \
pg_ts_config.h pg_ts_config_map.h pg_ts_dict.h \
pg_ts_parser.h pg_ts_template.h pg_extension.h \
+ pg_extension_control.h pg_extension_template.h pg_extension_uptmpl.h \
pg_foreign_data_wrapper.h pg_foreign_server.h pg_user_mapping.h \
pg_foreign_table.h \
pg_default_acl.h pg_seclabel.h pg_shseclabel.h pg_collation.h pg_range.h \
* Are there portability issues?
No.
* Will it work on Windows/BSD etc?
It should.
* Are the comments sufficient and accurate?
Yes.
* Does it do what it says, correctly?
According to the added regression tests, yes.
* Does it produce compiler warnings?
No.
* Can you make it crash?
No.
* Is everything done in a way that fits together coherently with other features/modules?
I think so.
* Are there interdependencies that can cause problems?
I don't know.
Best regards,
Zolt�n B�sz�rm�nyi
--
----------------------------------
Zolt�n B�sz�rm�nyi
Cybertec Sch�nig & Sch�nig GmbH
Gr�hrm�hlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
Attachments:
Hi,
Boszormenyi Zoltan <zb@cybertec.at> writes:
Here's a review for this patch.
Thanks for that review Zoltan!
No, it has one reject in src/bin/pg_dump/pg_backup_archiver.c.
It was obvious to fix, version 12a is attached.
Included in the new version of the patch (v13), attached.
It has extended the SQL reference nicely but the reference to
<xref linkend="extend-extensions"> was not obvious enough
regarding the list of control parameters.
Fixed in the attached.
I would like to see the control parameters documented in allcaps
in CREATE EXTENSION TEMPLATE. Then ALTER EXTENSION
TEMPLATE should reference the CREATE instead of the longer
text in <xref linkend="extend-extensions">. This xref can still
be there for reference in all three of CREATE/ALTER/DROP
EXTENSION TEMPLATE.
I didn't follow exactly your proposal here because I didn't want to have
to maintain the control parameter description list in two different
places. I've still added a detailed list with references and details and
more importantly with coverage of e.g. "NORELOCATABLE" which is not
covered in the referenced material.
But the version check is already wrong in src/bin/pg_dump/pg_dump.c
since this patch missed 9.3.+ if (fout->remoteVersion < 90300)
Fixed.
Nitpicking. This chunk has an extra unnecessary space:
Fixed.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachments:
templates.v13.patch.gzapplication/octet-streamDownload
� E�R�=iW�������sb�G����9G���D|s|}t���4���23�����WU���-���p�
���������^�����M7t6�����&��������Q�V�uy�z�?���-kgo�Q�S������`�s�9�#�>����3��n�f����0`a�O�8�1k�r�\��/���U��en5A���s�%!��H"s��������h�]n����^Y���8r�����K��C�����I���Q��#�N�������{��y@e������v ,��c�X����%������;I`��e'����8��-�,d%^�'