Thread: proposal - get_extension_version function

proposal - get_extension_version function

From
Pavel Stehule
Date:
Hi

I try to write a safeguard check that ensures the expected extension version for an extension library.

Some like

const char *expected_extversion = "2.5";

...

extoid = getExtensionOfObject(ProcedureRelationId, fcinfo->flinfo->fn_oid));
extversion = get_extension_version(extoid);
if (strcmp(expected_extversion, extversion) != 0)
   elog(ERROR, "extension \"%s\" needs \"ALTER EXTENSION %s UPDATE\",
          get_extension_name(extversion),
          get_extension_name(extversion)))

Currently the extension version is not simply readable - I need to read directly from the table.

Notes, comments?

Regards

Pavel

Re: proposal - get_extension_version function

From
Tom Lane
Date:
Pavel Stehule <pavel.stehule@gmail.com> writes:
> I try to write a safeguard check that ensures the expected extension
> version for an extension library.

This is a bad idea.  How will you do extension upgrades, if the new .so
won't run till you apply the extension upgrade script but the old .so
malfunctions as soon as you do?  You need to make the C code as forgiving
as possible, not as unforgiving as possible.

If you have C-level ABI changes you need to make, the usual fix is to
include some sort of version number in the C name of each individual
function you've changed, so that calls made with the old or the new SQL
definition will be routed to the right place.  There are multiple
examples of this in contrib/.

            regards, tom lane



Re: proposal - get_extension_version function

From
Jacob Champion
Date:
On Wed, Mar 8, 2023 at 10:49 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> This is a bad idea.  How will you do extension upgrades, if the new .so
> won't run till you apply the extension upgrade script but the old .so
> malfunctions as soon as you do?

Which upgrade paths allow you to have an old .so with a new version
number? I didn't realize that was an issue.

--Jacob



Re: proposal - get_extension_version function

From
Pavel Stehule
Date:


st 8. 3. 2023 v 20:04 odesílatel Jacob Champion <jchampion@timescale.com> napsal:
On Wed, Mar 8, 2023 at 10:49 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> This is a bad idea.  How will you do extension upgrades, if the new .so
> won't run till you apply the extension upgrade script but the old .so
> malfunctions as soon as you do?

Which upgrade paths allow you to have an old .so with a new version
number? I didn't realize that was an issue.

installation from rpm or deb packages



--Jacob

Re: proposal - get_extension_version function

From
Tom Lane
Date:
Jacob Champion <jchampion@timescale.com> writes:
> On Wed, Mar 8, 2023 at 10:49 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> This is a bad idea.  How will you do extension upgrades, if the new .so
>> won't run till you apply the extension upgrade script but the old .so
>> malfunctions as soon as you do?

> Which upgrade paths allow you to have an old .so with a new version
> number? I didn't realize that was an issue.

More usually, it's the other way around: new .so but SQL objects not
upgraded yet.  That's typical in a pg_upgrade to a new major version,
where the new installation may have a newer extension .so than the
old one did.  You can't run ALTER EXTENSION UPGRADE if the new .so
refuses to load with the old SQL objects ... which AFAICS is exactly
what Pavel's sketch would do.

If you have old .so and new SQL objects, it's likely that at least
some of those new objects won't work --- but it's good to not break
any more functionality than you have to.  That's why I suggest
managing the compatibility checks on a per-function level rather
than trying to have an overall version check.

            regards, tom lane



Re: proposal - get_extension_version function

From
Pavel Stehule
Date:


st 8. 3. 2023 v 19:49 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
Pavel Stehule <pavel.stehule@gmail.com> writes:
> I try to write a safeguard check that ensures the expected extension
> version for an extension library.

This is a bad idea.  How will you do extension upgrades, if the new .so
won't run till you apply the extension upgrade script but the old .so
malfunctions as soon as you do?  You need to make the C code as forgiving
as possible, not as unforgiving as possible.

This method doesn't break  updates. It allows any registration, just doesn't allow execution with unsynced SQL API.
 

If you have C-level ABI changes you need to make, the usual fix is to
include some sort of version number in the C name of each individual
function you've changed, so that calls made with the old or the new SQL
definition will be routed to the right place.  There are multiple
examples of this in contrib/.

In my extensions like plpgsql_check I don't want to promise compatible ABI. I support PostgreSQL 10 .. 16, and I really don't try to multiply code for any historic input/output.



 

                        regards, tom lane

Re: proposal - get_extension_version function

From
Pavel Stehule
Date:


st 8. 3. 2023 v 20:17 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
Jacob Champion <jchampion@timescale.com> writes:
> On Wed, Mar 8, 2023 at 10:49 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> This is a bad idea.  How will you do extension upgrades, if the new .so
>> won't run till you apply the extension upgrade script but the old .so
>> malfunctions as soon as you do?

> Which upgrade paths allow you to have an old .so with a new version
> number? I didn't realize that was an issue.

More usually, it's the other way around: new .so but SQL objects not
upgraded yet.  That's typical in a pg_upgrade to a new major version,
where the new installation may have a newer extension .so than the
old one did.  You can't run ALTER EXTENSION UPGRADE if the new .so
refuses to load with the old SQL objects ... which AFAICS is exactly
what Pavel's sketch would do.

If you have old .so and new SQL objects, it's likely that at least
some of those new objects won't work --- but it's good to not break
any more functionality than you have to.  That's why I suggest
managing the compatibility checks on a per-function level rather
than trying to have an overall version check.

There is agreement - I call this check from functions.


Regards

Pavel


                        regards, tom lane

Re: proposal - get_extension_version function

