Thread: Review: extension template
Hi, I reviewed Dimitri's work on extension templates [2]. There's some discussion still ongoing and the patch has gone through several revisions since its addition to the current CF. The patch has already been marked as 'returned with feedback', and I can support that resolution (for this CF). I still see value in the following review. Initially, I was confused about what the patch is supposed to achieve. The 'template' naming certainly contributed to that confusion. My mental model of a template is something that I can throw away after its use. (I note the text search "templates" don't follow that model, either). Where as up to now, extensions were something that I install (system wide) and then enable per database (by CREATE, which can be thought of as a misnomer as well). Of course you can think of such a system wide installation as a template for the creation (an instantiation per database). However, we didn't ever call the system wide installations templates. Nor does the patch start to adapt that naming scheme. 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... Dimitri responded promptly to a request to rebase the patch. Version 8 still applies cleanly to git master (as of Jul 5, 9ce9d). The patch matches git revision c0c507022ec912854e6658c5a10a3dedb1c36d67 of dim's github branch 'tmpl4' [2]. That's what I tested with. The base of the patched tree builds just fine (i.e. plain 'make'), although the compiler rightfully warns about an 'evi' variable being set but not used. extension.c, line 1170. Jaime mentioned that already. 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’ The build passes 'make check'. Additional tests are provided. Good. 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. 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". 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... 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. 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? 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. 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". ] 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. 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. I didn't look too deep into the code, but it seems Jaime and Hitoshi raised some valid points. Assorted very minor nit-picks: 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. 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. Regards Markus Wanner [1]: CF: Extension Templates https://commitfest.postgresql.org/action/patch_view?id=1032 [2]: Dimitri's github branch 'tmpl4': https://github.com/dimitri/postgres/tree/tmpl4
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
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
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
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
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
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
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
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
On 07/08/2013 09:26 AM, Heikki Linnakangas wrote: > 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. Either MODULE or PACKAGE would be good name candidates. Still, getting this functionality in seems more important than exact naming, though naming them "right" would be nice. > > - Heikki > >
> On 07/08/2013 09:26 AM, Heikki Linnakangas wrote: >> 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. Bypassing the file system entirely in order to install an extension. As soon as I figure out how to, including C-coded extensions. >> 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 The main feature of the extensions system is its ability to have a clean pg_restore process even when you use some extensions. That has been the only goal of the whole feature development. Let me stress that the most important value in that behavior is to be able to pg_restore using a newer version of the extension, the one that works with the target major version. When upgrading from 9.2 to 9.3 if you depend on keywords that now are reserved you need to install the newer version of the extension at pg_restore time. The main features I'm interested into beside a clean pg_restore are UPDATE scripts for extensions and dependency management, even if that still needs improvements. Those improvements will be relevant for both ways to make extensions available for your system. >> 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>". Not again the naming. A module is already documented as a shared object library (.so, .dll or .dylib) that PostgreSQL will LOAD for you. A patch has already been proposed to track which module is loaded in a session and offer that in a new system's view, pg_module. We can not use the name "module" for anything else, IMNSHO. >> 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. The main goal here is not to have the extension live inside the database but rather to be able to bypass using the server's filesystem in order to be able to CREATE EXTENSION foo; and then to still have pg_restore do the right thing on its own. If you want to scratch the new catalogs part, then just say that it's expected to be really complex to pg_restore a database using extensions, back to exactly how it was before 9.1: create the new database, create the extensions your dump depends on in that new database, now pg_restore your backup manually filtering away the extensions' objects or ignoring the errors when pg_restore tries to duplicate functions you already installed in the previous step. No fun. Hannu Krosing <hannu@krosing.net> writes: > Either MODULE or PACKAGE would be good name candidates. The name "package" is even worse than the "module" one because lots of people think they know exactly what is a package for having been using a closed source product that you might have heard of: they are trying to cope with our ability to implement new features on a yearly basis while not breaking anything we already have. > Still, getting this functionality in seems more important than exact > naming, though naming them "right" would be nice. Of course we want to do it right™. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On 06/10/2013 09:43 PM, Hannu Krosing wrote: > On 07/08/2013 09:26 AM, Heikki Linnakangas wrote: >> 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. > Either MODULE or PACKAGE would be good name candidates. > > Still, getting this functionality in seems more important than exact > naming, though naming them "right" would be nice. Remember that we already have quite a lot of extensions. And PGXN. Are we really so wedded to the idea of extensions "living" outside of the database that we need to come up with something different and incompatible? Or do you envision modules or packages to be compatible with extensions? Just putting another label on it so we can still claim extensions are strictly external to the database? Sorry, I don't get the idea, there. From a users perspective, I want extensions, modules, or packages to be managed somehow. Including upgrades, migrations (i.e. dump & restore) and removal. The approach of letting the distributors handle that packaging clearly has its limitations. What's so terribly wrong with Postgres itself providing better tools to manage those? Inventing yet another type of extension, module or package (compatible or not) doesn't help, but increases confusion even further. Or how do you explain to an author of an existing extension, whether or not he should convert his extension to a module (if you want those to be incompatible)? If it's the same thing, just with different loading mechanisms, please keep calling it the same: an extension. (And maintain compatibility between the different ways to load it.) I fully agree with the fundamental direction of Dimitri's patch. I think Postgres needs to better manage its extensions itself. Including dump and restore cycles. However, I think the implementation isn't optimal, yet. I pointed out a few usability issues and gave reasons why "template" is a misnomer (with the proposed implementation). "Extension" is not. (I still think "template" would be a good mental model. See my other thread... http://archives.postgresql.org/message-id/51D72C1D.7010609@bluegap.ch) Regards Markus Wanner
On 07/08/2013 10:20 AM, Dimitri Fontaine wrote: > Bypassing the file system entirely in order to install an extension. As > soon as I figure out how to, including C-coded extensions. Do I understand correctly that you want to keep the extensions (or their templates) out of the dump and require the user to "upload" it via libpq prior to the restor; instead of him having to install them via .deb or .rpm? This would explain why you keep the CREATE TEMPLATE FOR EXTENSION as a separate step from CREATE EXTENSION. And why you, too, insist on wanting templates, and not just a way to create an extension via libpq. However, why don't you follow the template model more closely? Why should the user be unable to create a template, if there already exists an extension of the same name? That's an unneeded and disturbing limitation, IMO. My wish: Please drop the pg_depend link between template and extension and make the templates shared across databases. So I also have to install the template only once per cluster. Keep calling them templates, then. (However, mind that file-system extension templates are templates as well. In-line vs. out-of-line templates, if you want.) I think you could then safely allow an upgrade of an extension that has been created from an out-of-line template by an upgrade script that lives in-line. And vice-versa. Just as an example. It all gets nicer and cleaner, if the in-line thing better matches the out-of-line one, IMO. An extension should look and behave exactly the same, independent of what kind of template it has been created from. And as we obviously cannot add a pg_depend link to a file on the file system, we better don't do that for the in-line variant, either, to maintain the symmetry. > The main feature of the extensions system is its ability to have a clean > pg_restore process even when you use some extensions. That has been the > only goal of the whole feature development. Great! Very much appreciated. > Let me stress that the most important value in that behavior is to be > able to pg_restore using a newer version of the extension, the one that > works with the target major version. When upgrading from 9.2 to 9.3 if > you depend on keywords that now are reserved you need to install the > newer version of the extension at pg_restore time. > > The main features I'm interested into beside a clean pg_restore are > UPDATE scripts for extensions and dependency management, even if that > still needs improvements. Those improvements will be relevant for both > ways to make extensions available for your system. +1 > We can not use the name "module" for anything else, IMNSHO. Agreed. > The main goal here is not to have the extension live inside the database > but rather to be able to bypass using the server's filesystem in order > to be able to CREATE EXTENSION foo; and then to still have pg_restore do > the right thing on its own. Note that with the current, out-of-line approach, the *extension* already lives inside the database. It's just the *template*, that doesn't. (Modulo DSO, but the patch doesn't handle those either, yet. So we're still kind of excluding those.) Allowing for templates to live inside the database as well is a good thing, IMO. > If you want to scratch the new catalogs part, then just say that it's > expected to be really complex to pg_restore a database using extensions, > back to exactly how it was before 9.1: create the new database, create > the extensions your dump depends on in that new database, now pg_restore > your backup manually filtering away the extensions' objects or ignoring > the errors when pg_restore tries to duplicate functions you already > installed in the previous step. No fun. Definitely not. Nobody wants to go back there. (And as Heikki pointed out, if you absolutely want to, you can even punish yourself that way.) Regards Markus Wanner
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
Attachment
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]: "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." 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. 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. >> 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?) http://www.postgresql.org/message-id/6466.1354817682@sss.pgh.pa.us [2]: Tom Lane, Re: Dumping an Extension's Script http://www.postgresql.org/message-id/6466.1354817682@sss.pgh.pa.us [3]: Robert Haas, Re: Dumping an Extension's Script http://www.postgresql.org/message-id/CA+TgmoabcTrThHV5xe8BP_TE+2wqZGNn-8o4kzQSmkL2W9QY8Q@mail.gmail.com [4]: Dimitri Fontaine: Re: Dumping an Extension's Script http://www.postgresql.org/message-id/m2ehj4u7bz.fsf@2ndQuadrant.fr
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 renameto 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 $$ select1; $$; $foo$; # create template for extension foo version '1.2' as $foo$ create function foo() returns int language sql as $$ select2; $$; $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-containedwrt 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
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
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
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 dumptime 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 gotdumped 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-linewas 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
On 7/8/13 4:20 AM, Dimitri Fontaine wrote: > Let me stress that the most important value in that behavior is to be > able to pg_restore using a newer version of the extension, the one that > works with the target major version. When upgrading from 9.2 to 9.3 if > you depend on keywords that now are reserved you need to install the > newer version of the extension at pg_restore time. I think there is an intrinsic conflict here. You have things inside the database and outside. When they depend on each other, it gets tricky. Extensions were invented to copy with that. They do the job, more or less. Now you want to take the same mechanism and apply it entirely inside the database. But that wasn't the point of extensions! That's how you get definitional issues like, should extensions be dumped or not. I don't believe the above use case. (Even if I did, it's marginal.) You should always be able to arrange things so that an upgrade of an inside-the-database-package is possible before or after pg_restore. pg_dump and pg_restore are interfaces between the database and the outside. They should have nothing to do with upgrading things that live entirely inside the database. There would be value to inside-the-database package management, but it should be a separate concept.
Peter, On 07/09/2013 11:04 PM, Peter Eisentraut wrote: > I think there is an intrinsic conflict here. You have things inside the > database and outside. When they depend on each other, it gets tricky. > Extensions were invented to copy with that. They do the job, more or > less. I agree. And to extend upon that, I think it's important to distinguish between the created extension and the available one, i.e. the template. Only the template lives outside. The created extension itself is firmly sitting in the database, possibly with multiple dependencies from other objects. It does not dependent on anything outside of the database (assuming the absence of a DSO of the extension, which does not follow that template concept). And yes, we decided the objects that are part of the extension should not get dumped with pg_dump. Nobody argues to change that. Note, however, that this very decision is what raises the "intrinsic conflict" for pg_restore, because CREATE EXTENSION in the dump depends on the outside extension. If anything, Dimitri's patch solves that. > Now you want to take the same mechanism and apply it entirely > inside the database. But that wasn't the point of extensions! That's > how you get definitional issues like, should extensions be dumped or not. IMO the point of extensions is to extend Postgres (with code that's not part of core). Whether their templates (SQL sources, if you want) are stored on the file system (outside) or within the database is irrelevant to the concept. Think of it that way: Take one of those FUSE-Postgres-FS things [1], which uses Postgres as the backend for a file system and allows you to store arbitrary files. Mount that to the extensions directory of your Postgres instance and make your extension templates available there (i.e. copy them there). CREATE EXTENSION would just work, reading the templates for the extension to create from itself, via that fuse wrapper. (If the FUSE wrapper itself was using an extension, you'd get into an interesting chicken and egg problem, but even that would be resolvable, because the installed extension doesn't depend on the template it was created from.) Think of Dimitri's patch as a simpler and more elegant way to achieve the very same thing. (Well, modulo our disagreement about the dependency between extension and templates.) And as opposed to the file system or fuse approach, you'd even gain transactional safety, consistency (i.e. a constraint can enforce a full version exists as the basis for an upgrade script), etc... But who am I to tell you the benefits of storing data in a database? Of course, you then also want to be able to backup your templates (not the extensions) stored in the database. Just like you keep a backup of your file-system templates. Either by simply making a copy, or maybe by keeping an RPM or DEB package of it available. Thus, of course, templates for extensions need to be dumped as well. > I don't believe the above use case. (Even if I did, it's marginal.) > You should always be able to arrange things so that an upgrade of an > inside-the-database-package is possible before or after pg_restore. Dimitri's scenario assumes an old and a new version of an extension as well as an old and a new Postgres major version. Where the old extension is not compatible with the new Postgres major version. Which certainly seems like a plausible scenario to me (postgis-2.0 is not compatible with Postgres-9.3, for example - granted, it carries a DSO, so it's not really a good example). Given how extensions work, to upgrade to the new Postgres major version *and* to the required new version of the extension, you don't ever need to "upgrade an inside-the-database-package". Instead, you need to: createdb -> provide templates -> CREATE EXTENSION -> restore data Now, CREATE EXTENSION and restoring your data has effectively been merged for the user, as pg_dump emits proper CREATE EXTENSION commands. "Providing templates" so far meant installing an RPM or DEB. Or copying the files manually. But in fact, how and where you provide templates for the extension is irrelevant to that order. And the possibility to merge the second step into the 'restore data' step certainly sounds appealing to me. > pg_dump and pg_restore are interfaces between the database and the > outside. They should have nothing to do with upgrading things that live > entirely inside the database. I don't get your point here. In my view, libpq is intended to modify the things that live inside the database, including extensions (i.e. ALTER EXTENSION ADD FUNCTION). Or what kind of "things that live entirely inside the database" do you have in mind. > There would be value to inside-the-database package management, but it > should be a separate concept. Anything that's incompatible to extensions is not gonna fly. There are too many of them available, already. We need to ease management of those, not come up with yet another concept. Regards Markus Wanner [1]: for example, pgfuse: database in user-space filesystem accessing a Postgresql database: https://github.com/andreasbaumann/pgfuse
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