Review: extension template

On 07/05/2013 09:05 PM, Markus Wanner wrote:

The patch has already
been marked as 'returned with feedback', and I can support that
resolution (for this CF).

Oops.. I just realize it's only set to "waiting on author", now. I guess
I confused the two states. Please excuse my glitch.

Dimitri, do you agree to resolve with "returned with feedback" for this
CF? Or how would you like to proceed?

Josh, thanks for linking the review in the CF.

Regards

Markus Wanner

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

Thanks a lot for your detailed review!

Markus Wanner <markus@bluegap.ch> writes:

Initially, I was confused about what the patch is supposed to achieve.
The 'template' naming certainly contributed to that confusion. My mental

Yes, I did share this viewpoint over the naming of the feature, but Tom
insisted that we already have those kind of templates for text search.

The major distinguishing factor is not the 'template' character of
extensions "installed" that way, but the storage place of the its
control data: filesystem vs system catalog. I'd either recommend
appropriate renaming to reflect that fact and to avoid confusing users;
or follow the "template" model better and decouple the extension from
its template - with implications on extensions requiring additional
binary code. Thinking of it, I kind of like that approach...

Could you go into more details into your ideas here? I don't understand
what you're suggesting.

Compiling pg_upgrade_support in contrib fails:

$SRC/contrib/pg_upgrade_support/pg_upgrade_support.c:193:8:
error: too few arguments to function ‘InsertExtensionTuple’

I don't have that problem here. Will try to reproduce early next week.

As originally mentioned by Heikki, if the tplscript doesn't parse, the
error message is just "syntax error at or near". That matches the
behavior of extensions installed on the file system. However, given this
adds a second way, a hint about where this error actually comes from is
mandatory, IMO.

Will have a look at what it takes to implement support for better error
messages. May I suggest to implement that later, though? I think this is
an improvement over the current system that will be complicated to get
right and I don't want that to swamp the current patch. After all, this
patch is already in its 3rd development cycle…

Trying to re-create a pre-existing template properly throws 'template
for extension "$NAME" version "$VERSION" already exists'. However, if
the extension is already enabled for that database, it instead says:
"extension "$NAME" already exists". I can see why that's fine if you
assume a strong binding between the "instantiation" and the "template".

The idea here is to protect against mixing the file based extension
installation mechanism with the catalogs one. I can see now that the
already installed extension could have been installed using a template
in the first place, so that message now seems strange.

I still think that we shouldn't allow creating a template for an
extension that is already installed, though. Do you have any suggestions
for a better error message?

However, it's possible to enable an extension and then rename its
template. The binding (as in pg_depend) is still there, but the above
error (in that case "extension $OLD_NAME already existing") certainly
doesn't make sense. One can argue whether or not an extension with a
different name is still the same extension at all...

When renaming a template, the check against existing extensions of the
same name is made against the new name of the template, so I don't see
what you say here in the code. Do you have a test case?

Trying to alter an inexistent or file-system stored extension doesn't
throw an error, but silently succeeds. Especially in the latter case,
that's utterly misleading. Please fix.

Fixed in my github branch, not producing a new patch version as of yet,
I think we need to set the new error messages first and I'm running
short of inspiration tonight.

That started to get me worried about the effects of a mixed
installation, but I quickly figured it's not possible to have a full
version on disk and then add an incremental upgrade via the system
catalogs. I think that's a fair limitation, as mixed installations would
pose their own set of issues. On the other hand, isn't ease of upgrades
a selling point for this feature?

The main issue to fix when you want to have that feature, which I want
to have, is how to define a sane pg_dump policy around the thing. As we
couldn't define that in the last rounds of reviews, we decided not to
allow the case yet.

I think that's a fair remark that we want to get there eventually, and
that like the superuser only limitation, that's something for a future
patch and not this one.

In any case, the error message could again be more specific:

(having extension 'pex' version '0.9' on the file-system)

# CREATE TEMPLATE FOR EXTENSION pex VERSION '1.0' ...
ERROR: extension "pex" already available

[ This one could mention it exists on the file-system. ]

# CREATE TEMPLATE FOR EXTENISON pex FROM '1.9' TO '1.10' AS ...
ERROR: no template for extension "pex"

This last error is technically correct only if you consider file system
extensions to not be templates. In any case, there is *something*
relevant called "pex" on the file-system, that prevents creation of the
template in the system catalogs. The error message should definitely be
more specific.

Will fix early next week.

With delight I note that renaming to an extension name that pre-exists
on the file-system is properly prohibited. Again, the error message
could be more specific and point to the appropriate place. However,
that's reasonably far down the wish-list.

[ Also note the nifty difference between "extension already available"
vs "extension already exists". The former seems to mean the "template"
exists, while the latter refers to the "instantiation". ]

Yeah, we might want to find some better naming for the on-file extension
"templates". We can't just call them extension templates though because
that is the new beast implemented in that patch and it needs to be part
of your pg_dump scripts, while the main feature of the file based kind
of templates is that they are *NOT* part of your dump.

That's the crux of that patch difficulties.

However, the other way around cannot be prevented by Postgres: Files
representing an extension (template, if you want) can always be created
alongside a pre-existing system catalog installation. Postgres has no
way to prevent that (other than ignoring the files, maybe, but...). It
seems the file system variant takes precedence over anything
pre-existing in the system catalogs. This should be documented.

Right. That will be documented in version 9 of the patch.

The documentation for ALTER TEMPLATE FOR EXTENSION says: "Currently, the
only supported functionality is to change the template's default
version." I don't understand what that's supposed to mean. You can
perfectly well rename and alter the template's code.

It's just something I forgot to cleanup when completing the feature set.
Cleaned up in my git branch.

In catalog/objectaddress.c, get_object_address_unqualified(): the case
OBJECT_TABLESPACE is being moved down. That looks like an like an
unintended change. Please revert.

Fixed in my github branch already, will appear in next patch version.

src/backend/commands/event_trigger.c, definition of
event_trigger_support: several unnecessary whitespaces added. These make
it hard(er) than necessary to review the patch.

Here with the following command I see no problem, please advise:

git diff --check --color-words postgres/master.. -- src/backend/commands/event_trigger.c

Markus Wanner <markus@bluegap.ch> writes:

Dimitri, do you agree to resolve with "returned with feedback" for this
CF? Or how would you like to proceed?

