Re: [PATCH] Support % wildcard in extension upgrade filenames

From: Mat Arye <mat(at)timescaledb(dot)com>
To: Regina Obe <lr(at)pcorp(dot)us>
Cc: strk(at)kbt(dot)io, Yurii Rashkovskii <yrashk(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Regina Obe <r(at)pcorp(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Support % wildcard in extension upgrade filenames
Date: 2023-04-24 17:06:24
Message-ID: CADsUR0AfzAcSEL4z4NzjQ2Kp16LXQx=dCc0oVvZuS_C_iNW1nQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi All,

I've done upgrade maintenance for multiple extensions now (TimescaleDB
core, Promscale) and I think the original suggestion (wildcard filenames
with control-file switch to enable) here is a good one.

Having one big upgrade file, at its core, enables you to do is move the
logic for "what parts of the script to run for an upgrade from version V1
to V2" from a script to generate .sql files to Plpgsql in the form of
something like:

DO$$

IF <guard> THEN

<execute upgrade>

<record metadata>

END IF;

$$;

Where the guard can check postgres catalogs, internal extension catalogs,
or anything else the extension wants. What we have done in the past is
record executions of non-idempotent migration scripts in an
extension catalog table and simply check if that row exists in the guard.
This allows for much easier management of backporting patches, for
instance. Having the full power of Plpgsql makes all of this much easier
and (more flexible) than in a build script that compiles various V1--v2.sql
upgrade files.

Currently, when we did this in Promscale, we also added V1--v2.sql upgrade
files as symlinks to "the one big file". Not the end of the world, but I
think a patch like the one suggested in this thread will make things a lot
nicer.

As for Tom's concern about downgrades, I think it's valid but it's a case
that is easy to test for in Plpgsql and either handle or error. For
example, we use semver so testing for a downgrade at the top of the upgrade
script is trivial. I think this concern is a good reason to have this
functionality enabled in the control file. That way, this issue can be
documented and then only extensions that test for valid upgrades in the
script can enable this. Such an "opt-in" prevents people who haven't
thought of the need to validate the version changes from using this by
accident.

I'll add two more related points:
1) When the extension framework was first implemented I don't believe DO
blocks existed, so handling upgrade logic in the script itself just wasn't
possible. That's why I think rethinking some of the design now makes sense.
2) This change also makes it easier for extensions that use versioned .so
files (by that I mean uses extension-<version>.so rather than
extension.so). Because such extensions can't really use the chaining
property of the existing upgrade system and so need to write a
direct X--Y.sql migration file for every prior version X. I know the system
wasn't designed for this, but in reality a lot of extensions do this.
Especially the more complex ones.

Hopefully this is helpful,
Mat

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Yurii Rashkovskii 2023-04-24 17:43:51 Re: [PATCH] Extend the length of BackgroundWorker.bgw_library_name
Previous Message Tom Lane 2023-04-24 17:03:27 Re: Memory leak in CachememoryContext