proposal - get_extension_version function

Started by Pavel Stehuleabout 3 years ago15 messageshackers
Jump to latest
#1Pavel Stehule
pavel.stehule@gmail.com

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

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Stehule (#1)
Re: proposal - get_extension_version function

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

#3Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Tom Lane (#2)
Re: proposal - get_extension_version function

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

#4Pavel Stehule
pavel.stehule@gmail.com
In reply to: Jacob Champion (#3)
Re: proposal - get_extension_version function

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

Show quoted text

--Jacob

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jacob Champion (#3)
Re: proposal - get_extension_version function

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

#6Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#2)
Re: proposal - get_extension_version function

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.

Show quoted text

regards, tom lane

#7Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#5)
Re: proposal - get_extension_version function

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.

https://github.com/okbob/plpgsql_check/commit/b0970ff319256207ffe5ba5f18b2a7476c7136f9

Regards

Pavel

Show quoted text

regards, tom lane

#8Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Tom Lane (#5)
Re: proposal - get_extension_version function

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

#9Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Pavel Stehule (#4)
Re: proposal - get_extension_version function

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

#10Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Pavel Stehule (#7)
Re: proposal - get_extension_version function

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

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jacob Champion (#9)
Re: proposal - get_extension_version function

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

#12Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Tom Lane (#11)
Re: proposal - get_extension_version function

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

#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jacob Champion (#12)
Re: proposal - get_extension_version function

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

#14Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#13)
Re: proposal - get_extension_version function

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

Show quoted text

regards, tom lane

#15Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#1)
Re: proposal - get_extension_version function

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

Show quoted text

Regards

Pavel

Attachments:

0001-get_extension_version-given-an-extension-OID-look-up.patchtext/x-patch; charset=US-ASCII; name=0001-get_extension_version-given-an-extension-OID-look-up.patchDownload+51-1