I'd like to proceed, it's the 3rd development cycle that I'm working on
this idea (used to be called "Inline Extensions", I also had a selective
pg_dump patch to allow dumping some extension scripts, etc). I really
wish we would be able to sort it out completely in this very CF and
that's why I'm not proposing any other patch this time.

Of course, we might still need another round at it, and that's life.

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

Salut Dimitri,

On 07/06/2013 10:30 PM, Dimitri Fontaine wrote:

Yes, I did share this viewpoint over the naming of the feature, but Tom
insisted that we already have those kind of templates for text search.

I think it's a question of what mental model we want extensions to
follow. See my other mail. IMO "inline" could well serve as a
differentiator against out-of-line, i.e. file-system based extensions
(or templates).

Could you go into more details into your ideas here? I don't understand
what you're suggesting.

Sorry for not making that clearer. See my follow-up mail "template-ify
(binary) extensions":
http://archives.postgresql.org/message-id/51D72C1D.7010609@bluegap.ch

Compiling pg_upgrade_support in contrib fails:

$SRC/contrib/pg_upgrade_support/pg_upgrade_support.c:193:8:
error: too few arguments to function ‘InsertExtensionTuple’

I don't have that problem here. Will try to reproduce early next week.

'make' followed by 'make -C contrib' reproduces that for me.

Will have a look at what it takes to implement support for better error
messages. May I suggest to implement that later, though?

Great, thanks. I agree this is not a major issue and can be deferred.

The idea here is to protect against mixing the file based extension
installation mechanism with the catalogs one. I can see now that the
already installed extension could have been installed using a template
in the first place, so that message now seems strange.

I still think that we shouldn't allow creating a template for an
extension that is already installed, though. Do you have any suggestions
for a better error message?

If we go for the template model, I beg to differ. In that mind-set, you
should be free to create (or delete) any kind of template without
affecting pre-existing extensions.

However, in case we follow the ancestor-derivate or link model with the
pg_depend connection, the error message seems fine.

However, it's possible to enable an extension and then rename its
template. The binding (as in pg_depend) is still there, but the above
error (in that case "extension $OLD_NAME already existing") certainly
doesn't make sense. One can argue whether or not an extension with a
different name is still the same extension at all...

When renaming a template, the check against existing extensions of the
same name is made against the new name of the template, so I don't see
what you say here in the code. Do you have a test case?

Consider this (not actually tested again, only off the top of my head):

# CREATE TEMPLATE FOR EXTENSION foo ...
ERROR: Extension "bar" already existing

"Uh? .. so what?" is my reaction to such an error message. It fails to
make the distinction between template and extension. Or rather parent
and derivate. The link between the two is the actual reason for the failure.

In any case, I'm arguing this "template" renaming behavior (and the
subsequent error) are the wrong thing to do, anyways. Because an
extension being linked to a parent of a different name is weird and IMO
not an acceptable state.

In the link model, the name should better be thought of as a property of
the parent. A rename of it should thus rename the derived extensions in
all databases. That would prevent the nasty case of having a parent with
different name than the derivative extension. (I note that existing
file-system extensions do not work that way. They are a lot closer to
the template model. However, that's just an argument for following that
as well for inline extensions and dropping the pg_depend link between
extension and template.)

In the template model, renaming the template should not have an effect
on any extension. Neither should creation or deletion of any template.
Thus, creating a template with the same name as a pre-existing extension
(in any database) is a completely fine and valid operation. No error
needs to be thrown. This nicely shows why I currently favor the template
approach: it seems easier to understand *and* easier to implement.

Trying to alter an inexistent or file-system stored extension doesn't
throw an error, but silently succeeds. Especially in the latter case,
that's utterly misleading. Please fix.

Fixed in my github branch

Nice.

That started to get me worried about the effects of a mixed
installation, but I quickly figured it's not possible to have a full
version on disk and then add an incremental upgrade via the system
catalogs. I think that's a fair limitation, as mixed installations would
pose their own set of issues. On the other hand, isn't ease of upgrades
a selling point for this feature?

The main issue to fix when you want to have that feature, which I want
to have, is how to define a sane pg_dump policy around the thing. As we
couldn't define that in the last rounds of reviews, we decided not to
allow the case yet.

I bet that's because people have different mental models in mind. But I
probably stressed that point enough by now...

I think that's a fair remark that we want to get there eventually, and
that like the superuser only limitation, that's something for a future
patch and not this one.

I agree to defer a specific implementation. I disagree in that we should
*not* commit something that follows a mixture of mental models
underneath and sets in stone a strange API we later need to be
compatible with.

Specifically, I request to either follow the template model more closely
(accompanied by a separate patch to adjust binary, out-of-line
templates) or follow the link model more closely. The current naming
doesn't match any of the two, so renaming seems inevitable.

Yeah, we might want to find some better naming for the on-file extension
"templates". We can't just call them extension templates though because
that is the new beast implemented in that patch and it needs to be part
of your pg_dump scripts, while the main feature of the file based kind
of templates is that they are *NOT* part of your dump.

That's one issue where the mixture of concepts shines through, yeah. I
fear simply (re)naming things is not quite enough, at this point.
Especially because the thing on the file-system is closer to being a
template than the system catalog based variant.

[ Maybe we could even go with "template extensions" being the
file-system ones vs. "inline extensions" being the system catalog
ones... In that case, they would follow different mental models. Which
might even be a reasonable solution. However, we need to be aware of
that difference and document it properly, so that users have a chance to
grasp the nifty difference. I'd rather prefer to eliminate such nifty
differences, though. ]

It's just something I forgot to cleanup when completing the feature set.
Cleaned up in my git branch.

Great!

src/backend/commands/event_trigger.c, definition of
event_trigger_support: several unnecessary whitespaces added. These make
it hard(er) than necessary to review the patch.

Here with the following command I see no problem, please advise:

git diff --check --color-words postgres/master.. -- src/backend/commands/event_trigger.c

<rant>That's why I personally don't trust git.</rant> Please look at the
patch you submitted.

Markus Wanner <markus@bluegap.ch> writes:

Dimitri, do you agree to resolve with "returned with feedback" for this
CF? Or how would you like to proceed?

I'd like to proceed, it's the 3rd development cycle that I'm working on
this idea

