On Tue, Jan 17, 2023 at 3:18 PM Jacob Champion <jchamp...@timescale.com> wrote:
> As a concrete example, Timescale's extension control file could look
> like this:
>
> default_version = '2.x.y'
> module_pathname = '$libdir/timescaledb-2.x.y'
> ...
> dump_version = true
>
> which would then cause pg_dump to issue a VERSION for its CREATE
> EXTENSION line. Other extensions would remain with the default
> (dump_version = false), so they'd be dumped without an explicit VERSION.
Even more concretely, here's a draft patch. There's no nuance --
setting dump_version affects all past versions of the extension, and
it can't be turned off at restore time. So extensions opting in would
have to be written to be side-by-side installable. (Ours is, in its
own way, but the PGDG installers don't allow it -- which maybe
highlights a weakness in this approach.) I'm still not sure if this
addresses Tom's concerns, or just adds new ones.
We could maybe give the user more control by overriding the default
version for an extension in a different TOC entry, and then add
options to ignore or include those version numbers during restore.
That doesn't address the side-by-side problem directly but gives an
escape hatch.
Earlier I wrote:
> I'm curious about your
> opinion on option 3, since it would naively seem to make pg_dump do
> _less_ work for a given extension.
This was definitely naive :D We can't just make use of the
binary-upgrade machinery to dump extension internals, because it pins
OIDs. So that might still be a valid approach, but it's not "less
work."
Thanks,
--Jacob
From 1a79eacbc6bdad8bb5e05418756d6ab35e9bab9e Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Fri, 27 Jan 2023 10:42:43 -0800
Subject: [PATCH] WIP: implement dump_version control option
The intent is to allow extensions to tie their internal catalog version
to a dump. This allows any dumped catalog tables to be upgraded after a
restore. The downside is that both the old and new extension versions
must be available for installation on the target.
This passes manual sanity checks, but there is no test yet.
---
src/backend/commands/extension.c | 29 +++++++++++++++++++++++++++++
src/bin/pg_dump/pg_dump.c | 25 ++++++++++++++++++++++---
src/bin/pg_dump/pg_dump.h | 1 +
src/include/catalog/pg_proc.dat | 4 ++++
4 files changed, 56 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index b1509cc505..b58c1d27a1 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -88,6 +88,7 @@ typedef struct ExtensionControlFile
bool relocatable; /* is ALTER EXTENSION SET SCHEMA supported? */
bool superuser; /* must be superuser to install? */
bool trusted; /* allow becoming superuser on the fly? */
+ bool dump_version; /* record the VERSION during pg_dump? */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
} ExtensionControlFile;
@@ -582,6 +583,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "dump_version") == 0)
+ {
+ if (!parse_bool(item->value, &control->dump_version))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" requires a Boolean value",
+ item->name)));
+ }
else if (strcmp(item->name, "encoding") == 0)
{
control->encoding = pg_valid_server_encoding(item->value);
@@ -639,6 +648,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->dump_version = false;
control->encoding = -1;
/*
@@ -2532,6 +2542,25 @@ pg_extension_config_dump(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+/*
+ * This function reports whether an extension's version should be written
+ * explicitly by pg_dump.
+ */
+Datum
+pg_extension_version_is_dumpable(PG_FUNCTION_ARGS)
+{
+ Name extname = PG_GETARG_NAME(0);
+ ExtensionControlFile *control;
+
+ /* Check extension name validity before any filesystem access */
+ check_valid_extension_name(NameStr(*extname));
+
+ /* Read the extension's control file */
+ control = read_extension_control_file(NameStr(*extname));
+
+ PG_RETURN_BOOL(control->dump_version);
+}
+
/*
* extension_config_remove
*
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 527c7651ab..177ffbede5 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5209,11 +5209,21 @@ getExtensions(Archive *fout, int *numExtensions)
int i_extversion;
int i_extconfig;
int i_extcondition;
+ int i_dump_version;
query = createPQExpBuffer();
appendPQExpBufferStr(query, "SELECT x.tableoid, x.oid, "
- "x.extname, n.nspname, x.extrelocatable, x.extversion, x.extconfig, x.extcondition "
+ "x.extname, n.nspname, x.extrelocatable, x.extversion, x.extconfig, x.extcondition, ");
+
+ if (fout->remoteVersion >= 160000)
+ appendPQExpBufferStr(query,
+ "pg_catalog.pg_extension_version_is_dumpable(x.extname) AS dump_version ");
+ else
+ appendPQExpBufferStr(query,
+ "false AS dump_version ");
+
+ appendPQExpBufferStr(query,
"FROM pg_extension x "
"JOIN pg_namespace n ON n.oid = x.extnamespace");
@@ -5231,6 +5241,7 @@ getExtensions(Archive *fout, int *numExtensions)
i_extversion = PQfnumber(res, "extversion");
i_extconfig = PQfnumber(res, "extconfig");
i_extcondition = PQfnumber(res, "extcondition");
+ i_dump_version = PQfnumber(res, "dump_version");
for (i = 0; i < ntups; i++)
{
@@ -5244,6 +5255,7 @@ getExtensions(Archive *fout, int *numExtensions)
extinfo[i].extversion = pg_strdup(PQgetvalue(res, i, i_extversion));
extinfo[i].extconfig = pg_strdup(PQgetvalue(res, i, i_extconfig));
extinfo[i].extcondition = pg_strdup(PQgetvalue(res, i, i_extcondition));
+ extinfo[i].dump_version = *(PQgetvalue(res, i, i_dump_version)) == 't';
/* Decide whether we want to dump it */
selectDumpableExtension(&(extinfo[i]), dopt);
@@ -10172,15 +10184,22 @@ dumpExtension(Archive *fout, const ExtensionInfo *extinfo)
/*
* In a regular dump, we simply create the extension, intentionally
* not specifying a version, so that the destination installation's
- * default version is used.
+ * default version is used. Extensions may opt into having their version
+ * dumped explicitly using their control file.
*
* Use of IF NOT EXISTS here is unlike our behavior for other object
* types; but there are various scenarios in which it's convenient to
* manually create the desired extension before restoring, so we
* prefer to allow it to exist already.
*/
- appendPQExpBuffer(q, "CREATE EXTENSION IF NOT EXISTS %s WITH SCHEMA %s;\n",
+ appendPQExpBuffer(q, "CREATE EXTENSION IF NOT EXISTS %s WITH SCHEMA %s",
qextname, fmtId(extinfo->namespace));
+ if (extinfo->dump_version)
+ {
+ appendPQExpBufferStr(q, " VERSION ");
+ appendStringLiteralAH(q, extinfo->extversion, fout);
+ }
+ appendPQExpBufferStr(q, ";\n");
}
else
{
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index e7cbd8d7ed..bef4633b97 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -180,6 +180,7 @@ typedef struct _extensionInfo
DumpableObject dobj;
char *namespace; /* schema containing extension's objects */
bool relocatable;
+ bool dump_version;
char *extversion;
char *extconfig; /* info about configuration tables */
char *extcondition;
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index c0f2a8a77c..4e0a6377b2 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10211,6 +10211,10 @@
proname => 'pg_extension_config_dump', provolatile => 'v', proparallel => 'u',
prorettype => 'void', proargtypes => 'regclass text',
prosrc => 'pg_extension_config_dump' },
+{ oid => '9043', descr => 'should pg_dump emit this extension\'s version?',
+ proname => 'pg_extension_version_is_dumpable', provolatile => 's',
+ prorettype => 'bool', proargtypes => 'name',
+ prosrc => 'pg_extension_version_is_dumpable' },
# SQL-spec window functions
{ oid => '3100', descr => 'row number within partition',
--
2.25.1
