It just occurred to me that the extensions patch thoroughly breaks
pg_upgrade, because pg_upgrade imagines that it can control the specific
OIDs assigned to certain SQL objects such as data types. That is of
course not gonna happen for objects within an extension. In fact, since
part of the point of an extension is that the set of objects it packages
might change across versions, it wouldn't be sensible to try to force
OID assignments even if there were a practical way to do it.

Offhand the only escape route that I can see is for a binary upgrade to
not try to install a newer version of an extension, but just duplicate
all the contained objects exactly. Which is probably less than trivial
to do --- we could get part of the way there by having pg_dump not
suppress the member objects in --binary-upgrade mode, but then we need
some awful kluge to re-establish their pg_depend links to the extension.
In any case that would ratchet the priority of ALTER EXTENSION UPGRADE
back up to a must-have-for-9.1, since pg_upgrade would then leave you
with a non-upgraded extension.

Now what?

regards, tom lane

Search Discussions

  • Dimitri Fontaine at Feb 8, 2011 at 5:09 pm

    Tom Lane writes:
    In any case that would ratchet the priority of ALTER EXTENSION UPGRADE
    back up to a must-have-for-9.1, since pg_upgrade would then leave you
    with a non-upgraded extension.

    Now what?
    What would be the problem with pg_upgrade acting the same as a
    dump&reload cycle as far as extensions are concerned? After all those
    can be considered as part of the schema, not part of the data, and the
    system catalogs are upgraded by the tool.

    It would then only break user objects that depend on the extension's
    objects OIDs, but that would be the same if they instead recorded the
    OID of catalog entries, right?

    So a valid answer for me would be that when you pg_upgrade, the
    extensions are installed again from their scripts. If you want to go
    further than that, you can insist on having the same version of the
    extension on both sides, but that would defeat the purpose of the tool
    somehow. After all you asked for an upgrade…

    Regards,
    --
    Dimitri Fontaine
    http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
  • Tom Lane at Feb 8, 2011 at 5:30 pm

    Dimitri Fontaine writes:
    Tom Lane <tgl@sss.pgh.pa.us> writes:
    Now what?
    What would be the problem with pg_upgrade acting the same as a
    dump&reload cycle as far as extensions are concerned?
    Um, how about "it doesn't work"?

    The reason that data type OIDs have to be preserved, for example, is
    that they might be out on disk. Suppose that you have the cube
    extension installed and there are some user tables containing columns of
    type cube[]. Those arrays are going to have type cube's OID embedded in
    them. If cube has a different OID after pg_upgrade then the arrays are
    broken.

    Even letting an extension script run and create data types that weren't
    there at all before is problematic, since those types could easily claim
    OIDs that need to be reserved for pre-existing types that appear later
    in the dump script.

    Similar issues arise for the other cases where pg_upgrade is forcing OID
    assignments; it's not doing that just for fun.

    regards, tom lane
  • Robert Haas at Feb 8, 2011 at 5:37 pm

    On Tue, Feb 8, 2011 at 11:53 AM, Tom Lane wrote:
    It just occurred to me that the extensions patch thoroughly breaks
    pg_upgrade, because pg_upgrade imagines that it can control the specific
    OIDs assigned to certain SQL objects such as data types.  That is of
    course not gonna happen for objects within an extension.  In fact, since
    part of the point of an extension is that the set of objects it packages
    might change across versions, it wouldn't be sensible to try to force
    OID assignments even if there were a practical way to do it.

    Offhand the only escape route that I can see is for a binary upgrade to
    not try to install a newer version of an extension, but just duplicate
    all the contained objects exactly.  Which is probably less than trivial
    to do --- we could get part of the way there by having pg_dump not
    suppress the member objects in --binary-upgrade mode, but then we need
    some awful kluge to re-establish their pg_depend links to the extension.
    In any case that would ratchet the priority of ALTER EXTENSION UPGRADE
    back up to a must-have-for-9.1, since pg_upgrade would then leave you
    with a non-upgraded extension.
    If we're going to commit any part of this for 9.1, it doesn't seem
    like there's much choice but to insert the above-mentioned awful
    kludge. It's certainly not going to work if we fail to preserve the
    OIDs in cases where pg_upgrade otherwise thinks it's necessary.

    I guess I'm sort of coming to the conclusion that ALTER EXTENSION ..
    UPGRADE is pretty much a must-have for a useful feature regardless of
    this issue. I had previously thought that we might be able to limp
    along with half a feature for one release - if you're not actually
    depending on anything in the extension, you could always drop it and
    the install the new version. But the more I think about it, the less
    realistic that sounds; dependencies on the extension are going to be
    very common, and while it may be practical to drop and recreate the
    dependent objects in some cases, it's certainly going to annoy a lot
    of people.

    So I think the choices, realistically, are boot both halves of this to
    9.2, or do it all now. I don't really have an opinion on which way to
    go with it. We still have more than 40 patches to deal with for this
    CommitFest, and certainly booting this would free up a bunch of your
    time to go work on other things. On the other hand, if you are fired
    up about getting this done, maybe it makes sense to get it knocked out
    while it's fresh in your mind instead of letting it linger for another
    six months. It's clearly a good feature, and I know that Dimitri has
    put a lot of work into getting it this far.

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
  • David E. Wheeler at Feb 8, 2011 at 5:58 pm

    On Feb 8, 2011, at 9:36 AM, Robert Haas wrote:

    I guess I'm sort of coming to the conclusion that ALTER EXTENSION ..
    UPGRADE is pretty much a must-have for a useful feature regardless of
    this issue. I had previously thought that we might be able to limp
    along with half a feature for one release - if you're not actually
    depending on anything in the extension, you could always drop it and
    the install the new version. But the more I think about it, the less
    realistic that sounds; dependencies on the extension are going to be
    very common, and while it may be practical to drop and recreate the
    dependent objects in some cases, it's certainly going to annoy a lot
    of people.
    I was just thinking about the upgrade issue, and was wondering if this was do-able:

    * No upgrade scripts. Nothing to concatenate, include, or maintain.
    * No version tracking

    Yeah, what I'm saying is, throw out the whole thing. Instead, what would happen is

    ALTER EXTENSION foo UPGRADE;

    Would do some trickery behind the scenes to just delete all those objects that are part of the extension (just like DROP EXTENSION should do), and then run the installation SQL script. So the objects would be exactly as specified in the installation script, with no need for the extension maintainer to worry about how to run upgrades, and no need for PostgreSQL to track version numbers.

    Is this do-able? I recognize that there could be some issues with settings tables and what-not, but surely that's no different than for dump and reload. The question in my mind is whether it would even be possible for the extension code to unload every bit of an extension and load the new stuff, inside a transaction.

    Thoughts?

    Best,

    David
  • Robert Haas at Feb 8, 2011 at 6:03 pm

    On Tue, Feb 8, 2011 at 12:57 PM, David E. Wheeler wrote:
    On Feb 8, 2011, at 9:36 AM, Robert Haas wrote:

    I guess I'm sort of coming to the conclusion that ALTER EXTENSION ..
    UPGRADE is pretty much a must-have for a useful feature regardless of
    this issue.  I had previously thought that we might be able to limp
    along with half a feature for one release - if you're not actually
    depending on anything in the extension, you could always drop it and
    the install the new version.  But the more I think about it, the less
    realistic that sounds; dependencies on the extension are going to be
    very common, and while it may be practical to drop and recreate the
    dependent objects in some cases, it's certainly going to annoy a lot
    of people.
    I was just thinking about the upgrade issue, and was wondering if this was do-able:

    * No upgrade scripts. Nothing to concatenate, include, or maintain.
    * No version tracking

    Yeah, what I'm saying is, throw out the whole thing. Instead, what would happen is

    ALTER EXTENSION foo UPGRADE;

    Would do some trickery behind the scenes to just delete all those objects that are part of the extension (just like DROP EXTENSION should do), and then run the installation SQL script. So the objects would be exactly as specified in the installation script, with no need for the extension maintainer to worry about how to run upgrades, and no need for PostgreSQL to track version numbers.

    Is this do-able? I recognize that there could be some issues with settings tables and what-not, but surely that's no different than for dump and reload. The question in my mind is whether it would even be possible for the extension code to unload every bit of an extension and load the new stuff, inside a transaction.
    No, this is not doable, or at least not in a way that provides any
    benefit over just dropping and reinstalling. The problem is that it
    is going to fall down all over the place if other objects are
    depending on objects provided by the extension. Like:

    CREATE VIEW v AS SELECT extensionfunc(1);

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
  • Tom Lane at Feb 8, 2011 at 6:06 pm

    Robert Haas writes:
    I guess I'm sort of coming to the conclusion that ALTER EXTENSION ..
    UPGRADE is pretty much a must-have for a useful feature regardless of
    this issue. I had previously thought that we might be able to limp
    along with half a feature for one release - if you're not actually
    depending on anything in the extension, you could always drop it and
    the install the new version. But the more I think about it, the less
    realistic that sounds; dependencies on the extension are going to be
    very common, and while it may be practical to drop and recreate the
    dependent objects in some cases, it's certainly going to annoy a lot
    of people.
    So I think the choices, realistically, are boot both halves of this to
    9.2, or do it all now. I don't really have an opinion on which way to
    go with it. We still have more than 40 patches to deal with for this
    CommitFest, and certainly booting this would free up a bunch of your
    time to go work on other things. On the other hand, if you are fired
    up about getting this done, maybe it makes sense to get it knocked out
    while it's fresh in your mind instead of letting it linger for another
    six months. It's clearly a good feature, and I know that Dimitri has
    put a lot of work into getting it this far.
    Personally I find the extension stuff to be a bigger deal than anything
    else remaining in the commitfest. Also, I've fixed a number of
    pre-existing bugs in passing, and I'd have to go extract those changes
    out of the current extensions patch if we abandon it now. So I'd like
    to keep pushing ahead on this.

    After further reflection I think that the pg_upgrade situation can be
    handled like this:

    1. Provide a way for CREATE EXTENSION to not run the script --- either
    add a grammar option for that, or just have a back-door flag that
    pg_upgrade can set via one of its special kluge functions. (Hm,
    actually we'd want it to install the old version number and comment
    too, so maybe just have a kluge function to create a pg_extension
    entry that agrees with the old entry.)

    2. Invent a command "ALTER EXTENSION extname ADD object-description"
    that simply adds a pg_depend entry marking an existing object as
    belonging to the extension. I think this was going to be part of the
    plan anyway for ALTER EXTENSION UPGRADE, specifically we need that for
    the bootstrap case of collecting some loose pre-existing objects into
    an extension.

    3. In --binary-upgrade mode, pg_dump doesn't suppress the individual
    member objects, but instead emits ALTER EXTENSION ADD after creating
    each member object.

    So that gets us to the point of being able to run pg_upgrade without
    changing the contents of the extension. After that we can look at
    ALTER EXTENSION UPGRADE.

    regards, tom lane
  • David E. Wheeler at Feb 8, 2011 at 6:19 pm

    On Feb 8, 2011, at 10:03 AM, Robert Haas wrote:

    No, this is not doable, or at least not in a way that provides any
    benefit over just dropping and reinstalling. The problem is that it
    is going to fall down all over the place if other objects are
    depending on objects provided by the extension. Like:

    CREATE VIEW v AS SELECT extensionfunc(1);
    Ah, right, of course. I don't suppose CREATE OR REPLACE would work, either, in some cases at least?

    Best,

    David
  • Tom Lane at Feb 8, 2011 at 6:24 pm

    "David E. Wheeler" <david@kineticode.com> writes:
    On Feb 8, 2011, at 10:03 AM, Robert Haas wrote:
    No, this is not doable, or at least not in a way that provides any
    benefit over just dropping and reinstalling. The problem is that it
    is going to fall down all over the place if other objects are
    depending on objects provided by the extension. Like:

    CREATE VIEW v AS SELECT extensionfunc(1);
    Ah, right, of course. I don't suppose CREATE OR REPLACE would work, either, in some cases at least?
    But figuring out just what commands to issue is pretty nearly AI-complete.
    The whole point of ALTER EXTENSION UPGRADE is to have a human do that
    and then give you a script to run.

    regards, tom lane
  • David E. Wheeler at Feb 8, 2011 at 6:27 pm

    On Feb 8, 2011, at 10:24 AM, Tom Lane wrote:

    Ah, right, of course. I don't suppose CREATE OR REPLACE would work, either, in some cases at least?
    But figuring out just what commands to issue is pretty nearly AI-complete.
    The whole point of ALTER EXTENSION UPGRADE is to have a human do that
    and then give you a script to run.
    Damn. Okay, was just trying to think outside the box a bit here.

    Best,

    David
  • Dimitri Fontaine at Feb 8, 2011 at 8:44 pm

    Tom Lane writes:
    that they might be out on disk. Suppose that you have the cube
    extension installed and there are some user tables containing columns of
    type cube[]. Those arrays are going to have type cube's OID embedded in
    them. If cube has a different OID after pg_upgrade then the arrays are
    broken.
    Forgot about the internals of arrays. Sure, that's a problem here.
    Even letting an extension script run and create data types that weren't
    there at all before is problematic, since those types could easily claim
    OIDs that need to be reserved for pre-existing types that appear later
    in the dump script.

    Similar issues arise for the other cases where pg_upgrade is forcing OID
    assignments; it's not doing that just for fun.
    There's provision to forcing the OID of an extension at CREATE EXTENSION
    time in the UPGRADE patch, but it does not handle the extension
    objects.

    I though system OIDs and user OIDs can't clash because of a 16384
    counter magic assignment at initdb, so I'm having a hard time getting to
    understand exactly the problem case. Will spend time on it tomorrow, if
    that still helps.

    Regards,
    --
    Dimitri Fontaine
    http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
  • Dimitri Fontaine at Feb 8, 2011 at 8:53 pm

    Tom Lane writes:
    Personally I find the extension stuff to be a bigger deal than anything
    else remaining in the commitfest. Also, I've fixed a number of
    pre-existing bugs in passing, and I'd have to go extract those changes
    out of the current extensions patch if we abandon it now. So I'd like
    to keep pushing ahead on this.
    Stealing words from Kevin, WOO-HOO :) I'll try to continue devote as
    much time as I did already to assist as much as I can here.
    After further reflection I think that the pg_upgrade situation can be
    handled like this:

    1. Provide a way for CREATE EXTENSION to not run the script --- either
    add a grammar option for that, or just have a back-door flag that
    pg_upgrade can set via one of its special kluge functions. (Hm,
    actually we'd want it to install the old version number and comment
    too, so maybe just have a kluge function to create a pg_extension
    entry that agrees with the old entry.)
    In the upgrade patch there's provision for not running the script:

    CREATE WRAPPER EXTENSION foo;

    That's pretty useful indeed. What it does now is read the control file
    to init the comment and other fields, but extversion is forced NULL.
    That allows to have special rules in how UPGRADE will pick a script.

    There's even code to do

    CREATE WRAPPER EXTENSION foo WITH SYSID 12345;

    We could add version and comment here for purposes of pg_upgrade.
    2. Invent a command "ALTER EXTENSION extname ADD object-description"
    that simply adds a pg_depend entry marking an existing object as
    belonging to the extension. I think this was going to be part of the
    plan anyway for ALTER EXTENSION UPGRADE, specifically we need that for
    the bootstrap case of collecting some loose pre-existing objects into
    an extension.
    In the upgrade patch, that's spelled ALTER OBJECT foo SET EXTENSION bar;
    and does exactly what you're proposing.
    3. In --binary-upgrade mode, pg_dump doesn't suppress the individual
    member objects, but instead emits ALTER EXTENSION ADD after creating
    each member object.

    So that gets us to the point of being able to run pg_upgrade without
    changing the contents of the extension. After that we can look at
    ALTER EXTENSION UPGRADE.
    Well, or begin there as the code you need is already written.

    Regards,
    --
    Dimitri Fontaine
    http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
  • Tom Lane at Feb 8, 2011 at 9:54 pm

    Dimitri Fontaine writes:
    Tom Lane <tgl@sss.pgh.pa.us> writes:
    2. Invent a command "ALTER EXTENSION extname ADD object-description"
    that simply adds a pg_depend entry marking an existing object as
    belonging to the extension. I think this was going to be part of the
    plan anyway for ALTER EXTENSION UPGRADE, specifically we need that for
    the bootstrap case of collecting some loose pre-existing objects into
    an extension.
    In the upgrade patch, that's spelled ALTER OBJECT foo SET EXTENSION bar;
    and does exactly what you're proposing.
    OK, that seems like an equally reasonable syntax; although doing it the
    way I was thinking might have less of a code and documentation footprint
    (I was vaguely imagining that it could share most of the COMMENT
    infrastructure --- but haven't looked yet). In any case it seems like
    this is a good piece to do next, since the required functionality is
    clear and it's essential for more than one reason.

    regards, tom lane
  • Dimitri Fontaine at Feb 8, 2011 at 10:07 pm

    Tom Lane writes:
    OK, that seems like an equally reasonable syntax; although doing it the
    way I was thinking might have less of a code and documentation footprint
    (I was vaguely imagining that it could share most of the COMMENT
    infrastructure --- but haven't looked yet). In any case it seems like
    this is a good piece to do next, since the required functionality is
    clear and it's essential for more than one reason.
    Well the code footprint is quite small already. When the grammar change
    and the alter.c addition is in, it boils down to:

    /*
    * ALTER OBJECT SET EXTENSION
    *
    * This form allows for upgrading from previous PostgreSQL version to one
    * supporting extensions, or to upgrade user objects to get to use the
    * extension infrastructure.
    *
    * All we have to do is record an INTERNAL dependency between the selected
    * object and the extension, which must of course already exist.
    */
    void
    AlterObjectExtension(const char *extname,
    Oid classId, Oid objectId, Oid objectSubId)
    {
    ObjectAddress *extension, *object;

    if (!superuser())
    ereport(ERROR,
    (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
    (errmsg("must be superuser to SET EXTENSION"))));

    extension = (ObjectAddress *)palloc(sizeof(ObjectAddress));
    extension->classId = ExtensionRelationId;
    extension->objectId = get_extension_oid(extname, false);
    extension->objectSubId = 0;

    object = (ObjectAddress *)palloc(sizeof(ObjectAddress));
    object->classId = classId;
    object->objectId = objectId;
    object->objectSubId = objectSubId;

    recordDependencyOn(object, extension, DEPENDENCY_INTERNAL);

    return;
    }

    Of course it needs some changes now, but well… I guess you'll be done
    with that before I get to rebase my patch, I can only target tomorrow
    now.

    Regards,
    --
    Dimitri Fontaine
    http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
  • Tom Lane at Feb 8, 2011 at 10:41 pm

    Dimitri Fontaine writes:
    Tom Lane <tgl@sss.pgh.pa.us> writes:
    [ ALTER object SET EXTENSION versus ALTER EXTENSION ADD object ]
    OK, that seems like an equally reasonable syntax; although doing it the
    way I was thinking might have less of a code and documentation footprint
    (I was vaguely imagining that it could share most of the COMMENT
    infrastructure --- but haven't looked yet). In any case it seems like
    this is a good piece to do next, since the required functionality is
    clear and it's essential for more than one reason.
    Well the code footprint is quite small already.
    I was thinking about it more from the documentation side: touch one man
    page versus touch nearly all the ALTER pages. In addition, if it's all
    on the ALTER EXTENSION page then we can reference that as a list of the
    types of objects managed by extensions, which is something that's
    documented nowhere right now.

    Has anybody got any strong preference for one of these alternatives on
    more abstract grounds? You could cite ALTER OBJECT SET NAMESPACE/OWNER
    as precedents for the one syntax, but COMMENT seems like a precedent
    for the other, so that consideration seems like nearly a wash to me.

    Any other opinions out there?

    regards, tom lane
  • Tom Lane at Feb 8, 2011 at 11:54 pm

    Dimitri Fontaine writes:
    Tom Lane <tgl@sss.pgh.pa.us> writes:
    (I was vaguely imagining that it could share most of the COMMENT
    infrastructure --- but haven't looked yet).
    Well the code footprint is quite small already.
    Having now looked at it a bit closer, I think the syntax choice is a
    complete wash from an implementation standpoint: either way, we'll have
    a list of bison productions that build AlterObjectExtensionStmt nodes,
    and it goes through the same way after that. I do think that the
    implementation will be a lot more compact if it relies on the COMMENT
    infrastructure (ie, get_object_address), but that's an independent
    choice.

    So really it boils down to which syntax seems more natural and/or easier
    to document. As I said, I think a centralized ALTER EXTENSION syntax
    has some advantages from the documentation standpoint; but that's not a
    terribly strong argument, especially given that Dimitri has already done
    a patch to document things the other way.

    Preferences anyone?

    regards, tom lane
  • Robert Haas at Feb 9, 2011 at 1:59 am

    On Tue, Feb 8, 2011 at 6:54 PM, Tom Lane wrote:
    Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes:
    Tom Lane <tgl@sss.pgh.pa.us> writes:
    (I was vaguely imagining that it could share most of the COMMENT
    infrastructure --- but haven't looked yet).
    Well the code footprint is quite small already.
    Having now looked at it a bit closer, I think the syntax choice is a
    complete wash from an implementation standpoint: either way, we'll have
    a list of bison productions that build AlterObjectExtensionStmt nodes,
    and it goes through the same way after that.  I do think that the
    implementation will be a lot more compact if it relies on the COMMENT
    infrastructure (ie, get_object_address), but that's an independent
    choice.

    So really it boils down to which syntax seems more natural and/or easier
    to document.  As I said, I think a centralized ALTER EXTENSION syntax
    has some advantages from the documentation standpoint; but that's not a
    terribly strong argument, especially given that Dimitri has already done
    a patch to document things the other way.

    Preferences anyone?
    The closest exstant parallel is probably:

    ALTER SEQUENCE foo OWNED BY bar;

    I think paralleling that would probably be the most SQL-ish thing to
    do, but I can't get excited about it. The ALTER EXTENSION syntax will
    be a lot more self-contained, with all of it one part of the grammar
    and one part of the documentation. And you could even allow multiple
    objects:

    ALTER EXTENSION extension_name ADD object-description [, ...];

    Which might be handy.

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
  • Tom Lane at Feb 9, 2011 at 2:17 am

    Robert Haas writes:
    On Tue, Feb 8, 2011 at 6:54 PM, Tom Lane wrote:
    Having now looked at it a bit closer, I think the syntax choice is a
    complete wash from an implementation standpoint: either way, we'll have
    a list of bison productions that build AlterObjectExtensionStmt nodes,
    and it goes through the same way after that.

    Preferences anyone?
    The closest exstant parallel is probably:
    ALTER SEQUENCE foo OWNED BY bar;
    I think paralleling that would probably be the most SQL-ish thing to
    do, but I can't get excited about it.
    ALTER SET SCHEMA is a relatively near match as well, I think, from a
    semantic standpoint.
    The ALTER EXTENSION syntax will
    be a lot more self-contained, with all of it one part of the grammar
    and one part of the documentation. And you could even allow multiple
    objects:
    ALTER EXTENSION extension_name ADD object-description [, ...];
    Which might be handy.
    Hmm, that's an interesting thought. It'd require rather more
    refactoring of the grammar and the parsetree representation than I care
    to get into right now, but that could be a foreseeable extension in
    future. On the other hand, it's not *that* exciting, and if multi ADD
    isn't supported in the very first version then probably nobody will ever
    want to rely on it in extension scripts.

    I'm still finding that the "document in one place" angle is the most
    compelling argument to me.

    regards, tom lane
  • Tom Lane at Feb 9, 2011 at 2:48 am

    Robert Haas writes:
    ... And you could even allow multiple objects:
    ALTER EXTENSION extension_name ADD object-description [, ...];
    Which might be handy.
    I just thought of a different way of coming at the question, which might
    help us make a choice.

    Like ALTER THING SET SCHEMA, ALTER THING SET EXTENSION is implicitly
    assuming that there can be only one owning extension for an object.
    Furthermore, it's not really intended for *removal* of an object from an
    extension (a concept that doesn't even exist for SET SCHEMA). We could
    take a page from COMMENT ON and use "SET EXTENSION NULL" for that, but
    that's surely more of a hack than anything else.

    In contrast, ALTER EXTENSION ADD doesn't presuppose that you couldn't
    add the object to multiple extensions; and it has a natural inverse,
    ALTER EXTENSION DROP. I am not necessarily suggesting that we will ever
    allow either of those things, but I do suggest that we should pick a
    syntax that doesn't look like it's being forced to conform if we ever
    want to do it. The DROP case at least seems like it might be wanted
    in the relatively near future.

    So that looks to me like a fairly good argument for the ADD syntax.

    regards, tom lane
  • Robert Haas at Feb 9, 2011 at 3:05 am

    On Tue, Feb 8, 2011 at 9:48 PM, Tom Lane wrote:
    Robert Haas <robertmhaas@gmail.com> writes:
    ... And you could even allow multiple objects:
    ALTER EXTENSION extension_name ADD object-description [, ...];
    Which might be handy.
    I just thought of a different way of coming at the question, which might
    help us make a choice.

    Like ALTER THING SET SCHEMA, ALTER THING SET EXTENSION is implicitly
    assuming that there can be only one owning extension for an object.
    I would assume that we would enforce that constraint anyway. No?
    Otherwise when you drop one of the two extensions, what happens to the
    object? Seems necessary for sanity.
    Furthermore, it's not really intended for *removal* of an object from an
    extension (a concept that doesn't even exist for SET SCHEMA).  We could
    take a page from COMMENT ON and use "SET EXTENSION NULL" for that, but
    that's surely more of a hack than anything else. True.
    In contrast, ALTER EXTENSION ADD doesn't presuppose that you couldn't
    add the object to multiple extensions; and it has a natural inverse,
    ALTER EXTENSION DROP.  I am not necessarily suggesting that we will ever
    allow either of those things, but I do suggest that we should pick a
    syntax that doesn't look like it's being forced to conform if we ever
    want to do it.  The DROP case at least seems like it might be wanted
    in the relatively near future. Yep.
    So that looks to me like a fairly good argument for the ADD syntax.
    OK by me. There's also the operator class stuff, as a parallel.

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
  • Tom Lane at Feb 9, 2011 at 3:25 am

    Robert Haas writes:
    On Tue, Feb 8, 2011 at 9:48 PM, Tom Lane wrote:
    Like ALTER THING SET SCHEMA, ALTER THING SET EXTENSION is implicitly
    assuming that there can be only one owning extension for an object.
    I would assume that we would enforce that constraint anyway. No?
    Otherwise when you drop one of the two extensions, what happens to the
    object? Seems necessary for sanity.
    Not sure --- what about nested extensions, for instance? Or you could
    think about objects that are shared between two extensions, and go away
    only if all those extensions are dropped. (RPM has exactly that
    behavior for files owned by multiple packages, to take a handy example.)

    My point is that the current restriction to just one containing
    extension seems to me to be an implementation restriction, rather than
    something inherent in the concept of extensions. I have no intention of
    trying to relax that restriction in the near future --- I'm just
    pointing out that it could become an interesting thing to do.

    regards, tom lane
  • Robert Haas at Feb 9, 2011 at 3:28 am

    On Tue, Feb 8, 2011 at 10:25 PM, Tom Lane wrote:
    Robert Haas <robertmhaas@gmail.com> writes:
    On Tue, Feb 8, 2011 at 9:48 PM, Tom Lane wrote:
    Like ALTER THING SET SCHEMA, ALTER THING SET EXTENSION is implicitly
    assuming that there can be only one owning extension for an object.
    I would assume that we would enforce that constraint anyway.  No?
    Otherwise when you drop one of the two extensions, what happens to the
    object?  Seems necessary for sanity.
    Not sure --- what about nested extensions, for instance?  Or you could
    think about objects that are shared between two extensions, and go away
    only if all those extensions are dropped.  (RPM has exactly that
    behavior for files owned by multiple packages, to take a handy example.)

    My point is that the current restriction to just one containing
    extension seems to me to be an implementation restriction, rather than
    something inherent in the concept of extensions.  I have no intention of
    trying to relax that restriction in the near future --- I'm just
    pointing out that it could become an interesting thing to do.
    OK. My point was that I think we should definitely *enforce* that
    restriction until we have a very clear vision of what it means to do
    anything else, so it sounds like we're basically in agreement.

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
  • Tom Lane at Feb 9, 2011 at 3:44 am

    Robert Haas writes:
    On Tue, Feb 8, 2011 at 10:25 PM, Tom Lane wrote:
    My point is that the current restriction to just one containing
    extension seems to me to be an implementation restriction, rather than
    something inherent in the concept of extensions.  I have no intention of
    trying to relax that restriction in the near future --- I'm just
    pointing out that it could become an interesting thing to do.
    OK. My point was that I think we should definitely *enforce* that
    restriction until we have a very clear vision of what it means to do
    anything else, so it sounds like we're basically in agreement.
    Oh, for certain. I've been busy revising Dimitri's patch to use the
    get_object_address infrastructure, and here's what I've got for the
    actual implementation as distinct from syntax:


    + /*
    + * Execute ALTER THING SET EXTENSION
    + */
    + void
    + ExecAlterObjectExtensionStmt(AlterObjectExtensionStmt *stmt)
    + {
    + ObjectAddress object;
    + ObjectAddress extension;
    + Relation relation;
    +
    + /*
    + * For now, insist on superuser privilege. Later we might want to
    + * relax this to ownership of the target object and the extension.
    + */
    + if (!superuser())
    + ereport(ERROR,
    + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
    + (errmsg("must be superuser to use ALTER SET EXTENSION"))));
    +
    + /*
    + * Translate the parser representation that identifies this object into
    + * an ObjectAddress. get_object_address() will throw an error if the
    + * object does not exist, and will also acquire a lock on the target
    + * to guard against concurrent DROP and SET EXTENSION operations.
    + */
    + object = get_object_address(stmt->objtype, stmt->objname, stmt->objargs,
    + &relation, ShareUpdateExclusiveLock);
    +
    + /*
    + * Complain if object is already attached to some extension.
    + */
    + if (getExtensionOfObject(object.classId, object.objectId) != InvalidOid)
    + ereport(ERROR,
    + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
    + errmsg("%s is already a member of an extension",
    + getObjectDescription(&object))));
    +
    + /*
    + * OK, add the dependency.
    + */
    + extension.classId = ExtensionRelationId;
    + extension.objectId = get_extension_oid(stmt->extname, false);
    + extension.objectSubId = 0;
    +
    + recordDependencyOn(&object, &extension, DEPENDENCY_EXTENSION);
    +
    + /*
    + * If get_object_address() opened the relation for us, we close it to keep
    + * the reference count correct - but we retain any locks acquired by
    + * get_object_address() until commit time, to guard against concurrent
    + * activity.
    + */
    + if (relation != NULL)
    + relation_close(relation, NoLock);
    + }


    regards, tom lane
  • Dimitri Fontaine at Feb 9, 2011 at 9:27 am

    Tom Lane writes:
    Like ALTER THING SET SCHEMA, ALTER THING SET EXTENSION is implicitly
    assuming that there can be only one owning extension for an object.
    Yes, I worked from the SET SCHEMA variant and mentally mapped SET
    EXTENSION there, if looked like the same idea applied to another
    "property" of the object.
    Furthermore, it's not really intended for *removal* of an object from an
    extension (a concept that doesn't even exist for SET SCHEMA). We could
    take a page from COMMENT ON and use "SET EXTENSION NULL" for that, but
    that's surely more of a hack than anything else.

    In contrast, ALTER EXTENSION ADD doesn't presuppose that you couldn't
    add the object to multiple extensions; and it has a natural inverse,
    Well I wouldn't want to get there. I'm not seeing what use case we
    would solve by having more than one extension install the same object, I
    would rather have a common extension that the others depend on.
    ALTER EXTENSION DROP. I am not necessarily suggesting that we will ever
    allow either of those things, but I do suggest that we should pick a
    syntax that doesn't look like it's being forced to conform if we ever
    want to do it. The DROP case at least seems like it might be wanted
    in the relatively near future.
    I didn't think of that case because I would think the upgrade script
    will just DROP OBJECT instead. But in some cases I can see extension
    authors wanting to ALTER EXTENSION DROP OBJECT in their upgrade script
    and provide a second-stage script or procedure to clean up the database
    once upgraded. Only when you don't need the object anymore you can drop
    it entirely.

    I'm not sure how contrived the use case is here, but I agree that being
    prepared for it makes sense.

    Adding more that one object in one command is not of a great value I
    think, because you still have to lock each object individually, and
    that's transaction bound. Unlike ALTER TABLE … ADD COLUMN where it's a
    huge benefit to be able to lock and update the table only once for a
    number of columns (add and drops).

    But at the same time once the work is done, adding some syntax
    flexibility and a loop or two doesn't look too bad if you wanted to get
    there. Well no strong opinion as I'm not doing the work :)

    As far as upgrade script for contrib extensions are concerned, we will
    be able to produce them from SQL, right? The trick is to install the
    extension first, of course.

    CREATE EXTENSION foo;

    CREATE SCHEMA empty_place;
    SET search_path TO empty_place;

    SELECT 'ALTER EXTENSION ' || E.extname || ' ADD '
    replace(pg_describe_object(classid, objid, 0),
    N.nspname, '@extschema@')
    ';'
    FROM pg_depend D
    JOIN pg_extension E ON D.refobjid = E.oid
    AND D.refclassid = E.tableoid
    JOIN pg_namespace N ON E.extnamespace = N.oid
    WHERE deptype = 'e' AND E.extname = 'foo';

    I think it would be a good idea to have that in the documentation to
    help authors prepare their first upgrade script. Well to the extend
    that a previous form of it is included in the docs I've put in the
    upgrade patch :)

    So replacing those scripts I've been working on to switch to the new
    syntax would be a matter of running a shell script. The time consuming
    part is definitely the testing, but that too can be scripted.

    DROP EXTENSION foo;
    \i path/to/share/contrib/foo.sql
    create wrapper extension foo;
    alter extension foo upgrade;
    \dx foo

    Regards,
    --
    Dimitri Fontaine
    http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
  • Tom Lane at Feb 9, 2011 at 2:47 pm

    Dimitri Fontaine writes:
    As far as upgrade script for contrib extensions are concerned, we will
    be able to produce them from SQL, right?
    Hm, interesting idea, but I'm afraid that pg_describe_object doesn't
    produce exactly the syntax you need.

    I had personally been thinking of generating the contrib upgrade scripts
    via search-and-replace on the existing uninstall scripts.

    regards, tom lane
  • Dimitri Fontaine at Feb 9, 2011 at 3:07 pm

    Tom Lane writes:
    Hm, interesting idea, but I'm afraid that pg_describe_object doesn't
    produce exactly the syntax you need.
    It's very close. I've produced the previous set like that and the only
    problem I had were with operator class and family objects, and with
    array types. In both case a very simple replace can be used, like
    replace int[] with _int and "for access method" with "using".

    So you just add a CASE in the SELECT I proposed. Well, I didn't do it
    because I was not sure that it would still be needed with the API you're
    using.
    I had personally been thinking of generating the contrib upgrade scripts
    via search-and-replace on the existing uninstall scripts.
    Maybe that would work too.

    Regards,
    --
    Dimitri Fontaine
    http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
  • David E. Wheeler at Feb 9, 2011 at 6:20 pm

    On Feb 8, 2011, at 6:48 PM, Tom Lane wrote:

    Like ALTER THING SET SCHEMA, ALTER THING SET EXTENSION is implicitly
    assuming that there can be only one owning extension for an object.
    Furthermore, it's not really intended for *removal* of an object from an
    extension (a concept that doesn't even exist for SET SCHEMA). We could
    take a page from COMMENT ON and use "SET EXTENSION NULL" for that, but
    that's surely more of a hack than anything else.

    In contrast, ALTER EXTENSION ADD doesn't presuppose that you couldn't
    add the object to multiple extensions; and it has a natural inverse,
    ALTER EXTENSION DROP. I am not necessarily suggesting that we will ever
    allow either of those things, but I do suggest that we should pick a
    syntax that doesn't look like it's being forced to conform if we ever
    want to do it. The DROP case at least seems like it might be wanted
    in the relatively near future.

    So that looks to me like a fairly good argument for the ADD syn
    It feels a lot more natural to me, frankly. I'd tend to think about what's grouped into an extension, and look for the documentation related to extensions for how to add an object to an extension. I don't think it would occur to me, on first pass, to look in the ALTER FUNCTION docs for how to add a function to an extension.

    In my mind, I'm modifying the extension, not the function.

    Best,

    David
  • Bruce Momjian at Feb 10, 2011 at 2:04 am

    Dimitri Fontaine wrote:
    Tom Lane <tgl@sss.pgh.pa.us> writes:
    In any case that would ratchet the priority of ALTER EXTENSION UPGRADE
    back up to a must-have-for-9.1, since pg_upgrade would then leave you
    with a non-upgraded extension.

    Now what?
    What would be the problem with pg_upgrade acting the same as a
    dump&reload cycle as far as extensions are concerned? After all those
    can be considered as part of the schema, not part of the data, and the
    system catalogs are upgraded by the tool.

    It would then only break user objects that depend on the extension's
    objects OIDs, but that would be the same if they instead recorded the
    OID of catalog entries, right?

    So a valid answer for me would be that when you pg_upgrade, the
    extensions are installed again from their scripts. If you want to go
    further than that, you can insist on having the same version of the
    extension on both sides, but that would defeat the purpose of the tool
    somehow. After all you asked for an upgrade?
    The C comment in pg_upgrade.c explains the problem:

    * We control all assignments of pg_type.oid because these oids are stored
    * in user composite type values.

    (Wow, I am glad I recorded all these details.)

    The problem is that pg_dump --binary-upgrade knows to call
    binary_upgrade.set_next_pg_type_oid() before CREATE TYPE (you can test
    it yourself to see), and I am afraid we will need to do something like
    that in the extension code, perhaps by supporting a --binary-upgrade
    flag like we do for pg_dump. That seems to be the cleanest approach.
    A worse approach would be to somehow pass oids to pg_upgrade and have it
    renumber things but that seems hopelessly error-prone.

    --
    Bruce Momjian <bruce@momjian.us> http://momjian.us
    EnterpriseDB http://enterprisedb.com

    + It's impossible for everything to be true. +
  • Tom Lane at Feb 10, 2011 at 3:42 pm

    Robert Haas writes:
    On Tue, Feb 8, 2011 at 9:48 PM, Tom Lane wrote:
    In contrast, ALTER EXTENSION ADD doesn't presuppose that you couldn't
    add the object to multiple extensions; and it has a natural inverse,
    ALTER EXTENSION DROP.  I am not necessarily suggesting that we will ever
    allow either of those things, but I do suggest that we should pick a
    syntax that doesn't look like it's being forced to conform if we ever
    want to do it.  The DROP case at least seems like it might be wanted
    in the relatively near future.
    Yep.
    Actually, it occurs to me that the need for ALTER EXTENSION DROP could
    be upon us sooner than we think. The cases where an extension upgrade
    script would need that are
    (1) you want to remove some deprecated piece of the extension's API;
    (2) you want to remove some no-longer-needed internal function.
    Without ALTER EXTENSION DROP it's flat out impossible to do either.

    Deprecated API is not exactly far to seek in our contrib modules,
    either --- the example that just reminded me of this is hstore's =>
    operator, which we're already going so far as to print warnings about.
    We're not going to get to remove that until at least one release after
    we support ALTER EXTENSION DROP.

    So I'm thinking it'd be smart to expend the small amount of additional
    effort needed to support DROP right off the bat. I think that
    AlterExtensionAddStmt could be extended with an add/drop boolean for
    a net addition of only a few dozen lines of code, most of that being a
    suitable search-and-delete function in pg_depend.c.

    Any objections?

    regards, tom lane
  • Robert Haas at Feb 10, 2011 at 3:52 pm

    On Thu, Feb 10, 2011 at 10:41 AM, Tom Lane wrote:
    Robert Haas <robertmhaas@gmail.com> writes:
    On Tue, Feb 8, 2011 at 9:48 PM, Tom Lane wrote:
    In contrast, ALTER EXTENSION ADD doesn't presuppose that you couldn't
    add the object to multiple extensions; and it has a natural inverse,
    ALTER EXTENSION DROP.  I am not necessarily suggesting that we will ever
    allow either of those things, but I do suggest that we should pick a
    syntax that doesn't look like it's being forced to conform if we ever
    want to do it.  The DROP case at least seems like it might be wanted
    in the relatively near future.
    Yep.
    Actually, it occurs to me that the need for ALTER EXTENSION DROP could
    be upon us sooner than we think.  The cases where an extension upgrade
    script would need that are
    (1) you want to remove some deprecated piece of the extension's API;
    (2) you want to remove some no-longer-needed internal function.
    Without ALTER EXTENSION DROP it's flat out impossible to do either.

    Deprecated API is not exactly far to seek in our contrib modules,
    either --- the example that just reminded me of this is hstore's =>
    operator, which we're already going so far as to print warnings about.
    We're not going to get to remove that until at least one release after
    we support ALTER EXTENSION DROP.

    So I'm thinking it'd be smart to expend the small amount of additional
    effort needed to support DROP right off the bat.  I think that
    AlterExtensionAddStmt could be extended with an add/drop boolean for
    a net addition of only a few dozen lines of code, most of that being a
    suitable search-and-delete function in pg_depend.c.

    Any objections?
    No, I was pretty much just waiting for you to arrive at the same
    conclusion I'd already reached. :-)

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
  • Dimitri Fontaine at Feb 10, 2011 at 3:59 pm

    Tom Lane writes:
    Actually, it occurs to me that the need for ALTER EXTENSION DROP could
    be upon us sooner than we think. The cases where an extension upgrade
    script would need that are
    (1) you want to remove some deprecated piece of the extension's API;
    (2) you want to remove some no-longer-needed internal function.
    Without ALTER EXTENSION DROP it's flat out impossible to do either.
    What if you just DROP FUNCTION in the upgrade script?

    That said if the function is used in some expression index or worse,
    triggers, you certainly want to give users the opportunity to delay the
    step where the function is no more part of the extension from the step
    where you get rid of it.

    But if the function is implemented in C and the newer shared object has
    removed it…
    So I'm thinking it'd be smart to expend the small amount of additional
    effort needed to support DROP right off the bat. I think that
    AlterExtensionAddStmt could be extended with an add/drop boolean for
    a net addition of only a few dozen lines of code, most of that being a
    suitable search-and-delete function in pg_depend.c.
    Given your phrasing about the size of this project, I can't see any
    downside here.

    Regards,
    --
    Dimitri Fontaine
    http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
  • Tom Lane at Feb 10, 2011 at 4:33 pm

    Dimitri Fontaine writes:
    Tom Lane <tgl@sss.pgh.pa.us> writes:
    Actually, it occurs to me that the need for ALTER EXTENSION DROP could
    be upon us sooner than we think. The cases where an extension upgrade
    script would need that are
    (1) you want to remove some deprecated piece of the extension's API;
    (2) you want to remove some no-longer-needed internal function.
    Without ALTER EXTENSION DROP it's flat out impossible to do either.
    What if you just DROP FUNCTION in the upgrade script?
    That would be rejected because you're not allowed to drop an individual
    member object of an extension. (And no, I don't want to have a kluge in
    dependency.c that makes that test work differently when
    creating_extension.)

    regards, tom lane
  • Dimitri Fontaine at Feb 10, 2011 at 4:59 pm

    Tom Lane writes:
    That would be rejected because you're not allowed to drop an individual
    member object of an extension. (And no, I don't want to have a kluge in
    dependency.c that makes that test work differently when
    creating_extension.)
    Fair enough, all the more as soon as we have ALTER EXTENSION DROP :)

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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedFeb 8, '11 at 4:54p
activeFeb 10, '11 at 4:59p
posts33
users5
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2022 Grokbase