Okay, I'm certainly not opposed to that and welcome further development.
I didn't change the status, so it should still be "waiting on author",
which seems reasonable to me, ATM.

(used to be called "Inline Extensions", I also had a selective
pg_dump patch to allow dumping some extension scripts, etc). I really
wish we would be able to sort it out completely in this very CF and
that's why I'm not proposing any other patch this time.

I understand it's tedious work, sometimes. Please look at this as
incremental progress. I would not have been able to reason about mental
models without your patch. Thanks for that, it was a necessary and good
step from my point of view. Please keep up the good work!

Regards

Markus Wanner

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 07/06/2013 10:30 PM, Dimitri Fontaine wrote:

I still think that we shouldn't allow creating a template for an
extension that is already installed, though. Do you have any suggestions
for a better error message?

Oh, I just realize that pg_extension_{template,control,uptmpl} are not
SHARED catalogs, but you need to install the template per-database and
then need to enable it - per-database *again*. Why is that?

If you want to just upload extensions to a database via libpq, that
should be a single step (maybe rather just CREATE EXTENSION ... AS ...)
If you want "templates", those should be applicable to all databases, no?

Regards

Markus Wanner

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 07/07/2013 02:55 PM, Markus Wanner wrote:

If you want to just upload extensions to a database via libpq..

Let's rephrase this in a (hopefully) more constructive way: I get the
impression you are trying to satisfy many different needs. Way more that
you need to scratch your own itch. To the point that I had trouble
figuring out what exactly the goal of your patch is.

My advice would be: Be brave! Dare to put down any request for
"templates" (including mine) and go for the simplest possible
implementation that allows you to create extensions via libpq. (Provided
that really is the itch you want to scratch. I'm still not quite sure I
got that right.)

As it stands, I get the impression the patch is trying to sell me a
feature that it doesn't really provide. If you stripped the template
stuff, including the CREATE TEMPLATE FOR EXTENSION command, but just
sold me this patch as a simple way to create an extension via libpq,
i.e. something closer to CREATE EXTENSION AS .. , I would immediately
buy that. Currently, while allowing an upload, it seems far from simple,
but adds quite a bit of unwanted complexity. If all I want is to upload
code for an extension via libpq, I don't want to deal with nifty
distinctions between templates and extensions.

Just my opinion, though. Maybe I'm still missing something.

Regards

Markus Wanner

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Markus Wanner <markus@bluegap.ch> writes:

Oh, I just realize that pg_extension_{template,control,uptmpl} are not
SHARED catalogs, but you need to install the template per-database and
then need to enable it - per-database *again*. Why is that?

Because the current model is not serving us well enough, with a single
module version per major version of PostgreSQL. Meaning for all the
clusters on the server, and all the databases in them.

We want to be able to have postgis 1.5 and 2.x installed in two
different databases in the same cluster, don't we?

Well the current patch we still can't because of the dynamically shared
object side of things, but that's not a good reason to impose the same
limitation on to the "template" idea.

Currently, while allowing an upload, it seems far from simple,
but adds quite a bit of unwanted complexity. If all I want is to upload
code for an extension via libpq, I don't want to deal with nifty
distinctions between templates and extensions.

Just my opinion, though. Maybe I'm still missing something.

Yes: dump & restore.

After playing around with several ideas around that in the past two
development cycles, the community consensus clearly is that "extensions"
are *NEVER* going to be part of your dump scripts.

Now when using templates you have no other source to install the
extensions from at pg_restore time, given what I just said.

The whole goal of the "template" idea is to offer a way to dump and
restore the data you need for CREATE EXTENSION to just work at restore
time, even when you sent the data over the wire.

Current extension are managed on the file system, the contract is that
it is the user's job to maintain and ship that, externally to PostgreSQL
responsibilities. All that PostgreSQL knows about is to issue the CREATE
EXTENSION command at pg_restore time.

With templates or in-line extensions, the contract is that the user asks
PostgreSQL to manage its extensions in the same way it does for the
other objects on the system. The design we found to address that is
called "Extension Templates" and is implemented in the current patch.

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

Hello Dimitri,

On 07/07/2013 09:51 PM, Dimitri Fontaine wrote:

Markus Wanner <markus@bluegap.ch> writes:

Oh, I just realize that pg_extension_{template,control,uptmpl} are not
SHARED catalogs, but you need to install the template per-database and
then need to enable it - per-database *again*. Why is that?

Because the current model is not serving us well enough, with a single
module version per major version of PostgreSQL. Meaning for all the
clusters on the server, and all the databases in them.

That's not an excuse for letting the user duplicate the effort of
installing templates for every version of his extension in every database.

We want to be able to have postgis 1.5 and 2.x installed in two
different databases in the same cluster, don't we?

The extension, yes. The template versions, no. There's utterly no point
in having different 2.0 versions of the same extension in different
databases.

Well the current patch we still can't because of the dynamically shared
object side of things, but that's not a good reason to impose the same
limitation on to the "template" idea.

Without a dynamically shared object, you can well have different
versions of an extension at work in different databases already.

After playing around with several ideas around that in the past two
development cycles, the community consensus clearly is that "extensions"
are *NEVER* going to be part of your dump scripts.

Sounds strange to me. If you want Postgres to manage extensions, it
needs the ability to backup and restore them. Otherwise, it doesn't
really ... well ... manage them.

Now when using templates you have no other source to install the
extensions from at pg_restore time, given what I just said.

The whole goal of the "template" idea is to offer a way to dump and
restore the data you need for CREATE EXTENSION to just work at restore
time, even when you sent the data over the wire.

Which in turn violates the above cited community consensus, then. You
lost me here. What's your point? I thought the goal of your patch was
the ability to upload an extension via libpq.

How does that address my concerns about usability and understandability
of how these things work? Why the strange dependencies between templates
and extensions? Or the ability to rename the template, but not the
extension - while still having the later depend on the former?

These things are what I'm opposing to. And I don't believe it
necessarily needs to be exactly that way for dump and restore to work.
Quite the opposite, in fact. Simpler design usually means simpler backup
and restore procedures.

Current extension are managed on the file system, the contract is that
it is the user's job to maintain and ship that, externally to PostgreSQL
responsibilities. All that PostgreSQL knows about is to issue the CREATE
EXTENSION command at pg_restore time.

