Re: [PATCH] Support % wildcard in extension upgrade filenames - Mailing list pgsql-hackers

From Mat Arye
Subject Re: [PATCH] Support % wildcard in extension upgrade filenames
Date
Msg-id CADsUR0AfzAcSEL4z4NzjQ2Kp16LXQx=dCc0oVvZuS_C_iNW1nQ@mail.gmail.com
Whole thread Raw
In response to RE: [PATCH] Support % wildcard in extension upgrade filenames  ("Regina Obe" <lr@pcorp.us>)
Responses RE: [PATCH] Support % wildcard in extension upgrade filenames
Re: [PATCH] Support % wildcard in extension upgrade filenames
List 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

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Memory leak in CachememoryContext
Next
From: Yurii Rashkovskii
Date:
Subject: Re: [PATCH] Extend the length of BackgroundWorker.bgw_library_name