Обсуждение: 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

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

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

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

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

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

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

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

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