With templates or in-line extensions, the contract is that the user asks
PostgreSQL to manage its extensions in the same way it does for the
other objects on the system.

Understood.

The design we found to address that is
called "Extension Templates" and is implemented in the current patch.

I placed my concerns with the proposed implementation. It's certainly
not the only way how Postgres can manage its extensions. And I still
hope we can come up with something that's simpler to use and easier to
understand.

However, I'm not a committer nor have I written code for this. I did my
review and proposed two possible (opposite) directions for clean up and
simplification of the design. I would now like others to chime in.

Regards

Markus Wanner

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 08.07.2013 00:48, Markus Wanner wrote:

On 07/07/2013 09:51 PM, Dimitri Fontaine wrote:

The design we found to address that is
called "Extension Templates" and is implemented in the current patch.

I placed my concerns with the proposed implementation. It's certainly
not the only way how Postgres can manage its extensions. And I still
hope we can come up with something that's simpler to use and easier to
understand.

I'm just now dabbling back to this thread after skipping a lot of
discussion, and I'm disappointed to see that this still seems to be
running in circles on the same basic question: What exactly is the patch
trying to accomplish.

The whole point of extensions, as they were originally implemented, is
to allow them to be managed *outside* the database. In particular, they
are not included in pg_dump. If you do want them to be included in
pg_dump, why create it as an extension in the first place? Why not just
run the create script and create the functions, datatypes etc. directly,
like you always did before extensions were even invented.

I think the reason is that extensions provide some handy packaging of
the functions etc, so that you can just do "DROP EXTENSION foo" to get
rid of all of them. Also, pg_extension table keeps track of the
currently installed version. Perhaps we need to step back and invent
another concept that is totally separate from extensions, to provide
those features. Let's call them "modules". A module is like an
extension, in that all the objects in the module can be dropped with a
simple "DROP MODULE foo" command. To create a module, you run "CREATE
MODULE foo AS <SQL script to create the objects in the module>".

I believe that would be pretty much exactly what Dimitri's original
inline extension patches did, except that it's not called an extension,
but a module. I think it's largely been the naming that has been the
problem with this patch from the very beginning. We came up with the
concept of templates after we had decided that the originally proposed
behavior was not what we want from something called extensions. But if
you rewind to the very beginning, the problem was just with the name.
The concept was useful, but not something we want to call an extension,
because the distinguishing feature of an extension is that it lives
outside the database and is *not* included in pg_dump.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

Please find attached to this mail version 9 of the Extension Templates
patch with fixes for the review up to now.

Markus Wanner <markus@bluegap.ch> writes:

I still think that we shouldn't allow creating a template for an
extension that is already installed, though. Do you have any suggestions
for a better error message?

If we go for the template model, I beg to differ. In that mind-set, you
should be free to create (or delete) any kind of template without
affecting pre-existing extensions.

Then what happens at pg_restore time? the CREATE EXTENSION in the backup
script will suddenly install the other extension's that happen to have
the same name? I think erroring out is the only safe way here.

However, in case we follow the ancestor-derivate or link model with the
pg_depend connection, the error message seems fine.

It's all about pg_restore really, but it's easy to forget about that and
get started into other views of the world. I'll try to be better at not
following those tracks and just hammer my answers with "pg_restore" now.

In any case, I'm arguing this "template" renaming behavior (and the
subsequent error) are the wrong thing to do, anyways. Because an
extension being linked to a parent of a different name is weird and IMO
not an acceptable state.

Yes, you're right on spot here. So I've amended the patch to implement
the following behavior (and have added a new regression test):

# CREATE TEMPLATE FOR EXTENSION foo VERSION 'v' AS '';
# CREATE EXTENSION foo;
# ALTER TEMPLATE FOR EXTENSION foo RENAME TO bar;
ERROR: 55006: template for extension "foo" is in use
DETAIL: extension "foo" already exists
LOCATION: AlterExtensionTemplateRename, template.c:1040
STATEMENT: ALTER TEMPLATE FOR EXTENSION foo RENAME TO bar;

I bet that's because people have different mental models in mind. But I
probably stressed that point enough by now...

FWIW, I do agree. But my understanding is that the community consensus
is not going that way.

Specifically, I request to either follow the template model more closely
(accompanied by a separate patch to adjust binary, out-of-line
templates) or follow the link model more closely. The current naming
doesn't match any of the two, so renaming seems inevitable.

I think we need to follow the link model more closely because that's the
consensus, and I will fix all the remaning discrepancies in between the
two models that we can find. Please continue showing them to me!

src/backend/commands/event_trigger.c, definition of
event_trigger_support: several unnecessary whitespaces added. These make
it hard(er) than necessary to review the patch.

Fixed in the attached version 9 of the patch.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

Hello Dimitri,

On 07/08/2013 11:49 AM, Dimitri Fontaine wrote:

Please find attached to this mail version 9 of the Extension Templates
patch with fixes for the review up to now.

Thanks, cool.

Markus Wanner <markus@bluegap.ch> writes:

I still think that we shouldn't allow creating a template for an
extension that is already installed, though. Do you have any suggestions
for a better error message?

If we go for the template model, I beg to differ. In that mind-set, you
should be free to create (or delete) any kind of template without
affecting pre-existing extensions.

Then what happens at pg_restore time? the CREATE EXTENSION in the backup
script will suddenly install the other extension's that happen to have
the same name? I think erroring out is the only safe way here.

Extensions are commonly identified by name (installed ones as well as
available ones, i.e. templates).

Thus I think if a user renames a template, he might have good reasons to
do so. He likely *wants* it to be a template for a different extension.
Likewise when (re-)creating a template with the very same name of a
pre-existing, installed extension.

Maybe the user just wanted to make a "backup" of the template prior to
modifying it. If he then gives the new template the same name as the old
one had, it very likely is similar, compatible or otherwise intended to
replace the former one.

The file-system templates work exactly that way (modulo DSOs). If you
create an extension, then modify (or rename and re-create under the same
name) the template on disk, then dump and restore, you end up with the
new version of it. That's how it worked so far. It's simple to
understand and use.

It's all about pg_restore really, but it's easy to forget about that and
get started into other views of the world. I'll try to be better at not
following those tracks and just hammer my answers with "pg_restore" now.