From
Jacob Champion
Date:
On Wed, Mar 8, 2023 at 11:18 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Jacob Champion <jchampion@timescale.com> writes:
> > On Wed, Mar 8, 2023 at 10:49 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> This is a bad idea.  How will you do extension upgrades, if the new .so
> >> won't run till you apply the extension upgrade script but the old .so
> >> malfunctions as soon as you do?
>
> > Which upgrade paths allow you to have an old .so with a new version
> > number? I didn't realize that was an issue.
>
> More usually, it's the other way around: new .so but SQL objects not
> upgraded yet.  That's typical in a pg_upgrade to a new major version,
> where the new installation may have a newer extension .so than the
> old one did.

That's the opposite case though; I think the expectation of backwards
compatibility from C to SQL is very different from (infinite?)
forwards compatibility from C to SQL.

> If you have old .so and new SQL objects, it's likely that at least
> some of those new objects won't work --- but it's good to not break
> any more functionality than you have to.

To me it doesn't seem like a partial break is safer than refusing to
execute in the face of old-C-and-new-SQL -- assuming it's safe at all?
A bailout seems pretty reasonable in that case.

Thanks,
--Jacob



Re: proposal - get_extension_version function

From
Jacob Champion
Date:
On Wed, Mar 8, 2023 at 11:11 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
> installation from rpm or deb packages

Right, but I thought the safe order for a downgrade was to issue the
SQL downgrade first (thus putting the system back into the
post-upgrade state), and only then replacing the packages with prior
versions.

--Jacob



Re: proposal - get_extension_version function

From
Jacob Champion
Date:
On Wed, Mar 8, 2023 at 11:22 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
> There is agreement - I call this check from functions.

I think pg_auto_failover does this too, or at least used to.

Timescale does strict compatibility checks as well. It's not entirely
comparable to your implementation, though.

--Jacob



Re: proposal - get_extension_version function

From
Tom Lane
Date:
Jacob Champion <jchampion@timescale.com> writes:
> On Wed, Mar 8, 2023 at 11:11 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> installation from rpm or deb packages

> Right, but I thought the safe order for a downgrade was to issue the
> SQL downgrade first (thus putting the system back into the
> post-upgrade state), and only then replacing the packages with prior
> versions.

Pavel's proposed check would break that too.  There's going to be some
interval where the SQL definitions are not in sync with the .so version,
so you really want the .so to support at least two versions' worth of
SQL objects.

            regards, tom lane



Re: proposal - get_extension_version function

From
Jacob Champion
Date:
On Wed, Mar 8, 2023 at 1:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Pavel's proposed check would break that too.  There's going to be some
> interval where the SQL definitions are not in sync with the .so version,
> so you really want the .so to support at least two versions' worth of
> SQL objects.

I think we're in agreement that the extension must be able to load
with SQL version X and binary version X+1. (Pavel too, if I'm reading
the argument correctly -- the proposal is to gate execution paths, not
init time. And Pavel's not the only one implementing that today.)

What I'm trying to pin down is the project's position on the reverse
-- binary version X and SQL version X+1 -- because that seems
generally unmaintainable, and I don't understand why an author would
pay that tax if they could just avoid it by bailing out entirely. (If
an author wants to allow that, great, but does everyone have to?)

--Jacob



Re: proposal - get_extension_version function

From
Tom Lane
Date:
Jacob Champion <jchampion@timescale.com> writes:
> What I'm trying to pin down is the project's position on the reverse
> -- binary version X and SQL version X+1 -- because that seems
> generally unmaintainable, and I don't understand why an author would
> pay that tax if they could just avoid it by bailing out entirely. (If
> an author wants to allow that, great, but does everyone have to?)

Hard to say.  Our experience with the standard contrib modules is that
it really isn't much additional trouble; but perhaps more-complex modules
would have more interdependencies between functions.  In any case,
I fail to see the need for basing things on a catalog lookup rather
than embedding API version numbers in relevant C symbols.

            regards, tom lane



Re: proposal - get_extension_version function

From
Pavel Stehule
Date:


st 8. 3. 2023 v 23:43 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
Jacob Champion <jchampion@timescale.com> writes:
> What I'm trying to pin down is the project's position on the reverse
> -- binary version X and SQL version X+1 -- because that seems
> generally unmaintainable, and I don't understand why an author would
> pay that tax if they could just avoid it by bailing out entirely. (If
> an author wants to allow that, great, but does everyone have to?)

Hard to say.  Our experience with the standard contrib modules is that
it really isn't much additional trouble; but perhaps more-complex modules
would have more interdependencies between functions.  In any case,
I fail to see the need for basing things on a catalog lookup rather
than embedding API version numbers in relevant C symbols.

How can you check it? There is not any callback now.

Regards

Pavel


 

                        regards, tom lane

Re: proposal - get_extension_version function

From
Pavel Stehule
Date:
Hi


st 8. 3. 2023 v 17:58 odesílatel Pavel Stehule <pavel.stehule@gmail.com> napsal:
Hi

I try to write a safeguard check that ensures the expected extension version for an extension library.

Some like

const char *expected_extversion = "2.5";

...

extoid = getExtensionOfObject(ProcedureRelationId, fcinfo->flinfo->fn_oid));
extversion = get_extension_version(extoid);
if (strcmp(expected_extversion, extversion) != 0)
   elog(ERROR, "extension \"%s\" needs \"ALTER EXTENSION %s UPDATE\",
          get_extension_name(extversion),
          get_extension_name(extversion)))

Currently the extension version is not simply readable - I need to read directly from the table.

Notes, comments?

attached patch

Regards

Pavel
 

Regards

Pavel

Attachment