On Mon, 4 Nov 2024 at 20:42, Heikki Linnakangas <hlinn...@iki.fi> wrote: > Having spent some time playing with this, I quite like option C: break > compatibility, but provide an out-of-tree header file with > *forward*-compatibility macros. That encourages extension authors to > adapt to new idioms, but avoids having to sprinkle extension code with > #if version checks to support old versions.
+1 maintaining a subset of these things for every extension is kind of a pain > My plan is to put this on the Wiki Why the wiki and not as a file in the repo? Seems like it would be nice to update this file together with patches that introduce such breakages. To be clear, I think it shouldn't be possible to #include the file, such a forward compatibility file should always be copy-pasted. But having it in the same place as the code seems useful, just like we update docs together with the code. > We could add helpers for previous > incompatibilities in v17 and v16 too, although at quick glance I'm not > sure what those might be. One thing that I personally add to any extension I maintain is the new foreach macros introduced in PG17, because they are just so much nicer to use. > I tested this approach by adapting pg_cron to build with these patches > and the compatibility header, and compiling it with all supported server > versoins. Works great, see adapt-pg_cron.patch. Looks like a simple change indeed. On Mon, 4 Nov 2024 at 20:42, Heikki Linnakangas <hlinn...@iki.fi> wrote: > > On 31/10/2024 02:32, Michael Paquier wrote: > > On Wed, Oct 30, 2024 at 01:23:54PM -0400, Robert Haas wrote: > >> On Wed, Oct 30, 2024 at 12:03 PM Heikki Linnakangas <hlinn...@iki.fi> > >> wrote: > >>> C) We could provide "forward-compatibility" macros in a separate header > >>> file, to make the new "SetInterrupt" etc calls work in old PostgreSQL > >>> versions. Many of the extensions already have a header file like this, > >>> see e.g. citusdata/citus/src/include/pg_version_compat.h, > >>> pipelinedb/pipelinedb/include/compat.h. It might actually be a good idea > >>> to provide a semi-official header file like this on the Postgres wiki, > >>> to help extension authors. It would encourage extensions to use the > >>> latest idioms, while still being able to compile for older versions. > >>> > >>> I'm leaning towards option C). Let's rip off the band-aid, but provide > >>> documentation for how to adapt your extension code. And provide a > >>> forwards-compatibility header on the wiki, that extension authors can > >>> use to make the new Interrupt calls work against old server versions. > >> > >> I don't know which of these options is best, but I don't find any of > >> them categorically unacceptable. > > > > Looking at the compatibility macros of 0008 for the latches with > > INTERRUPT_GENERAL_WAKEUP under latch.h, the changes are not that bad > > to adapt to, IMO. It reminds of f25968c49697: hard breakage, no > > complaints I've heard of because I guess that most folks have been > > using an in-house compatibility headers. > > > > A big disadvantage of B is that someone may decide to add new code in > > core that depends on the past routines, and we'd better avoid that for > > this new layer of APIs for interrupt handling. A is a subset of C: do > > a hard switch in the core code, with C mentioning a compatibility > > layer in the wiki that does not exist in the core code. Any of A or C > > is OK, I would not choose B for the core backend. > Having spent some time playing with this, I quite like option C: break > compatibility, but provide an out-of-tree header file with > *forward*-compatibility macros. That encourages extension authors to > adapt to new idioms, but avoids having to sprinkle extension code with > #if version checks to support old versions. > > See attached pg_version_compatibility.h header. It allows compiling code > that uses basic SendInterrupt, RaiseInterrupt, WaitInterrupt calls with > older server versions. My plan is to put this on the Wiki, and update it > with similar compatibility helpers for other changes we might make in > v18 or future versions. We could add helpers for previous > incompatibilities in v17 and v16 too, although at quick glance I'm not > sure what those might be. > > I tested this approach by adapting pg_cron to build with these patches > and the compatibility header, and compiling it with all supported server > versoins. Works great, see adapt-pg_cron.patch. > > I pushed the preliminary cleanup patches from this patch set earlier, > only the main patches remain. Attached is a new version of those, with > mostly comment cleanups. > > -- > Heikki Linnakangas > Neon (https://neon.tech)