That's unlikely to be of much help. It's not like pg_restore would stop
to work. It would just work differently. More like the file-system
templates. More like the users already knows and (likely) expects it.
And more like the "template" model that you advertise.

Or how do you think would pg_restore fail, if you followed the mental
model of the template?

In any case, I'm arguing this "template" renaming behavior (and the
subsequent error) are the wrong thing to do, anyways. Because an
extension being linked to a parent of a different name is weird and IMO
not an acceptable state.

Yes, you're right on spot here. So I've amended the patch to implement
the following behavior (and have added a new regression test):

# CREATE TEMPLATE FOR EXTENSION foo VERSION 'v' AS '';
# CREATE EXTENSION foo;
# ALTER TEMPLATE FOR EXTENSION foo RENAME TO bar;
ERROR: 55006: template for extension "foo" is in use
DETAIL: extension "foo" already exists
LOCATION: AlterExtensionTemplateRename, template.c:1040
STATEMENT: ALTER TEMPLATE FOR EXTENSION foo RENAME TO bar;

Okay, good, this prevents the strange state.

However, this also means you restrict the user even further... How can
he save a copy of an existing template, before (re-)creating it with
CREATE TEMPLATE FOR EXTENSION?

On the file system, a simple cp or mv is sufficient before
(re)installing the package from your distribution, for example.

I bet that's because people have different mental models in mind. But I
probably stressed that point enough by now...

FWIW, I do agree.

Good. Why do you continue to propose the link model?

But my understanding is that the community consensus
is not going that way.

What way? And what community consensus?

Re-reading some of the past discussions, I didn't find anybody voting
for a dependency between the template and the created extension. And at
least Tom pretty clearly had the template model in mind, when he wrote
[1]: Tom Lane, Re: in-catalog Extension Scripts and Control parameters (templates?) /messages/by-id/6466.1354817682@sss.pgh.pa.us
to do with altering an extension of the same name (which might or might
not even be installed)." or [2]Tom Lane, Re: Dumping an Extension's Script /messages/by-id/6466.1354817682@sss.pgh.pa.us: "But conflating this functionality
[i.e. extension templates] with installed extensions is just going to
create headaches."

The closest I found was Robert Haas mentioning [3]Robert Haas, Re: Dumping an Extension's Script /messages/by-id/CA+TgmoabcTrThHV5xe8BP_TE+2wqZGNn-8o4kzQSmkL2W9QY8Q@mail.gmail.com, that "[he doesn't]
see a problem having more than one kind of extensions". However, please
mind the context. He doesn't really sound enthusiastic, either.

I'm puzzled about some of your words in that thread. In the very message
Robert responded to, you wrote [4]Dimitri Fontaine: Re: Dumping an Extension's Script /messages/by-id/m2ehj4u7bz.fsf@2ndQuadrant.fr: "Having more than one way to ship an
extension is good, having two different animals with two different
incompatible behaviors named the same thing is bad."

With the link-model, you are now proposing to create exactly that. Two
different kinds of extensions that are not compatible with each other.
One that is independent and one that depends on the "template" it got
created from.

Specifically, I request to either follow the template model more closely
(accompanied by a separate patch to adjust binary, out-of-line
templates) or follow the link model more closely. The current naming
doesn't match any of the two, so renaming seems inevitable.

I think we need to follow the link model more closely because that's the
consensus

If that's really the case - which I doubt at the moment - I'll certainly
accept that. I really recommend to rename the feature (and the
commands), in that case, though. We may rather call the existing
file-system thingie an "extension template", instead, as it becomes a
good differentiator to what you're proposing.

and I will fix all the remaning discrepancies in between the
two models that we can find.

How about ALTER EXTENSION ADD (or DROP)? With the link on the
"template", you'd have to prohibit that ALTER as well, based on the
exact same grounds as the RENAME: The installed extension would
otherwise differ from the "template" it is linked to.

See how this creates an animal pretty different from the current
extensions? And IMO something that's needlessly restricted in many ways.

Please continue showing them to me!

Sure, I'm happy to continue reviewing.

Regards

Markus Wanner

[1]: Tom Lane, Re: in-catalog Extension Scripts and Control parameters (templates?) /messages/by-id/6466.1354817682@sss.pgh.pa.us
(templates?)
/messages/by-id/6466.1354817682@sss.pgh.pa.us

[2]: Tom Lane, Re: Dumping an Extension's Script /messages/by-id/6466.1354817682@sss.pgh.pa.us
/messages/by-id/6466.1354817682@sss.pgh.pa.us

[3]: Robert Haas, Re: Dumping an Extension's Script /messages/by-id/CA+TgmoabcTrThHV5xe8BP_TE+2wqZGNn-8o4kzQSmkL2W9QY8Q@mail.gmail.com
/messages/by-id/CA+TgmoabcTrThHV5xe8BP_TE+2wqZGNn-8o4kzQSmkL2W9QY8Q@mail.gmail.com

[4]: Dimitri Fontaine: Re: Dumping an Extension's Script /messages/by-id/m2ehj4u7bz.fsf@2ndQuadrant.fr
/messages/by-id/m2ehj4u7bz.fsf@2ndQuadrant.fr

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Markus Wanner <markus@bluegap.ch> writes:

Then what happens at pg_restore time? the CREATE EXTENSION in the backup
script will suddenly install the other extension's that happen to have
the same name? I think erroring out is the only safe way here.

Extensions are commonly identified by name (installed ones as well as
available ones, i.e. templates).

Thus I think if a user renames a template, he might have good reasons to
do so. He likely *wants* it to be a template for a different extension.
Likewise when (re-)creating a template with the very same name of a
pre-existing, installed extension.

I can understand that as a incomplete step towards a "migration" of some
sorts, but if we just allow to rename a template we open ourselves to be
producing non-restorable dumps (see below). I'm not at ease with that.

Maybe the user just wanted to make a "backup" of the template prior to
modifying it. If he then gives the new template the same name as the old
one had, it very likely is similar, compatible or otherwise intended to
replace the former one.

The file-system templates work exactly that way (modulo DSOs). If you
create an extension, then modify (or rename and re-create under the same
name) the template on disk, then dump and restore, you end up with the
new version of it. That's how it worked so far. It's simple to
understand and use.

We have absolutely no control over the file-system templates and that's
why they work differently, I think. There's not even the notion of a
transaction over there.

Or how do you think would pg_restore fail, if you followed the mental
model of the template?

# create template for extension foo version 'x' as '';
# create extension foo;
# alter template for extension foo rename to bar;

$ pg_dump | psql

And now it's impossible to CREATE EXTENSION foo, because there's no
source to install it from available. I think we should actively prevent
that scenario to happen in the field (current patch prevents it).

Now, if I'm in the minority, let's just change that.

However, this also means you restrict the user even further... How can
he save a copy of an existing template, before (re-)creating it with
CREATE TEMPLATE FOR EXTENSION?

On the file system, a simple cp or mv is sufficient before
(re)installing the package from your distribution, for example.

Usually what you do when you want to change an extension is that you
provide an upgrade script then run it with ALTER EXTENSION UPDATE.

Sometimes what you do is prepare a new installation script for a new
version of your extension and you don't provide an upgrade script, then
you update with the following method, in the case when you edited the
default_version property of the .control file:

# drop extension foo; -- drops version 1.0
# create extension foo; -- installs version 1.2

The current file system based extensions allow you to maintain
separately the files foo--1.0.sql and foo--1.2.sql, and you don't need
to copy a current version of the whole extension away before hacking the
new version.

The Extension Template facility in the current patch allows you to do
much the same:

# create template for extension foo version '1.0' as $foo$
create function foo() returns int language sql as $$ select 1; $$;
$foo$;

# create template for extension foo version '1.2' as $foo$
create function foo() returns int language sql as $$ select 2; $$;
$foo$;

# select ctlname, ctldefault, ctlversion
from pg_extension_control
where ctlname = 'foo';
ctlname | ctldefault | ctlversion
---------+------------+------------
foo | t | 1.0
foo | f | 1.2
(2 rows)

# create extension foo;
# select foo();
foo
-----
1
(1 row)

And now you can "upgrade" with:

# drop extension foo;
# create extension foo version '1.2';
# select foo();
foo
-----
2
(1 row)

Or even:

# alter template for extension foo set default version '1.2';
# drop extension foo;
# create extension foo;
# select foo();
foo
-----
2
(1 row)

So I don't see that we've broken any use case here, really. I think I
understand your objection in principles, but it appears to me that we
would gain nothing here by allowing broken pg_dump scripts.

What way? And what community consensus?

I see that you've spent extra time and effort to better understand any
community consensus that might exist around this patch series, and I
want to say thank you for that!

Re-reading some of the past discussions, I didn't find anybody voting
for a dependency between the template and the created extension. And at
least Tom pretty clearly had the template model in mind, when he wrote
[1]: "We don't want it to look like manipulating a template has anything
to do with altering an extension of the same name (which might or might
not even be installed)." or [2]: "But conflating this functionality
[i.e. extension templates] with installed extensions is just going to
create headaches."

That was a comment against that proposal:

ALTER EXTENSION … CONFIGURATION FOR 'version' SET param = value, …;
ALTER EXTENSION … SET TEMPLATE FOR 'version' AS $$ … $$;
ALTER EXTENSION … SET TEMPLATE FROM 'version' TO 'version' AS …

So the patch we have now stays away from conflating templates and
extensions. Still, a dependency in between the extension object and the
template it came from is put into place so that we trust the pg_dump
script to contain enough information to be able to actually restore the
extension later. I think it's important to keep that dependency.

Basically what I'm saying in this too long email is that I need other
contributors to voice-in.

The closest I found was Robert Haas mentioning [3], that "[he doesn't]
see a problem having more than one kind of extensions". However, please
mind the context. He doesn't really sound enthusiastic, either.

That was talking about a very different proposal than the one we're
currently analysing. The whole point of the current proposal has been to
avoid having another kind of extensions.

I'm puzzled about some of your words in that thread. In the very message
Robert responded to, you wrote [4]: "Having more than one way to ship an
extension is good, having two different animals with two different
incompatible behaviors named the same thing is bad."

With the link-model, you are now proposing to create exactly that. Two
different kinds of extensions that are not compatible with each other.
One that is independent and one that depends on the "template" it got
created from.

I don't see that, actually. The goal here still is that once installed
the behavior of an extension is always the same. The only difference we
have now is how to guarantee that we can restore a dump:

- using the filesystem based templates, no guarantee at all is offered
by PostgreSQL, that's the developer/sysadmin responsibility, the
dump script has external dependencies that you must manage yourself;

- using the catalog based templates, PostgreSQL guarantees that any
dump you take of the database will be self-contained wrt extensions,
so that you will be able to actually restore it.

Any other discrepency would be qualified as a bug I'm willing to fix.

If that's really the case - which I doubt at the moment - I'll certainly
accept that. I really recommend to rename the feature (and the
commands), in that case, though. We may rather call the existing
file-system thingie an "extension template", instead, as it becomes a
good differentiator to what you're proposing.

Any proposals?

How about ALTER EXTENSION ADD (or DROP)? With the link on the
"template", you'd have to prohibit that ALTER as well, based on the
exact same grounds as the RENAME: The installed extension would
otherwise differ from the "template" it is linked to.

You're "supposed" to be using that from within the template scripts
themselves. The main use case is the upgrade scripts from "unpackaged".

I could see foreclosing that danger by enforcing that creating_extension
is true in those commands.

See how this creates an animal pretty different from the current
extensions? And IMO something that's needlessly restricted in many ways.

Well really I'm not convinced. If you use ALTER EXTENSION ADD against an
extension that you did install from the file system, then you don't know
what will happen after a dump and restore cycle, because you didn't
alter the files to match what you did, presumably.

If you do the same thing against an extension that you did install from
a catalog template, you just managed to open yourself to the same
hazards… I'd be already patching that like proposed above if it was not
for getting some more input and better assess if my understanding
matches that of Tom and Heikki on those fine points.

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

Dimitri,

leaving the template vs link model aside for a moment, here are some
other issues I run into. All under the assumption that we want the link
model.

On 07/08/2013 11:49 AM, Dimitri Fontaine wrote:

Please find attached to this mail version 9 of the Extension Templates
patch with fixes for the review up to now.

First of all, I figured that creation of a template of a newer version
is prohibited in case an extension exists:

db1=# CREATE TEMPLATE FOR EXTENSION foo VERSION '0.0' AS $foo$ SELECT 1; $foo$;
CREATE TEMPLATE FOR EXTENSION
db1=# CREATE EXTENSION foo;
CREATE EXTENSION
db1=# CREATE TEMPLATE FOR EXTENSION foo VERSION '0.1' AS $foo$ SELECT 2; $foo$;
ERROR: extension "foo" already exists

Why is that?

I then came to think of the upgrade scripts... what do we link against
if an extension has been created from some full version and then one or
more upgrade scripts got applied?

Currently, creation of additional upgrade scripts are not blocked. Which
is a good thing, IMO. I don't like the block on newer full versions.
However, the upgrade doesn't seem to change the dependency, so you can
still delete the update script after the update. Consider this:

db1=# CREATE TEMPLATE FOR EXTENSION foo VERSION '0.0' AS $$ $$;
CREATE TEMPLATE FOR EXTENSION
db1=# CREATE EXTENSION foo;
CREATE EXTENSION
db1=# CREATE TEMPLATE FOR EXTENSION foo FROM '0.0' TO '0.1' AS $$ $$;
CREATE TEMPLATE FOR EXTENSION
db1=# ALTER EXTENSION foo UPDATE TO '0.1';
ALTER EXTENSION
db1=# SELECT * FROM pg_extension;
extname | extowner | extnamespace | extrelocatable | extversion | extconfig | extcondition
---------+----------+--------------+----------------+------------+-----------+--------------
plpgsql | 10 | 11 | f | 1.0 | |
foo | 10 | 2200 | f | 0.1 | |
(2 rows)

db1=# DROP TEMPLATE FOR EXTENSION foo FROM '0.0' TO '0.1';
DROP TEMPLATE FOR EXTENSION

db1=# SELECT * FROM pg_extension;
extname | extowner | extnamespace | extrelocatable | extversion | extconfig | extcondition
---------+----------+--------------+----------------+------------+-----------+--------------
plpgsql | 10 | 11 | f | 1.0 | |
foo | 10 | 2200 | f | 0.1 | |
(2 rows)

In this state, extension foo as of version '0.1' is installed, but
running this through dump & restore, you'll only get back '0.0'.

Interestingly, the following "works" (in the sense that the DROP of the
upgrade script is prohibited):

db1=# CREATE TEMPLATE FOR EXTENSION foo VERSION '0.0' AS $$ $$;
CREATE TEMPLATE FOR EXTENSION
db1=# CREATE TEMPLATE FOR EXTENSION foo FROM '0.0' TO '0.1' AS $$ $$;
CREATE TEMPLATE FOR EXTENSION
db1=# CREATE EXTENSION foo VERSION '0.1';
CREATE EXTENSION
db1=# DROP TEMPLATE FOR EXTENSION foo FROM '0.0' TO '0.1';
ERROR: cannot drop update template for extension foo because other objects depend on it
DETAIL: extension foo depends on control template for extension foo
HINT: Use DROP ... CASCADE to drop the dependent objects too.

However, in that case, you are free to drop the full version:

db1=# DROP TEMPLATE FOR EXTENSION foo VERSION '0.0';
DROP TEMPLATE FOR EXTENSION

This certainly creates a bad state that leads to an error, when run
through dump & restore.

Maybe this one...

db1=# CREATE TEMPLATE FOR EXTENSION foo VERSION '0.0' AS $$ $$;
CREATE TEMPLATE FOR EXTENSION
db1=# CREATE TEMPLATE FOR EXTENSION foo FROM '0.0' TO '0.1' AS $$ $$;
CREATE TEMPLATE FOR EXTENSION
db1=# DROP TEMPLATE FOR EXTENSION foo VERSION '0.0';
DROP TEMPLATE FOR EXTENSION

... should already err out here ...

db1=# CREATE EXTENSION foo;
ERROR: Extension "foo" is not available from "/tmp/pginst/usr/local/pgsql/share/extension" nor as a template
db1=# CREATE EXTENSION foo VERSION '0.1';
ERROR: Extension "foo" is not available from "/tmp/pginst/usr/local/pgsql/share/extension" nor as a template

... and not only here.

I.e. the TO version should probably have a dependency on the FROM
version (that might even be useful in the template model).

Another thing that surprised me is the inability to have an upgrade
script *and* a full version (for the same extension target version).
Even if that's intended behavior, the error message could be improved:

db1=# CREATE TEMPLATE FOR EXTENSION foo VERSION '0.0' AS $$ $$;
CREATE TEMPLATE FOR EXTENSION
db1=# CREATE TEMPLATE FOR EXTENSION foo FROM '0.0' TO '0.1' AS $$ $$;
CREATE TEMPLATE FOR EXTENSION
db1=# CREATE TEMPLATE FOR EXTENSION foo VERSION '0.1' AS $$ $$;
ERROR: duplicate key value violates unique constraint "pg_extension_control_name_version_index"
DETAIL: Key (ctlname, ctlversion)=(foo, 0.1) already exists.

Regards

Markus Wanner

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Markus Wanner <markus@bluegap.ch> writes:

First of all, I figured that creation of a template of a newer version
is prohibited in case an extension exists:

Ooops, that's a bug I need to fix.

I then came to think of the upgrade scripts... what do we link against
if an extension has been created from some full version and then one or
more upgrade scripts got applied?

Currently, creation of additional upgrade scripts are not blocked. Which
is a good thing, IMO. I don't like the block on newer full versions.
However, the upgrade doesn't seem to change the dependency, so you can
still delete the update script after the update. Consider this:

We should allow both cases and move the dependency when a script is
installed, and also maintain dependencies in between an upgrade script
and its previous element in the link, either another upgrade script or
the default_full_version you can start with… actually, both.

I.e. the TO version should probably have a dependency on the FROM
version (that might even be useful in the template model).

Agreed.

Another thing that surprised me is the inability to have an upgrade
script *and* a full version (for the same extension target version).
Even if that's intended behavior, the error message could be improved:

Will fix too. Thanks for your extended testing!

Josh, if running out of time on this CF, feel free to mark this one as
"returned with feedback": I will still try to make it under the current
commit fest, but with CHAR(13) starting soon and the current state of
the patch it's clearly reasonnable to say it's not ready yet.

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

Salut Dimitri,

On 07/09/2013 12:40 PM, Dimitri Fontaine wrote:

Markus Wanner <markus@bluegap.ch> writes:

Or how do you think would pg_restore fail, if you followed the mental
model of the template?

# create template for extension foo version 'x' as '';
# create extension foo;
# alter template for extension foo rename to bar;

$ pg_dump | psql

And now it's impossible to CREATE EXTENSION foo, because there's no
source to install it from available. I think we should actively prevent
that scenario to happen in the field (current patch prevents it).

I see. You value that property a lot.

However, I don't think the plain ability to create an extension is quite
enough to ensure a consistent restore, though. You'd also have to ensure
you recreate the very *same* contents of the extension that you dumped.

Your patch doesn't do that. It seems to stop enforcing consistency at
some arbitrary point in between. For example, you can ALTER a function
that's part of the extension. Or ALTER TEMPLATE FOR EXTENSION while an
extension of that version is installed.

Usually what you do when you want to change an extension is that you
provide an upgrade script then run it with ALTER EXTENSION UPDATE.

As a developer, I often happen to work on one and the same version, but
test multiple modifications. This ability to change an extension
"on-the-fly" seems pretty valuable to me.

The current file system based extensions allow you to maintain
separately the files foo--1.0.sql and foo--1.2.sql, and you don't need
to copy a current version of the whole extension away before hacking the
new version.

The Extension Template facility in the current patch allows you to do
much the same

Sure. I'm aware of that ability and appreciate it.

I see that you've spent extra time and effort to better understand any
community consensus that might exist around this patch series, and I
want to say thank you for that!

Thank you for your patience in pursuing this and improving user
experience with extensions. I really appreciate this work.

Basically what I'm saying in this too long email is that I need other
contributors to voice-in.

I fully agree. I don't want to hold this patch back any further, if it's
just me.

I really recommend to rename the feature (and the
commands), in that case, though. We may rather call the existing
file-system thingie an "extension template", instead, as it becomes a
good differentiator to what you're proposing.

Any proposals?

"Inline extension" is a bit contradictory. Maybe "managed extension"
describes best what you're aiming for.

In a similar vein, "out-of-line extension" sounds redundant to me, so
I'd rather characterize the file-system thingie as "extension templates"
wherever a clear distinction between the two is needed.

How about ALTER EXTENSION ADD (or DROP)? With the link on the
"template", you'd have to prohibit that ALTER as well, based on the
exact same grounds as the RENAME: The installed extension would
otherwise differ from the "template" it is linked to.

You're "supposed" to be using that from within the template scripts
themselves. The main use case is the upgrade scripts from "unpackaged".

Agreed. The documentation says it's "mainly useful in extension update
scripts".

I could see foreclosing that danger by enforcing that creating_extension
is true in those commands.

For managed extensions only, right? It's not been prohibited for
extensions so far.

See how this creates an animal pretty different from the current
extensions? And IMO something that's needlessly restricted in many ways.

Well really I'm not convinced. If you use ALTER EXTENSION ADD against an
extension that you did install from the file system, then you don't know
what will happen after a dump and restore cycle, because you didn't
alter the files to match what you did, presumably.

Yeah. The user learned to work according to the template model. Maybe
that was not the best model to start with. And I certainly understand
your desire to ensure a consistent dump & restore cycle. However...

If you do the same thing against an extension that you did install from
a catalog template, you just managed to open yourself to the same
hazards…

... I think the current patch basically just moves the potential
hazards. Maybe these moved hazards are less dramatic and justify the
move. Let's recap:

In either case (template model & link model) the patch:

a) guarantees to restore the template scripts and settings of
all managed extensions

With the link model (and v9 of your patch, which includes the RENAME
fix, and pretending there are no other bugs):

b) it guarantees *some* revision of an extension version (identified
by name and version) that has been created at dump time can be
restored from a template from the dump

If you'd additionally restrict ALTER EXTENSION ADD/DROP as well as ALTER
TEMPLATE FOR EXTENSION AS ..., you'd also get:

c) it guarantees the set of objects created by managed extensions
is restored to exactly the same set as the one that got dumped

So far, neither template nor link model guarantee:

d) the contents (functions, types, operators, ...) of the actual
extension installed matches the contents before the dump

In the template model, you'd get the following benefit:

e) templates (esp. upgrade scripts) can be used in a mix and match
fashion, it doesn't matter whether in-line or out-of-line was
used, you can always apply update scripts from any of the two.

Given d), I personally don't value b) and c) all that much and rather
accuse them of pretending a bit of a false safety.

I'm pretty keen about a) and really appreciate that part of Dimitri's
patch. I see great value in e) as well.

What do others think?

Regards

Markus Wanner

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

Please find attached a new version (v10) of the patch that fixes the
reported dependencies problems and add some new regression tests to
cover them.

The patch implements the solution we discuted privately with Markus
while at the CHAR(13) conference:

- create template for extension is now possible even if an extension
is already installed, so that you can install a template for a new
version of the extension;

- all the scripts used to install an extension are now set as
dependencies so that you can't drop parts of what you need at
restore time;

- you can create extension for template x version 'y' when you already
had an upgrade path leading to that same version 'y', but only if
your set of parameters for the version 'y' remains the same as
what's already installed in the auxilliary control entry;

- fix a pg_dump bug by using special dollar quoting $extname$.

Markus Wanner <markus@bluegap.ch> writes:

db1=# CREATE TEMPLATE FOR EXTENSION foo VERSION '0.1' AS $foo$ SELECT 2; $foo$;
ERROR: extension "foo" already exists

Fixed in the attached.

db1=# DROP TEMPLATE FOR EXTENSION foo FROM '0.0' TO '0.1';
DROP TEMPLATE FOR EXTENSION

In this state, extension foo as of version '0.1' is installed, but
running this through dump & restore, you'll only get back '0.0'.

Fixed in the attached.

This certainly creates a bad state that leads to an error, when run
through dump & restore.

Fixed in the attached.

db1=# DROP TEMPLATE FOR EXTENSION foo VERSION '0.0';
DROP TEMPLATE FOR EXTENSION

... should already err out here ...

Fixed in the attached.

Another thing that surprised me is the inability to have an upgrade
script *and* a full version (for the same extension target version).
Even if that's intended behavior, the error message could be improved:

Fixed in the attached by allowing both to co-exist.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support