agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH 4/4] rework where incremental sort paths are created
35+ messages / 8 participants
[nested] [flat]
* [PATCH 4/4] rework where incremental sort paths are created
@ 2019-07-09 00:14 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Tomas Vondra @ 2019-07-09 00:14 UTC (permalink / raw)
---
src/backend/optimizer/path/allpaths.c | 269 -----------------------
src/backend/optimizer/plan/planner.c | 299 ++++++++++++++++++++++++++
2 files changed, 299 insertions(+), 269 deletions(-)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 34a0fb4d32..3efc807164 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2665,242 +2665,6 @@ set_worktable_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
add_path(rel, create_worktablescan_path(root, rel, required_outer));
}
-
-
-/*
- * Find an equivalence class member expression, all of whose Vars, come from
- * the indicated relation.
- */
-static Expr *
-find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel)
-{
- ListCell *lc_em;
-
- foreach(lc_em, ec->ec_members)
- {
- EquivalenceMember *em = lfirst(lc_em);
-
- if (bms_is_subset(em->em_relids, rel->relids) &&
- !bms_is_empty(em->em_relids))
- {
- /*
- * If there is more than one equivalence member whose Vars are
- * taken entirely from this relation, we'll be content to choose
- * any one of those.
- */
- return em->em_expr;
- }
- }
-
- /* We didn't find any suitable equivalence class expression */
- return NULL;
-}
-
-/*
- * get_useful_ecs_for_relation
- * Determine which EquivalenceClasses might be involved in useful
- * orderings of this relation.
- *
- * This function is in some respects a mirror image of the core function
- * pathkeys_useful_for_merging: for a regular table, we know what indexes
- * we have and want to test whether any of them are useful. For a foreign
- * table, we don't know what indexes are present on the remote side but
- * want to speculate about which ones we'd like to use if they existed.
- *
- * This function returns a list of potentially-useful equivalence classes,
- * but it does not guarantee that an EquivalenceMember exists which contains
- * Vars only from the given relation. For example, given ft1 JOIN t1 ON
- * ft1.x + t1.x = 0, this function will say that the equivalence class
- * containing ft1.x + t1.x is potentially useful. Supposing ft1 is remote and
- * t1 is local (or on a different server), it will turn out that no useful
- * ORDER BY clause can be generated. It's not our job to figure that out
- * here; we're only interested in identifying relevant ECs.
- */
-static List *
-get_useful_ecs_for_relation(PlannerInfo *root, RelOptInfo *rel)
-{
- List *useful_eclass_list = NIL;
- ListCell *lc;
- Relids relids;
-
- /*
- * First, consider whether any active EC is potentially useful for a merge
- * join against this relation.
- */
- if (rel->has_eclass_joins)
- {
- foreach(lc, root->eq_classes)
- {
- EquivalenceClass *cur_ec = (EquivalenceClass *) lfirst(lc);
-
- if (eclass_useful_for_merging(root, cur_ec, rel))
- useful_eclass_list = lappend(useful_eclass_list, cur_ec);
- }
- }
-
- /*
- * Next, consider whether there are any non-EC derivable join clauses that
- * are merge-joinable. If the joininfo list is empty, we can exit
- * quickly.
- */
- if (rel->joininfo == NIL)
- return useful_eclass_list;
-
- /* If this is a child rel, we must use the topmost parent rel to search. */
- if (IS_OTHER_REL(rel))
- {
- Assert(!bms_is_empty(rel->top_parent_relids));
- relids = rel->top_parent_relids;
- }
- else
- relids = rel->relids;
-
- /* Check each join clause in turn. */
- foreach(lc, rel->joininfo)
- {
- RestrictInfo *restrictinfo = (RestrictInfo *) lfirst(lc);
-
- /* Consider only mergejoinable clauses */
- if (restrictinfo->mergeopfamilies == NIL)
- continue;
-
- /* Make sure we've got canonical ECs. */
- update_mergeclause_eclasses(root, restrictinfo);
-
- /*
- * restrictinfo->mergeopfamilies != NIL is sufficient to guarantee
- * that left_ec and right_ec will be initialized, per comments in
- * distribute_qual_to_rels.
- *
- * We want to identify which side of this merge-joinable clause
- * contains columns from the relation produced by this RelOptInfo. We
- * test for overlap, not containment, because there could be extra
- * relations on either side. For example, suppose we've got something
- * like ((A JOIN B ON A.x = B.x) JOIN C ON A.y = C.y) LEFT JOIN D ON
- * A.y = D.y. The input rel might be the joinrel between A and B, and
- * we'll consider the join clause A.y = D.y. relids contains a
- * relation not involved in the join class (B) and the equivalence
- * class for the left-hand side of the clause contains a relation not
- * involved in the input rel (C). Despite the fact that we have only
- * overlap and not containment in either direction, A.y is potentially
- * useful as a sort column.
- *
- * Note that it's even possible that relids overlaps neither side of
- * the join clause. For example, consider A LEFT JOIN B ON A.x = B.x
- * AND A.x = 1. The clause A.x = 1 will appear in B's joininfo list,
- * but overlaps neither side of B. In that case, we just skip this
- * join clause, since it doesn't suggest a useful sort order for this
- * relation.
- */
- if (bms_overlap(relids, restrictinfo->right_ec->ec_relids))
- useful_eclass_list = list_append_unique_ptr(useful_eclass_list,
- restrictinfo->right_ec);
- else if (bms_overlap(relids, restrictinfo->left_ec->ec_relids))
- useful_eclass_list = list_append_unique_ptr(useful_eclass_list,
- restrictinfo->left_ec);
- }
-
- return useful_eclass_list;
-}
-
-/*
- * get_useful_pathkeys_for_relation
- * Determine which orderings of a relation might be useful.
- *
- * Getting data in sorted order can be useful either because the requested
- * order matches the final output ordering for the overall query we're
- * planning, or because it enables an efficient merge join. Here, we try
- * to figure out which pathkeys to consider.
- */
-static List *
-get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
-{
- List *useful_pathkeys_list = NIL;
- List *useful_eclass_list;
- EquivalenceClass *query_ec = NULL;
- ListCell *lc;
-
- /*
- * Pushing the query_pathkeys to the remote server is always worth
- * considering, because it might let us avoid a local sort.
- */
- if (root->query_pathkeys)
- {
- bool query_pathkeys_ok = true;
-
- foreach(lc, root->query_pathkeys)
- {
- PathKey *pathkey = (PathKey *) lfirst(lc);
- EquivalenceClass *pathkey_ec = pathkey->pk_eclass;
- Expr *em_expr;
-
- /*
- * The planner and executor don't have any clever strategy for
- * taking data sorted by a prefix of the query's pathkeys and
- * getting it to be sorted by all of those pathkeys. We'll just
- * end up resorting the entire data set. So, unless we can push
- * down all of the query pathkeys, forget it.
- *
- * is_foreign_expr would detect volatile expressions as well, but
- * checking ec_has_volatile here saves some cycles.
- */
- if (pathkey_ec->ec_has_volatile ||
- !(em_expr = find_em_expr_for_rel(pathkey_ec, rel)))
- {
- query_pathkeys_ok = false;
- break;
- }
- }
-
- if (query_pathkeys_ok)
- useful_pathkeys_list = list_make1(list_copy(root->query_pathkeys));
- }
-
- /* Get the list of interesting EquivalenceClasses. */
- useful_eclass_list = get_useful_ecs_for_relation(root, rel);
-
- /* Extract unique EC for query, if any, so we don't consider it again. */
- if (list_length(root->query_pathkeys) == 1)
- {
- PathKey *query_pathkey = linitial(root->query_pathkeys);
-
- query_ec = query_pathkey->pk_eclass;
- }
-
- /*
- * As a heuristic, the only pathkeys we consider here are those of length
- * one. It's surely possible to consider more, but since each one we
- * choose to consider will generate a round-trip to the remote side, we
- * need to be a bit cautious here. It would sure be nice to have a local
- * cache of information about remote index definitions...
- */
- foreach(lc, useful_eclass_list)
- {
- EquivalenceClass *cur_ec = lfirst(lc);
- Expr *em_expr;
- PathKey *pathkey;
-
- /* If redundant with what we did above, skip it. */
- if (cur_ec == query_ec)
- continue;
-
- /* If no pushable expression for this rel, skip it. */
- em_expr = find_em_expr_for_rel(cur_ec, rel);
- if (em_expr == NULL)
- continue;
-
- /* Looks like we can generate a pathkey, so let's do it. */
- pathkey = make_canonical_pathkey(root, cur_ec,
- linitial_oid(cur_ec->ec_opfamilies),
- BTLessStrategyNumber,
- false);
- useful_pathkeys_list = lappend(useful_pathkeys_list,
- list_make1(pathkey));
- }
-
- return useful_pathkeys_list;
-}
-
/*
* generate_gather_paths
* Generate parallel access paths for a relation by pushing a Gather or
@@ -2955,10 +2719,6 @@ generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows)
{
Path *subpath = (Path *) lfirst(lc);
GatherMergePath *path;
- bool is_sorted;
- int presorted_keys;
- List *useful_pathkeys_list = NIL; /* List of all pathkeys */
- ListCell *lc;
if (subpath->pathkeys == NIL)
continue;
@@ -2967,35 +2727,6 @@ generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows)
path = create_gather_merge_path(root, rel, subpath, rel->reltarget,
subpath->pathkeys, NULL, rowsp);
add_path(rel, &path->path);
-
- /* consider incremental sort for interesting orderings */
- useful_pathkeys_list = get_useful_pathkeys_for_relation(root, rel);
-
- foreach(lc, useful_pathkeys_list)
- {
- List *useful_pathkeys = lfirst(lc);
-
- is_sorted = pathkeys_common_contained_in(useful_pathkeys,
- subpath->pathkeys,
- &presorted_keys);
-
- if (!is_sorted && (presorted_keys > 0))
- {
- /* Also consider incremental sort. */
- subpath = (Path *) create_incremental_sort_path(root,
- rel,
- subpath,
- useful_pathkeys,
- presorted_keys,
- -1);
-
- path = create_gather_merge_path(root, rel, subpath, rel->reltarget,
- subpath->pathkeys, NULL, rowsp);
-
- add_path(rel, &path->path);
- }
- }
-
}
}
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 16996b1bc2..ecad427c40 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -5068,6 +5068,48 @@ create_ordered_paths(PlannerInfo *root,
add_path(ordered_rel, path);
}
+
+ /* also consider incremental sorts on all partial paths */
+ {
+ ListCell *lc;
+ foreach (lc, input_rel->partial_pathlist)
+ {
+ Path *input_path = (Path *) lfirst(lc);
+ Path *sorted_path = input_path;
+ bool is_sorted;
+ int presorted_keys;
+
+ /* already handled above */
+ if (input_path == cheapest_partial_path)
+ continue;
+
+ is_sorted = pathkeys_common_contained_in(root->sort_pathkeys,
+ input_path->pathkeys, &presorted_keys);
+
+ /* also ignore already sorted paths */
+ if (is_sorted)
+ continue;
+
+ if (presorted_keys > 0)
+ {
+ /* Also consider incremental sort. */
+ sorted_path = (Path *) create_incremental_sort_path(root,
+ ordered_rel,
+ input_path,
+ root->sort_pathkeys,
+ presorted_keys,
+ limit_tuples);
+
+ /* Add projection step if needed */
+ if (sorted_path->pathtarget != target)
+ sorted_path = apply_projection_to_path(root, ordered_rel,
+ sorted_path, target);
+
+ add_path(ordered_rel, sorted_path);
+ }
+ }
+
+ }
}
/*
@@ -6484,6 +6526,80 @@ add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel,
}
}
+
+ /*
+ * Use any available suitably-sorted path as input, with incremental
+ * sort path.
+ */
+ foreach(lc, input_rel->pathlist)
+ {
+ Path *path = (Path *) lfirst(lc);
+ bool is_sorted;
+ int presorted_keys;
+
+ is_sorted = pathkeys_common_contained_in(root->group_pathkeys,
+ path->pathkeys,
+ &presorted_keys);
+
+ if (is_sorted)
+ continue;
+
+ if (presorted_keys == 0)
+ continue;
+
+ path = (Path *) create_incremental_sort_path(root,
+ grouped_rel,
+ path,
+ root->group_pathkeys,
+ presorted_keys,
+ -1.0);
+
+ /* Now decide what to stick atop it */
+ if (parse->groupingSets)
+ {
+ consider_groupingsets_paths(root, grouped_rel,
+ path, true, can_hash,
+ gd, agg_costs, dNumGroups);
+ }
+ else if (parse->hasAggs)
+ {
+ /*
+ * We have aggregation, possibly with plain GROUP BY. Make
+ * an AggPath.
+ */
+ add_path(grouped_rel, (Path *)
+ create_agg_path(root,
+ grouped_rel,
+ path,
+ grouped_rel->reltarget,
+ parse->groupClause ? AGG_SORTED : AGG_PLAIN,
+ AGGSPLIT_SIMPLE,
+ parse->groupClause,
+ havingQual,
+ agg_costs,
+ dNumGroups));
+ }
+ else if (parse->groupClause)
+ {
+ /*
+ * We have GROUP BY without aggregation or grouping sets.
+ * Make a GroupPath.
+ */
+ add_path(grouped_rel, (Path *)
+ create_group_path(root,
+ grouped_rel,
+ path,
+ parse->groupClause,
+ havingQual,
+ dNumGroups));
+ }
+ else
+ {
+ /* Other cases should have been handled above */
+ Assert(false);
+ }
+ }
+
/*
* Instead of operating directly on the input relation, we can
* consider finalizing a partially aggregated path.
@@ -6530,6 +6646,53 @@ add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel,
havingQual,
dNumGroups));
}
+
+ /* incremental sort */
+ foreach(lc, partially_grouped_rel->pathlist)
+ {
+ Path *path = (Path *) lfirst(lc);
+ bool is_sorted;
+ int presorted_keys;
+
+ is_sorted = pathkeys_common_contained_in(root->group_pathkeys,
+ path->pathkeys,
+ &presorted_keys);
+
+ if (is_sorted)
+ continue;
+
+ if (presorted_keys == 0)
+ continue;
+
+ path = (Path *) create_incremental_sort_path(root,
+ grouped_rel,
+ path,
+ root->group_pathkeys,
+ presorted_keys,
+ -1.0);
+
+ if (parse->hasAggs)
+ add_path(grouped_rel, (Path *)
+ create_agg_path(root,
+ grouped_rel,
+ path,
+ grouped_rel->reltarget,
+ parse->groupClause ? AGG_SORTED : AGG_PLAIN,
+ AGGSPLIT_FINAL_DESERIAL,
+ parse->groupClause,
+ havingQual,
+ agg_final_costs,
+ dNumGroups));
+ else
+ add_path(grouped_rel, (Path *)
+ create_group_path(root,
+ grouped_rel,
+ path,
+ parse->groupClause,
+ havingQual,
+ dNumGroups));
+ }
+
}
}
@@ -6798,6 +6961,57 @@ create_partial_grouping_paths(PlannerInfo *root,
dNumPartialGroups));
}
}
+
+ /*
+ * Use any available suitably-sorted path as input, and also consider
+ * sorting the cheapest partial path.
+ */
+ foreach(lc, input_rel->pathlist)
+ {
+ Path *path = (Path *) lfirst(lc);
+ bool is_sorted;
+ int presorted_keys;
+
+ is_sorted = pathkeys_common_contained_in(root->group_pathkeys,
+ path->pathkeys,
+ &presorted_keys);
+
+ /* also ignore already sorted paths */
+ if (is_sorted)
+ continue;
+
+ if (presorted_keys == 0)
+ continue;
+
+ /* add incremental sort */
+ path = (Path *) create_incremental_sort_path(root,
+ partially_grouped_rel,
+ path,
+ root->group_pathkeys,
+ presorted_keys,
+ -1.0);
+
+ if (parse->hasAggs)
+ add_path(partially_grouped_rel, (Path *)
+ create_agg_path(root,
+ partially_grouped_rel,
+ path,
+ partially_grouped_rel->reltarget,
+ parse->groupClause ? AGG_SORTED : AGG_PLAIN,
+ AGGSPLIT_INITIAL_SERIAL,
+ parse->groupClause,
+ NIL,
+ agg_partial_costs,
+ dNumPartialGroups));
+ else
+ add_path(partially_grouped_rel, (Path *)
+ create_group_path(root,
+ partially_grouped_rel,
+ path,
+ parse->groupClause,
+ NIL,
+ dNumPartialGroups));
+ }
}
if (can_sort && cheapest_partial_path != NULL)
@@ -6842,6 +7056,52 @@ create_partial_grouping_paths(PlannerInfo *root,
dNumPartialPartialGroups));
}
}
+
+ /* consider incremental sort */
+ foreach(lc, input_rel->partial_pathlist)
+ {
+ Path *path = (Path *) lfirst(lc);
+ bool is_sorted;
+ int presorted_keys;
+
+ is_sorted = pathkeys_common_contained_in(root->group_pathkeys,
+ path->pathkeys,
+ &presorted_keys);
+
+ if (is_sorted)
+ continue;
+
+ if (presorted_keys == 0)
+ continue;
+
+ path = (Path *) create_incremental_sort_path(root,
+ partially_grouped_rel,
+ path,
+ root->group_pathkeys,
+ presorted_keys,
+ -1.0);
+
+ if (parse->hasAggs)
+ add_partial_path(partially_grouped_rel, (Path *)
+ create_agg_path(root,
+ partially_grouped_rel,
+ path,
+ partially_grouped_rel->reltarget,
+ parse->groupClause ? AGG_SORTED : AGG_PLAIN,
+ AGGSPLIT_INITIAL_SERIAL,
+ parse->groupClause,
+ NIL,
+ agg_partial_costs,
+ dNumPartialPartialGroups));
+ else
+ add_partial_path(partially_grouped_rel, (Path *)
+ create_group_path(root,
+ partially_grouped_rel,
+ path,
+ parse->groupClause,
+ NIL,
+ dNumPartialPartialGroups));
+ }
}
if (can_hash && cheapest_total_path != NULL)
@@ -6938,6 +7198,7 @@ create_partial_grouping_paths(PlannerInfo *root,
static void
gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel)
{
+ ListCell *lc;
Path *cheapest_partial_path;
/* Try Gather for unordered paths and Gather Merge for ordered ones. */
@@ -6967,6 +7228,44 @@ gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel)
add_path(rel, path);
}
+
+ /* also consider incremental sort on all partial paths */
+ foreach (lc, rel->partial_pathlist)
+ {
+ Path *path = (Path *) lfirst(lc);
+ bool is_sorted;
+ int presorted_keys;
+ double total_groups;
+
+ is_sorted = pathkeys_common_contained_in(root->group_pathkeys,
+ path->pathkeys,
+ &presorted_keys);
+
+ if (is_sorted)
+ continue;
+
+ if (presorted_keys == 0)
+ continue;
+
+ path = (Path *) create_incremental_sort_path(root,
+ rel,
+ path,
+ root->group_pathkeys,
+ presorted_keys,
+ -1.0);
+
+ path = (Path *)
+ create_gather_merge_path(root,
+ rel,
+ path,
+ rel->reltarget,
+ root->group_pathkeys,
+ NULL,
+ &total_groups);
+
+ add_path(rel, path);
+ }
+
}
/*
--
2.20.1
--5bdgxkv4n6n2squ7--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
---
src/backend/commands/extension.c | 58 ++++++++++++++++++++++++++++----
1 file changed, 52 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1a62e5dac5..ea8825fcff 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -86,6 +86,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 wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
} ExtensionControlFile;
@@ -128,6 +129,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -579,6 +581,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ 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);
@@ -636,6 +646,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -890,7 +901,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1215,13 +1234,23 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ if (result != NIL)
+ return result;
- return result;
+ /* Find wildcard path, if allowed by control file */
+ if ( control->wildcard_upgrades )
+ {
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+
+ if (result != NIL)
+ return result;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3392,3 +3421,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
--
2.34.1
--jbsevty42g3gdjf3--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 8 ++++
src/backend/commands/extension.c | 42 ++++++++++++++++---
src/test/modules/test_extensions/Makefile | 6 ++-
.../expected/test_extensions.out | 15 +++++++
src/test/modules/test_extensions/meson.build | 3 ++
.../test_extensions/sql/test_extensions.sql | 7 ++++
.../test_ext_wildcard1--%--2.0.sql | 6 +++
.../test_ext_wildcard1--1.0.sql | 6 +++
.../test_ext_wildcard1.control | 3 ++
9 files changed, 88 insertions(+), 8 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 46e873a166..c79140f669 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1081,6 +1081,14 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
<literal>1.1</literal>).
</para>
+ <para>
+ The literal value <literal>%</literal> can be used as the
+ <replaceable>old_version</replaceable> component in an extension
+ update script for it to match any version. Such wildcard update
+ scripts will only be used when no explicit path is found from
+ old to target version.
+ </para>
+
<para>
Given that a suitable update script is available, the command
<command>ALTER EXTENSION UPDATE</command> will update an installed extension
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1a62e5dac5..e3ea9dba30 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -128,6 +128,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -890,7 +891,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( ! file_exists(filename) )
+ {
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1214,14 +1222,19 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ /* Find wildcard path, if no explicit path was found */
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- return result;
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3392,3 +3405,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index c3139ab0fc..4fe2d82b6e 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -6,14 +6,16 @@ PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
- test_ext_evttrig
+ test_ext_evttrig test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
- test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql
+ test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql \
+
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..1c4dc5be42 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -312,3 +312,18 @@ Objects in extension "test_ext_cine"
table ext_cine_tab3
(9 rows)
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index e95a9f2e7e..79d90b34c1 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/src/test/modules/test_extensions/meson.build
@@ -29,6 +29,9 @@ install_data(
'test_ext_evttrig--1.0--2.0.sql',
'test_ext_evttrig--1.0.sql',
'test_ext_evttrig.control',
+ 'test_ext_wildcard1--1.0.sql',
+ 'test_ext_wildcard1--%--2.0.sql',
+ 'test_ext_wildcard1.control',
kwargs: contrib_data_args,
)
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..071845e8df 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -209,3 +209,10 @@ CREATE EXTENSION test_ext_cine;
ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
+
+
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..0c2fc6fca6
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,3 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
--
2.34.1
--flyqbkgig2a5xh3s--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v4] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 14 +++++
src/backend/commands/extension.c | 58 +++++++++++++++++--
src/test/modules/test_extensions/Makefile | 7 ++-
.../expected/test_extensions.out | 18 ++++++
.../test_extensions/sql/test_extensions.sql | 9 +++
.../test_ext_wildcard1--%--2.0.sql | 6 ++
.../test_ext_wildcard1--1.0.sql | 6 ++
.../test_ext_wildcard1.control | 4 ++
8 files changed, 113 insertions(+), 9 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5c..3d4003eaef 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -822,6 +822,20 @@ RETURNS anycompatible AS ...
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="extend-extensions-wildcard-upgrade">
+ <term><varname>wildcard_upgrades</varname> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ This parameter, if set to <literal>true</literal> (which is not the
+ default), allows <command>ALTER EXTENSION</command> to consider
+ a wildcard character <literal>%</literal> as matching any version of
+ the extension. Such wildcard match will only be used when no
+ perfect match is found for a version.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 535072d181..c05055f5a0 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 wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
List *no_relocate; /* names of prerequisite extensions that
@@ -132,6 +133,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -584,6 +586,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ 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);
@@ -656,6 +666,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -913,7 +924,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1281,13 +1300,23 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ if (result != NIL)
+ return result;
- return result;
+ /* Find wildcard path, if allowed by control file */
+ if ( control->wildcard_upgrades )
+ {
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+
+ if (result != NIL)
+ return result;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3491,3 +3520,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 1388c0fb0b..105138d08a 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -8,8 +8,8 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_extschema \
test_ext_evttrig \
- test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
-
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3 \
+ test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
@@ -20,7 +20,8 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
test_ext_req_schema1--1.0.sql \
test_ext_req_schema2--1.0.sql \
- test_ext_req_schema3--1.0.sql
+ test_ext_req_schema3--1.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 472627a232..270840183d 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -445,3 +445,21 @@ SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
NOTICE: drop cascades to extension test_ext_req_schema2
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 51327cc321..bb567e0f19 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -276,3 +276,12 @@ ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok
SELECT test_s_dep2.dep_req1();
SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
+
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..865e37fa88
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,4 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
+wildcard_upgrades = true
--
2.34.1
--6ay2r5v7k3ozwyik--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v2] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
---
src/backend/commands/extension.c | 58 ++++++++++++++++++++++++++++----
1 file changed, 52 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 6b6720c690..e36a79ae75 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -86,6 +86,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 wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
} ExtensionControlFile;
@@ -128,6 +129,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -579,6 +581,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ 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);
@@ -636,6 +646,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -890,7 +901,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1215,13 +1234,23 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ if (result != NIL)
+ return result;
- return result;
+ /* Find wildcard path, if allowed by control file */
+ if ( control->wildcard_upgrades )
+ {
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+
+ if (result != NIL)
+ return result;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3392,3 +3421,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ AssertArg(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
--
2.34.1
--339zka8KxudZ+6RE--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v3] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 14 +++++
src/backend/commands/extension.c | 58 +++++++++++++++++--
src/test/modules/test_extensions/Makefile | 7 ++-
.../expected/test_extensions.out | 15 +++++
.../test_extensions/sql/test_extensions.sql | 9 +++
.../test_ext_wildcard1--%--2.0.sql | 6 ++
.../test_ext_wildcard1--1.0.sql | 6 ++
.../test_ext_wildcard1.control | 4 ++
8 files changed, 110 insertions(+), 9 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5c..3d4003eaef 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -822,6 +822,20 @@ RETURNS anycompatible AS ...
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="extend-extensions-wildcard-upgrade">
+ <term><varname>wildcard_upgrades</varname> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ This parameter, if set to <literal>true</literal> (which is not the
+ default), allows <command>ALTER EXTENSION</command> to consider
+ a wildcard character <literal>%</literal> as matching any version of
+ the extension. Such wildcard match will only be used when no
+ perfect match is found for a version.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 0eabe18335..207b4649f2 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 wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
List *no_relocate; /* names of prerequisite extensions that
@@ -132,6 +133,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -584,6 +586,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ 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);
@@ -656,6 +666,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -913,7 +924,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1259,13 +1278,23 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ if (result != NIL)
+ return result;
- return result;
+ /* Find wildcard path, if allowed by control file */
+ if ( control->wildcard_upgrades )
+ {
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+
+ if (result != NIL)
+ return result;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3470,3 +3499,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 70fc0c8e66..5a8205fd5d 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -7,8 +7,8 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_evttrig \
- test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
-
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3 \
+ test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
@@ -18,7 +18,8 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
test_ext_req_schema1--1.0.sql \
test_ext_req_schema2--1.0.sql \
- test_ext_req_schema3--1.0.sql
+ test_ext_req_schema3--1.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index a31775a260..0d4d8b4b70 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -389,3 +389,18 @@ SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
NOTICE: drop cascades to extension test_ext_req_schema2
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index f4947e7da6..3c40710fc1 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -232,3 +232,12 @@ ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok
SELECT test_s_dep2.dep_req1();
SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
+
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..865e37fa88
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,4 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
+wildcard_upgrades = true
--
2.34.1
--twb4kgisshvw5z4p--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v1] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 8 ++++
src/backend/commands/extension.c | 42 ++++++++++++++++---
src/test/modules/test_extensions/Makefile | 6 ++-
.../expected/test_extensions.out | 15 +++++++
src/test/modules/test_extensions/meson.build | 3 ++
.../test_extensions/sql/test_extensions.sql | 7 ++++
.../test_ext_wildcard1--%--2.0.sql | 6 +++
.../test_ext_wildcard1--1.0.sql | 6 +++
.../test_ext_wildcard1.control | 3 ++
9 files changed, 88 insertions(+), 8 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index b70cbe83ae..f1f0ae1244 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1081,6 +1081,14 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
<literal>1.1</literal>).
</para>
+ <para>
+ The literal value <literal>%</literal> can be used as the
+ <replaceable>old_version</replaceable> component in an extension
+ update script for it to match any version. Such wildcard update
+ scripts will only be used when no explicit path is found from
+ old to target version.
+ </para>
+
<para>
Given that a suitable update script is available, the command
<command>ALTER EXTENSION UPDATE</command> will update an installed extension
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 02ff4a9a7f..6df0fd403a 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -130,6 +130,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -893,7 +894,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( ! file_exists(filename) )
+ {
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1217,14 +1225,19 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ /* Find wildcard path, if no explicit path was found */
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- return result;
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3395,3 +3408,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index c3139ab0fc..4fe2d82b6e 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -6,14 +6,16 @@ PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
- test_ext_evttrig
+ test_ext_evttrig test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
- test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql
+ test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql \
+
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..1c4dc5be42 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -312,3 +312,18 @@ Objects in extension "test_ext_cine"
table ext_cine_tab3
(9 rows)
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index c3af3e1721..026be6a879 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/src/test/modules/test_extensions/meson.build
@@ -30,6 +30,9 @@ test_install_data += files(
'test_ext_evttrig--1.0--2.0.sql',
'test_ext_evttrig--1.0.sql',
'test_ext_evttrig.control',
+ 'test_ext_wildcard1--1.0.sql',
+ 'test_ext_wildcard1--%--2.0.sql',
+ 'test_ext_wildcard1.control',
)
tests += {
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..071845e8df 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -209,3 +209,10 @@ CREATE EXTENSION test_ext_cine;
ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
+
+
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..0c2fc6fca6
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,3 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
--
2.34.1
--bhtumk242qxsvzi5--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 8 ++++
src/backend/commands/extension.c | 42 ++++++++++++++++---
src/test/modules/test_extensions/Makefile | 6 ++-
.../expected/test_extensions.out | 15 +++++++
.../test_extensions/sql/test_extensions.sql | 7 ++++
.../test_ext_wildcard1--%--2.0.sql | 6 +++
.../test_ext_wildcard1--1.0.sql | 6 +++
.../test_ext_wildcard1.control | 3 ++
8 files changed, 85 insertions(+), 8 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 46e873a166..c79140f669 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1081,6 +1081,14 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
<literal>1.1</literal>).
</para>
+ <para>
+ The literal value <literal>%</literal> can be used as the
+ <replaceable>old_version</replaceable> component in an extension
+ update script for it to match any version. Such wildcard update
+ scripts will only be used when no explicit path is found from
+ old to target version.
+ </para>
+
<para>
Given that a suitable update script is available, the command
<command>ALTER EXTENSION UPDATE</command> will update an installed extension
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1a62e5dac5..e3ea9dba30 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -128,6 +128,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -890,7 +891,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( ! file_exists(filename) )
+ {
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1214,14 +1222,19 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ /* Find wildcard path, if no explicit path was found */
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- return result;
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3392,3 +3405,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index c3139ab0fc..4fe2d82b6e 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -6,14 +6,16 @@ PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
- test_ext_evttrig
+ test_ext_evttrig test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
- test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql
+ test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql \
+
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..1c4dc5be42 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -312,3 +312,18 @@ Objects in extension "test_ext_cine"
table ext_cine_tab3
(9 rows)
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..071845e8df 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -209,3 +209,10 @@ CREATE EXTENSION test_ext_cine;
ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
+
+
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..0c2fc6fca6
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,3 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
--
2.34.1
--pikujl27r76rlaub--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v2] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 8 ++++
src/backend/commands/extension.c | 42 ++++++++++++++++---
src/test/modules/test_extensions/Makefile | 7 ++--
.../expected/test_extensions.out | 16 +++++++
src/test/modules/test_extensions/meson.build | 3 ++
.../test_extensions/sql/test_extensions.sql | 9 ++++
.../test_ext_wildcard1--%--2.0.sql | 6 +++
.../test_ext_wildcard1--1.0.sql | 6 +++
.../test_ext_wildcard1.control | 3 ++
9 files changed, 91 insertions(+), 9 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5c..bdd463b81f 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1120,6 +1120,14 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
<literal>1.1</literal>).
</para>
+ <para>
+ The literal value <literal>%</literal> can be used as the
+ <replaceable>old_version</replaceable> component in an extension
+ update script for it to match any version. Such wildcard update
+ scripts will only be used when no explicit path is found from
+ old to target version.
+ </para>
+
<para>
Given that a suitable update script is available, the command
<command>ALTER EXTENSION UPDATE</command> will update an installed extension
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 0eabe18335..36b6d7e01a 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -132,6 +132,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -913,7 +914,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( ! file_exists(filename) )
+ {
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1258,14 +1266,19 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ /* Find wildcard path, if no explicit path was found */
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- return result;
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3470,3 +3483,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 70fc0c8e66..beec04eea3 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -7,8 +7,8 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_evttrig \
- test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
-
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3 \
+ test_ext_evttrig test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
@@ -18,7 +18,8 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
test_ext_req_schema1--1.0.sql \
test_ext_req_schema2--1.0.sql \
- test_ext_req_schema3--1.0.sql
+ test_ext_req_schema3--1.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql \
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index a31775a260..790b9b9368 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -389,3 +389,19 @@ SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
NOTICE: drop cascades to extension test_ext_req_schema2
+
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index 29e5bb2fb5..7b14d65545 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/src/test/modules/test_extensions/meson.build
@@ -36,6 +36,9 @@ test_install_data += files(
'test_ext_req_schema2.control',
'test_ext_req_schema3--1.0.sql',
'test_ext_req_schema3.control',
+ 'test_ext_wildcard1--1.0.sql',
+ 'test_ext_wildcard1--%--2.0.sql',
+ 'test_ext_wildcard1.control',
)
tests += {
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index f4947e7da6..676face363 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -232,3 +232,12 @@ ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok
SELECT test_s_dep2.dep_req1();
SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
+
+--
+-- Test wildcard upgrade
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..0c2fc6fca6
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,3 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
--
2.34.1
--nzgxg63vlhpg7nhu--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v4] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 14 +++++
src/backend/commands/extension.c | 58 +++++++++++++++++--
src/test/modules/test_extensions/Makefile | 7 ++-
.../expected/test_extensions.out | 18 ++++++
.../test_extensions/sql/test_extensions.sql | 9 +++
.../test_ext_wildcard1--%--2.0.sql | 6 ++
.../test_ext_wildcard1--1.0.sql | 6 ++
.../test_ext_wildcard1.control | 4 ++
8 files changed, 113 insertions(+), 9 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5c..3d4003eaef 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -822,6 +822,20 @@ RETURNS anycompatible AS ...
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="extend-extensions-wildcard-upgrade">
+ <term><varname>wildcard_upgrades</varname> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ This parameter, if set to <literal>true</literal> (which is not the
+ default), allows <command>ALTER EXTENSION</command> to consider
+ a wildcard character <literal>%</literal> as matching any version of
+ the extension. Such wildcard match will only be used when no
+ perfect match is found for a version.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 535072d181..c05055f5a0 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 wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
List *no_relocate; /* names of prerequisite extensions that
@@ -132,6 +133,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -584,6 +586,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ 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);
@@ -656,6 +666,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -913,7 +924,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1281,13 +1300,23 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ if (result != NIL)
+ return result;
- return result;
+ /* Find wildcard path, if allowed by control file */
+ if ( control->wildcard_upgrades )
+ {
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+
+ if (result != NIL)
+ return result;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3491,3 +3520,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 1388c0fb0b..105138d08a 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -8,8 +8,8 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_extschema \
test_ext_evttrig \
- test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
-
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3 \
+ test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
@@ -20,7 +20,8 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
test_ext_req_schema1--1.0.sql \
test_ext_req_schema2--1.0.sql \
- test_ext_req_schema3--1.0.sql
+ test_ext_req_schema3--1.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 472627a232..270840183d 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -445,3 +445,21 @@ SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
NOTICE: drop cascades to extension test_ext_req_schema2
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 51327cc321..bb567e0f19 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -276,3 +276,12 @@ ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok
SELECT test_s_dep2.dep_req1();
SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
+
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..865e37fa88
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,4 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
+wildcard_upgrades = true
--
2.34.1
--6ay2r5v7k3ozwyik--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 14 +++++
src/backend/commands/extension.c | 58 +++++++++++++++++--
src/test/modules/test_extensions/Makefile | 6 +-
.../expected/test_extensions.out | 15 +++++
.../test_extensions/sql/test_extensions.sql | 7 +++
.../test_ext_wildcard1--%--2.0.sql | 6 ++
.../test_ext_wildcard1--1.0.sql | 6 ++
.../test_ext_wildcard1.control | 4 ++
8 files changed, 108 insertions(+), 8 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 46e873a166..4012652574 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -807,6 +807,20 @@ RETURNS anycompatible AS ...
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><varname>wildcard_upgrades</varname> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ This parameter, if set to <literal>true</literal> (which is not the
+ default), allows <command>ALTER EXTENSION</command> to consider
+ a wildcard character <literal>%</literal> as matching any version of
+ the extension. Such wildcard match will only be used when no
+ perfect match is found for a version.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1a62e5dac5..ea8825fcff 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -86,6 +86,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 wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
} ExtensionControlFile;
@@ -128,6 +129,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -579,6 +581,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ 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);
@@ -636,6 +646,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -890,7 +901,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1215,13 +1234,23 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ if (result != NIL)
+ return result;
- return result;
+ /* Find wildcard path, if allowed by control file */
+ if ( control->wildcard_upgrades )
+ {
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+
+ if (result != NIL)
+ return result;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3392,3 +3421,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index c3139ab0fc..4fe2d82b6e 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -6,14 +6,16 @@ PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
- test_ext_evttrig
+ test_ext_evttrig test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
- test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql
+ test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql \
+
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..1c4dc5be42 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -312,3 +312,18 @@ Objects in extension "test_ext_cine"
table ext_cine_tab3
(9 rows)
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..071845e8df 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -209,3 +209,10 @@ CREATE EXTENSION test_ext_cine;
ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
+
+
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..865e37fa88
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,4 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
+wildcard_upgrades = true
--
2.34.1
--sbf3wqvs4ajivmh2--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v4] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 14 +++++
src/backend/commands/extension.c | 58 +++++++++++++++++--
src/test/modules/test_extensions/Makefile | 7 ++-
.../expected/test_extensions.out | 18 ++++++
.../test_extensions/sql/test_extensions.sql | 9 +++
.../test_ext_wildcard1--%--2.0.sql | 6 ++
.../test_ext_wildcard1--1.0.sql | 6 ++
.../test_ext_wildcard1.control | 4 ++
8 files changed, 113 insertions(+), 9 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5c..3d4003eaef 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -822,6 +822,20 @@ RETURNS anycompatible AS ...
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="extend-extensions-wildcard-upgrade">
+ <term><varname>wildcard_upgrades</varname> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ This parameter, if set to <literal>true</literal> (which is not the
+ default), allows <command>ALTER EXTENSION</command> to consider
+ a wildcard character <literal>%</literal> as matching any version of
+ the extension. Such wildcard match will only be used when no
+ perfect match is found for a version.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 535072d181..c05055f5a0 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 wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
List *no_relocate; /* names of prerequisite extensions that
@@ -132,6 +133,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -584,6 +586,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ 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);
@@ -656,6 +666,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -913,7 +924,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1281,13 +1300,23 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ if (result != NIL)
+ return result;
- return result;
+ /* Find wildcard path, if allowed by control file */
+ if ( control->wildcard_upgrades )
+ {
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+
+ if (result != NIL)
+ return result;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3491,3 +3520,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 1388c0fb0b..105138d08a 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -8,8 +8,8 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_extschema \
test_ext_evttrig \
- test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
-
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3 \
+ test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
@@ -20,7 +20,8 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
test_ext_req_schema1--1.0.sql \
test_ext_req_schema2--1.0.sql \
- test_ext_req_schema3--1.0.sql
+ test_ext_req_schema3--1.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 472627a232..270840183d 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -445,3 +445,21 @@ SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
NOTICE: drop cascades to extension test_ext_req_schema2
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 51327cc321..bb567e0f19 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -276,3 +276,12 @@ ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok
SELECT test_s_dep2.dep_req1();
SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
+
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..865e37fa88
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,4 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
+wildcard_upgrades = true
--
2.34.1
--6ay2r5v7k3ozwyik--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v2] Allow wildcard (%) in extension upgrade paths
@ 2022-09-14 09:10 Sandro Santilli <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Sandro Santilli @ 2022-09-14 09:10 UTC (permalink / raw)
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
---
src/backend/commands/extension.c | 58 ++++++++++++++++++++++++++++----
1 file changed, 52 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 6b6720c690..e36a79ae75 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -86,6 +86,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 wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
} ExtensionControlFile;
@@ -128,6 +129,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -579,6 +581,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ 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);
@@ -636,6 +646,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -890,7 +901,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1215,13 +1234,23 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ if (result != NIL)
+ return result;
- return result;
+ /* Find wildcard path, if allowed by control file */
+ if ( control->wildcard_upgrades )
+ {
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+
+ if (result != NIL)
+ return result;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3392,3 +3421,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ AssertArg(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
--
2.34.1
--339zka8KxudZ+6RE--
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v1 1/3] Rename pg_popcount_avx512.c to pg_popcount_x86_64.c.
@ 2026-01-14 17:37 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Nathan Bossart @ 2026-01-14 17:37 UTC (permalink / raw)
This is preparatory work for a follow-up commit that will move the
rest of the x86-64-specific popcount code to this file.
---
src/port/Makefile | 2 +-
src/port/meson.build | 2 +-
src/port/{pg_popcount_avx512.c => pg_popcount_x86_64.c} | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
rename src/port/{pg_popcount_avx512.c => pg_popcount_x86_64.c} (98%)
diff --git a/src/port/Makefile b/src/port/Makefile
index 4274949dfa4..1f95f27112f 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -47,7 +47,7 @@ OBJS = \
pg_localeconv_r.o \
pg_numa.o \
pg_popcount_aarch64.o \
- pg_popcount_avx512.o \
+ pg_popcount_x86_64.o \
pg_strong_random.o \
pgcheckdir.o \
pgmkdirp.o \
diff --git a/src/port/meson.build b/src/port/meson.build
index 28655142ebe..1daa6f47835 100644
--- a/src/port/meson.build
+++ b/src/port/meson.build
@@ -10,7 +10,7 @@ pgport_sources = [
'pg_localeconv_r.c',
'pg_numa.c',
'pg_popcount_aarch64.c',
- 'pg_popcount_avx512.c',
+ 'pg_popcount_x86_64.c',
'pg_strong_random.c',
'pgcheckdir.c',
'pgmkdirp.c',
diff --git a/src/port/pg_popcount_avx512.c b/src/port/pg_popcount_x86_64.c
similarity index 98%
rename from src/port/pg_popcount_avx512.c
rename to src/port/pg_popcount_x86_64.c
index 407b610bacb..453c7a06ce9 100644
--- a/src/port/pg_popcount_avx512.c
+++ b/src/port/pg_popcount_x86_64.c
@@ -1,12 +1,12 @@
/*-------------------------------------------------------------------------
*
- * pg_popcount_avx512.c
- * Holds the AVX-512 pg_popcount() implementation.
+ * pg_popcount_x86_64.c
+ * Holds the x86-64 pg_popcount() implementations.
*
* Copyright (c) 2024-2026, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/port/pg_popcount_avx512.c
+ * src/port/pg_popcount_x86_64.c
*
*-------------------------------------------------------------------------
*/
--
2.50.1 (Apple Git-155)
--RzZ1MyDExwO9gtVK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename=v1-0002-Move-x86-popcount-code-to-pg_popcount_x86_64.c.patch
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v1 1/3] Rename pg_popcount_avx512.c to pg_popcount_x86_64.c.
@ 2026-01-14 17:37 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Nathan Bossart @ 2026-01-14 17:37 UTC (permalink / raw)
This is preparatory work for a follow-up commit that will move the
rest of the x86-64-specific popcount code to this file.
---
src/port/Makefile | 2 +-
src/port/meson.build | 2 +-
src/port/{pg_popcount_avx512.c => pg_popcount_x86_64.c} | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
rename src/port/{pg_popcount_avx512.c => pg_popcount_x86_64.c} (98%)
diff --git a/src/port/Makefile b/src/port/Makefile
index 4274949dfa4..1f95f27112f 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -47,7 +47,7 @@ OBJS = \
pg_localeconv_r.o \
pg_numa.o \
pg_popcount_aarch64.o \
- pg_popcount_avx512.o \
+ pg_popcount_x86_64.o \
pg_strong_random.o \
pgcheckdir.o \
pgmkdirp.o \
diff --git a/src/port/meson.build b/src/port/meson.build
index 28655142ebe..1daa6f47835 100644
--- a/src/port/meson.build
+++ b/src/port/meson.build
@@ -10,7 +10,7 @@ pgport_sources = [
'pg_localeconv_r.c',
'pg_numa.c',
'pg_popcount_aarch64.c',
- 'pg_popcount_avx512.c',
+ 'pg_popcount_x86_64.c',
'pg_strong_random.c',
'pgcheckdir.c',
'pgmkdirp.c',
diff --git a/src/port/pg_popcount_avx512.c b/src/port/pg_popcount_x86_64.c
similarity index 98%
rename from src/port/pg_popcount_avx512.c
rename to src/port/pg_popcount_x86_64.c
index 407b610bacb..453c7a06ce9 100644
--- a/src/port/pg_popcount_avx512.c
+++ b/src/port/pg_popcount_x86_64.c
@@ -1,12 +1,12 @@
/*-------------------------------------------------------------------------
*
- * pg_popcount_avx512.c
- * Holds the AVX-512 pg_popcount() implementation.
+ * pg_popcount_x86_64.c
+ * Holds the x86-64 pg_popcount() implementations.
*
* Copyright (c) 2024-2026, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/port/pg_popcount_avx512.c
+ * src/port/pg_popcount_x86_64.c
*
*-------------------------------------------------------------------------
*/
--
2.50.1 (Apple Git-155)
--RzZ1MyDExwO9gtVK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename=v1-0002-Move-x86-popcount-code-to-pg_popcount_x86_64.c.patch
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v2 1/4] Rename pg_popcount_avx512.c to pg_popcount_x86.c.
@ 2026-01-14 17:37 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Nathan Bossart @ 2026-01-14 17:37 UTC (permalink / raw)
This is preparatory work for a follow-up commit that will move the
rest of the x86-64-specific popcount code to this file.
---
src/port/Makefile | 2 +-
src/port/meson.build | 2 +-
src/port/{pg_popcount_avx512.c => pg_popcount_x86.c} | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
rename src/port/{pg_popcount_avx512.c => pg_popcount_x86.c} (98%)
diff --git a/src/port/Makefile b/src/port/Makefile
index 4274949dfa4..6e3b7d154ed 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -47,7 +47,7 @@ OBJS = \
pg_localeconv_r.o \
pg_numa.o \
pg_popcount_aarch64.o \
- pg_popcount_avx512.o \
+ pg_popcount_x86.o \
pg_strong_random.o \
pgcheckdir.o \
pgmkdirp.o \
diff --git a/src/port/meson.build b/src/port/meson.build
index 28655142ebe..d7d4e705b89 100644
--- a/src/port/meson.build
+++ b/src/port/meson.build
@@ -10,7 +10,7 @@ pgport_sources = [
'pg_localeconv_r.c',
'pg_numa.c',
'pg_popcount_aarch64.c',
- 'pg_popcount_avx512.c',
+ 'pg_popcount_x86.c',
'pg_strong_random.c',
'pgcheckdir.c',
'pgmkdirp.c',
diff --git a/src/port/pg_popcount_avx512.c b/src/port/pg_popcount_x86.c
similarity index 98%
rename from src/port/pg_popcount_avx512.c
rename to src/port/pg_popcount_x86.c
index 407b610bacb..453c7a06ce9 100644
--- a/src/port/pg_popcount_avx512.c
+++ b/src/port/pg_popcount_x86.c
@@ -1,12 +1,12 @@
/*-------------------------------------------------------------------------
*
- * pg_popcount_avx512.c
- * Holds the AVX-512 pg_popcount() implementation.
+ * pg_popcount_x86_64.c
+ * Holds the x86-64 pg_popcount() implementations.
*
* Copyright (c) 2024-2026, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/port/pg_popcount_avx512.c
+ * src/port/pg_popcount_x86_64.c
*
*-------------------------------------------------------------------------
*/
--
2.50.1 (Apple Git-155)
--KuW0xlRTZoVkcm/J
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename=v2-0002-Move-x86-popcount-code-to-pg_popcount_x86_64.c.patch
^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v2 3/4] psql: bump minimum supported version to v10
@ 2026-04-17 18:34 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Nathan Bossart @ 2026-04-17 18:34 UTC (permalink / raw)
---
doc/src/sgml/ref/psql-ref.sgml | 2 +-
src/bin/psql/command.c | 23 +--
src/bin/psql/describe.c | 255 +--------------------------------
3 files changed, 11 insertions(+), 269 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 7c05afd4719..56c2692e618 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -5523,7 +5523,7 @@ PSQL_EDITOR_LINENUMBER_ARG='--line '
or an older major version. Backslash commands are particularly likely
to fail if the server is of a newer version than <application>psql</application>
itself. However, backslash commands of the <literal>\d</literal> family should
- work with servers of versions back to 9.2, though not necessarily with
+ work with servers of versions back to 10, though not necessarily with
servers newer than <application>psql</application> itself. The general
functionality of running SQL commands and displaying query results
should also work with servers of a newer major version, but this cannot
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 493400f9090..c9573d4b765 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4471,10 +4471,10 @@ connection_warnings(bool in_startup)
/*
* Warn if server's major version is newer than ours, or if server
- * predates our support cutoff (currently 9.2).
+ * predates our support cutoff (currently 10).
*/
if (pset.sversion / 100 > client_ver / 100 ||
- pset.sversion < 90200)
+ pset.sversion < 100000)
printf(_("WARNING: %s major version %s, server major version %s.\n"
" Some psql features might not work.\n"),
pset.progname,
@@ -6272,15 +6272,13 @@ get_create_object_cmd(EditableObjectType obj_type, Oid oid,
* ensure the right view gets replaced. Also, check relation kind
* to be sure it's a view.
*
- * Starting with PG 9.4, views may have WITH [LOCAL|CASCADED]
+ * Views may have WITH [LOCAL|CASCADED]
* CHECK OPTION. These are not part of the view definition
* returned by pg_get_viewdef() and so need to be retrieved
- * separately. Materialized views (introduced in 9.3) may have
+ * separately. Materialized views may have
* arbitrary storage parameter reloptions.
*/
printfPQExpBuffer(query, "/* %s */\n", _("Get view's definition and details"));
- if (pset.sversion >= 90400)
- {
appendPQExpBuffer(query,
"SELECT nspname, relname, relkind, "
"pg_catalog.pg_get_viewdef(c.oid, true), "
@@ -6291,19 +6289,6 @@ get_create_object_cmd(EditableObjectType obj_type, Oid oid,
"LEFT JOIN pg_catalog.pg_namespace n "
"ON c.relnamespace = n.oid WHERE c.oid = %u",
oid);
- }
- else
- {
- appendPQExpBuffer(query,
- "SELECT nspname, relname, relkind, "
- "pg_catalog.pg_get_viewdef(c.oid, true), "
- "c.reloptions AS reloptions, "
- "NULL AS checkoption "
- "FROM pg_catalog.pg_class c "
- "LEFT JOIN pg_catalog.pg_namespace n "
- "ON c.relnamespace = n.oid WHERE c.oid = %u",
- oid);
- }
break;
}
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index e1449654f96..76d299fb55c 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3,9 +3,9 @@
*
* Support for the various \d ("describe") commands. Note that the current
* expectation is that all functions in this file will succeed when working
- * with servers of versions 9.2 and up. It's okay to omit irrelevant
+ * with servers of versions 10 and up. It's okay to omit irrelevant
* information for an old server, but not to fail outright. (But failing
- * against a pre-9.2 server is allowed.)
+ * against a pre-10 server is allowed.)
*
* Copyright (c) 2000-2026, PostgreSQL Global Development Group
*
@@ -98,20 +98,12 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
gettext_noop("Result data type"),
gettext_noop("Argument data types"));
- if (pset.sversion >= 110000)
appendPQExpBuffer(&buf,
" pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"\n"
"FROM pg_catalog.pg_proc p\n"
" LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace\n"
"WHERE p.prokind = " CppAsString2(PROKIND_AGGREGATE) "\n",
gettext_noop("Description"));
- else
- appendPQExpBuffer(&buf,
- " pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"\n"
- "FROM pg_catalog.pg_proc p\n"
- " LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace\n"
- "WHERE p.proisagg\n",
- gettext_noop("Description"));
if (!showSystem && !pattern)
appendPQExpBufferStr(&buf, " AND n.nspname <> 'pg_catalog'\n"
@@ -154,16 +146,6 @@ describeAccessMethods(const char *pattern, bool verbose)
printQueryOpt myopt = pset.popt;
static const bool translate_columns[] = {false, true, false, false};
- if (pset.sversion < 90600)
- {
- char sverbuf[32];
-
- pg_log_error("The server (version %s) does not support access methods.",
- formatPGVersionNumber(pset.sversion, false,
- sverbuf, sizeof(sverbuf)));
- return true;
- }
-
initPQExpBuffer(&buf);
printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching access methods"));
@@ -312,9 +294,6 @@ describeFunctions(const char *functypes, const char *func_pattern,
printQueryOpt myopt = pset.popt;
static const bool translate_columns[] = {false, false, false, false, true, true, true, false, true, true, false, false, false, false};
- /* No "Parallel" column before 9.6 */
- static const bool translate_columns_pre_96[] = {false, false, false, false, true, true, false, true, true, false, false, false, false};
-
if (strlen(functypes) != strspn(functypes, df_options))
{
pg_log_error("\\df only takes [%s] as options", df_options);
@@ -400,7 +379,6 @@ describeFunctions(const char *functypes, const char *func_pattern,
gettext_noop("stable"),
gettext_noop("volatile"),
gettext_noop("Volatility"));
- if (pset.sversion >= 90600)
appendPQExpBuffer(&buf,
",\n CASE\n"
" WHEN p.proparallel = "
@@ -613,16 +591,8 @@ describeFunctions(const char *functypes, const char *func_pattern,
myopt.title = _("List of functions");
myopt.translate_header = true;
- if (pset.sversion >= 90600)
- {
myopt.translate_columns = translate_columns;
myopt.n_translate_columns = lengthof(translate_columns);
- }
- else
- {
- myopt.translate_columns = translate_columns_pre_96;
- myopt.n_translate_columns = lengthof(translate_columns_pre_96);
- }
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
@@ -1108,38 +1078,6 @@ permissionsList(const char *pattern, bool showSystem)
" ), E'\\n') AS \"%s\"",
gettext_noop("Column privileges"));
- if (pset.sversion >= 90500 && pset.sversion < 100000)
- appendPQExpBuffer(&buf,
- ",\n pg_catalog.array_to_string(ARRAY(\n"
- " SELECT polname\n"
- " || CASE WHEN polcmd != '*' THEN\n"
- " E' (' || polcmd::pg_catalog.text || E'):'\n"
- " ELSE E':'\n"
- " END\n"
- " || CASE WHEN polqual IS NOT NULL THEN\n"
- " E'\\n (u): ' || pg_catalog.pg_get_expr(polqual, polrelid)\n"
- " ELSE E''\n"
- " END\n"
- " || CASE WHEN polwithcheck IS NOT NULL THEN\n"
- " E'\\n (c): ' || pg_catalog.pg_get_expr(polwithcheck, polrelid)\n"
- " ELSE E''\n"
- " END"
- " || CASE WHEN polroles <> '{0}' THEN\n"
- " E'\\n to: ' || pg_catalog.array_to_string(\n"
- " ARRAY(\n"
- " SELECT rolname\n"
- " FROM pg_catalog.pg_roles\n"
- " WHERE oid = ANY (polroles)\n"
- " ORDER BY 1\n"
- " ), E', ')\n"
- " ELSE E''\n"
- " END\n"
- " FROM pg_catalog.pg_policy pol\n"
- " WHERE polrelid = c.oid), E'\\n')\n"
- " AS \"%s\"",
- gettext_noop("Policies"));
-
- if (pset.sversion >= 100000)
appendPQExpBuffer(&buf,
",\n pg_catalog.array_to_string(ARRAY(\n"
" SELECT polname\n"
@@ -1666,7 +1604,7 @@ describeOneTableDetails(const char *schemaname,
: "''"),
oid);
}
- else if (pset.sversion >= 100000)
+ else
{
appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
@@ -1683,57 +1621,6 @@ describeOneTableDetails(const char *schemaname,
: "''"),
oid);
}
- else if (pset.sversion >= 90500)
- {
- appendPQExpBuffer(&buf,
- "SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
- "c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
- "c.relhasoids, false as relispartition, %s, c.reltablespace, "
- "CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END, "
- "c.relpersistence, c.relreplident\n"
- "FROM pg_catalog.pg_class c\n "
- "LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)\n"
- "WHERE c.oid = '%s';",
- (verbose ?
- "pg_catalog.array_to_string(c.reloptions || "
- "array(select 'toast.' || x from pg_catalog.unnest(tc.reloptions) x), ', ')\n"
- : "''"),
- oid);
- }
- else if (pset.sversion >= 90400)
- {
- appendPQExpBuffer(&buf,
- "SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
- "c.relhastriggers, false, false, c.relhasoids, "
- "false as relispartition, %s, c.reltablespace, "
- "CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END, "
- "c.relpersistence, c.relreplident\n"
- "FROM pg_catalog.pg_class c\n "
- "LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)\n"
- "WHERE c.oid = '%s';",
- (verbose ?
- "pg_catalog.array_to_string(c.reloptions || "
- "array(select 'toast.' || x from pg_catalog.unnest(tc.reloptions) x), ', ')\n"
- : "''"),
- oid);
- }
- else
- {
- appendPQExpBuffer(&buf,
- "SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
- "c.relhastriggers, false, false, c.relhasoids, "
- "false as relispartition, %s, c.reltablespace, "
- "CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END, "
- "c.relpersistence\n"
- "FROM pg_catalog.pg_class c\n "
- "LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)\n"
- "WHERE c.oid = '%s';",
- (verbose ?
- "pg_catalog.array_to_string(c.reloptions || "
- "array(select 'toast.' || x from pg_catalog.unnest(tc.reloptions) x), ', ')\n"
- : "''"),
- oid);
- }
res = PSQLexec(buf.data);
if (!res)
@@ -1761,8 +1648,7 @@ describeOneTableDetails(const char *schemaname,
tableinfo.reloftype = (strcmp(PQgetvalue(res, 0, 11), "") != 0) ?
pg_strdup(PQgetvalue(res, 0, 11)) : NULL;
tableinfo.relpersistence = *(PQgetvalue(res, 0, 12));
- tableinfo.relreplident = (pset.sversion >= 90400) ?
- *(PQgetvalue(res, 0, 13)) : 'd';
+ tableinfo.relreplident = *(PQgetvalue(res, 0, 13));
if (pset.sversion >= 120000)
tableinfo.relam = PQgetisnull(res, 0, 14) ?
NULL : pg_strdup(PQgetvalue(res, 0, 14));
@@ -1781,8 +1667,6 @@ describeOneTableDetails(const char *schemaname,
char *footers[3] = {NULL, NULL, NULL};
printfPQExpBuffer(&buf, "/* %s */\n", _("Get sequence information"));
- if (pset.sversion >= 100000)
- {
appendPQExpBuffer(&buf,
"SELECT pg_catalog.format_type(seqtypid, NULL) AS \"%s\",\n"
" seqstart AS \"%s\",\n"
@@ -1804,30 +1688,6 @@ describeOneTableDetails(const char *schemaname,
"FROM pg_catalog.pg_sequence\n"
"WHERE seqrelid = '%s';",
oid);
- }
- else
- {
- appendPQExpBuffer(&buf,
- "SELECT 'bigint' AS \"%s\",\n"
- " start_value AS \"%s\",\n"
- " min_value AS \"%s\",\n"
- " max_value AS \"%s\",\n"
- " increment_by AS \"%s\",\n"
- " CASE WHEN is_cycled THEN '%s' ELSE '%s' END AS \"%s\",\n"
- " cache_value AS \"%s\"\n",
- gettext_noop("Type"),
- gettext_noop("Start"),
- gettext_noop("Minimum"),
- gettext_noop("Maximum"),
- gettext_noop("Increment"),
- gettext_noop("yes"),
- gettext_noop("no"),
- gettext_noop("Cycles?"),
- gettext_noop("Cache"));
- appendPQExpBuffer(&buf, "FROM %s", fmtId(schemaname));
- /* must be separate because fmtId isn't reentrant */
- appendPQExpBuffer(&buf, ".%s;", fmtId(relationname));
- }
res = PSQLexec(buf.data);
if (!res)
@@ -2045,10 +1905,7 @@ describeOneTableDetails(const char *schemaname,
appendPQExpBufferStr(&buf, ",\n (SELECT c.collname FROM pg_catalog.pg_collation c, pg_catalog.pg_type t\n"
" WHERE c.oid = a.attcollation AND t.oid = a.atttypid AND a.attcollation <> t.typcollation) AS attcollation");
attcoll_col = cols++;
- if (pset.sversion >= 100000)
appendPQExpBufferStr(&buf, ",\n a.attidentity");
- else
- appendPQExpBufferStr(&buf, ",\n ''::pg_catalog.char AS attidentity");
attidentity_col = cols++;
if (pset.sversion >= 120000)
appendPQExpBufferStr(&buf, ",\n a.attgenerated");
@@ -2059,14 +1916,11 @@ describeOneTableDetails(const char *schemaname,
if (tableinfo.relkind == RELKIND_INDEX ||
tableinfo.relkind == RELKIND_PARTITIONED_INDEX)
{
- if (pset.sversion >= 110000)
- {
appendPQExpBuffer(&buf, ",\n CASE WHEN a.attnum <= (SELECT i.indnkeyatts FROM pg_catalog.pg_index i WHERE i.indexrelid = '%s') THEN '%s' ELSE '%s' END AS is_key",
oid,
gettext_noop("yes"),
gettext_noop("no"));
isindexkey_col = cols++;
- }
appendPQExpBufferStr(&buf, ",\n pg_catalog.pg_get_indexdef(a.attrelid, a.attnum, TRUE) AS indexdef");
indexdef_col = cols++;
}
@@ -2461,10 +2315,7 @@ describeOneTableDetails(const char *schemaname,
CppAsString2(CONSTRAINT_EXCLUSION) ") AND "
"condeferred) AS condeferred,\n");
- if (pset.sversion >= 90400)
appendPQExpBufferStr(&buf, "i.indisreplident,\n");
- else
- appendPQExpBufferStr(&buf, "false AS indisreplident,\n");
if (pset.sversion >= 150000)
appendPQExpBufferStr(&buf, "i.indnullsnotdistinct,\n");
@@ -2569,10 +2420,7 @@ describeOneTableDetails(const char *schemaname,
"pg_catalog.pg_get_indexdef(i.indexrelid, 0, true),\n "
"pg_catalog.pg_get_constraintdef(con.oid, true), "
"contype, condeferrable, condeferred");
- if (pset.sversion >= 90400)
appendPQExpBufferStr(&buf, ", i.indisreplident");
- else
- appendPQExpBufferStr(&buf, ", false AS indisreplident");
appendPQExpBufferStr(&buf, ", c2.reltablespace");
if (pset.sversion >= 180000)
appendPQExpBufferStr(&buf, ", con.conperiod");
@@ -2823,17 +2671,11 @@ describeOneTableDetails(const char *schemaname,
PQclear(result);
/* print any row-level policies */
- if (pset.sversion >= 90500)
- {
printfPQExpBuffer(&buf, "/* %s */\n",
_("Get row-level policies for this table"));
appendPQExpBufferStr(&buf, "SELECT pol.polname,");
- if (pset.sversion >= 100000)
appendPQExpBufferStr(&buf,
" pol.polpermissive,\n");
- else
- appendPQExpBufferStr(&buf,
- " 't' as polpermissive,\n");
appendPQExpBuffer(&buf,
" CASE WHEN pol.polroles = '{0}' THEN NULL ELSE pg_catalog.array_to_string(array(select rolname from pg_catalog.pg_roles where oid = any (pol.polroles) order by 1),',') END,\n"
" pg_catalog.pg_get_expr(pol.polqual, pol.polrelid),\n"
@@ -2904,7 +2746,6 @@ describeOneTableDetails(const char *schemaname,
printTableAddFooter(&cont, buf.data);
}
PQclear(result);
- }
/* print any extended statistics */
if (pset.sversion >= 140000)
@@ -3007,7 +2848,7 @@ describeOneTableDetails(const char *schemaname,
}
PQclear(result);
}
- else if (pset.sversion >= 100000)
+ else
{
printfPQExpBuffer(&buf, "/* %s */\n",
_("Get extended statistics for this table"));
@@ -3173,8 +3014,6 @@ describeOneTableDetails(const char *schemaname,
}
/* print any publications */
- if (pset.sversion >= 100000)
- {
printfPQExpBuffer(&buf, "/* %s */\n",
_("Get publications that publish this table"));
if (pset.sversion >= 150000)
@@ -3284,7 +3123,6 @@ describeOneTableDetails(const char *schemaname,
printTableAddFooter(&cont, buf.data);
}
PQclear(result);
- }
/* Print publications where the table is in the EXCEPT clause */
if (pset.sversion >= 190000)
@@ -3706,7 +3544,7 @@ describeOneTableDetails(const char *schemaname,
"ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT',"
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
- else if (pset.sversion >= 100000)
+ else
appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" false AS inhdetachpending,"
@@ -3716,14 +3554,6 @@ describeOneTableDetails(const char *schemaname,
"ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT',"
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
- else
- appendPQExpBuffer(&buf,
- "SELECT c.oid::pg_catalog.regclass, c.relkind,"
- " false AS inhdetachpending, NULL\n"
- "FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n"
- "WHERE c.oid = i.inhrelid AND i.inhparent = '%s'\n"
- "ORDER BY c.oid::pg_catalog.regclass::pg_catalog.text;",
- oid);
result = PSQLexec(buf.data);
if (!result)
@@ -3964,11 +3794,7 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
ncols++;
}
appendPQExpBufferStr(&buf, "\n, r.rolreplication");
-
- if (pset.sversion >= 90500)
- {
appendPQExpBufferStr(&buf, "\n, r.rolbypassrls");
- }
appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_roles r\n");
@@ -4023,7 +3849,6 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
if (strcmp(PQgetvalue(res, i, (verbose ? 9 : 8)), "t") == 0)
add_role_attribute(&buf, _("Replication"));
- if (pset.sversion >= 90500)
if (strcmp(PQgetvalue(res, i, (verbose ? 10 : 9)), "t") == 0)
add_role_attribute(&buf, _("Bypass RLS"));
@@ -4514,19 +4339,6 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
const char *tabletitle;
bool mixed_output = false;
- /*
- * Note: Declarative table partitioning is only supported as of Pg 10.0.
- */
- if (pset.sversion < 100000)
- {
- char sverbuf[32];
-
- pg_log_error("The server (version %s) does not support declarative table partitioning.",
- formatPGVersionNumber(pset.sversion, false,
- sverbuf, sizeof(sverbuf)));
- return true;
- }
-
/* If no relation kind was selected, show them all */
if (!showTables && !showIndexes)
showTables = showIndexes = true;
@@ -5034,16 +4846,6 @@ listEventTriggers(const char *pattern, bool verbose)
static const bool translate_columns[] =
{false, false, false, true, false, false, false};
- if (pset.sversion < 90300)
- {
- char sverbuf[32];
-
- pg_log_error("The server (version %s) does not support event triggers.",
- formatPGVersionNumber(pset.sversion, false,
- sverbuf, sizeof(sverbuf)));
- return true;
- }
-
initPQExpBuffer(&buf);
printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching event triggers"));
@@ -5113,16 +4915,6 @@ listExtendedStats(const char *pattern, bool verbose)
PGresult *res;
printQueryOpt myopt = pset.popt;
- if (pset.sversion < 100000)
- {
- char sverbuf[32];
-
- pg_log_error("The server (version %s) does not support extended statistics.",
- formatPGVersionNumber(pset.sversion, false,
- sverbuf, sizeof(sverbuf)));
- return true;
- }
-
initPQExpBuffer(&buf);
printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching extended statistics"));
@@ -5352,7 +5144,6 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
gettext_noop("Schema"),
gettext_noop("Name"));
- if (pset.sversion >= 100000)
appendPQExpBuffer(&buf,
" CASE c.collprovider "
"WHEN " CppAsString2(COLLPROVIDER_DEFAULT) " THEN 'default' "
@@ -5361,10 +5152,6 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
"WHEN " CppAsString2(COLLPROVIDER_ICU) " THEN 'icu' "
"END AS \"%s\",\n",
gettext_noop("Provider"));
- else
- appendPQExpBuffer(&buf,
- " 'libc' AS \"%s\",\n",
- gettext_noop("Provider"));
appendPQExpBuffer(&buf,
" c.collcollate AS \"%s\",\n"
@@ -6688,16 +6475,6 @@ listPublications(const char *pattern)
printQueryOpt myopt = pset.popt;
static const bool translate_columns[] = {false, false, false, false, false, false, false, false, false, false};
- if (pset.sversion < 100000)
- {
- char sverbuf[32];
-
- pg_log_error("The server (version %s) does not support publications.",
- formatPGVersionNumber(pset.sversion, false,
- sverbuf, sizeof(sverbuf)));
- return true;
- }
-
initPQExpBuffer(&buf);
printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching publications"));
@@ -6835,16 +6612,6 @@ describePublications(const char *pattern)
PQExpBufferData title;
printTableContent cont;
- if (pset.sversion < 100000)
- {
- char sverbuf[32];
-
- pg_log_error("The server (version %s) does not support publications.",
- formatPGVersionNumber(pset.sversion, false,
- sverbuf, sizeof(sverbuf)));
- return true;
- }
-
has_pubsequence = (pset.sversion >= 190000);
has_pubtruncate = (pset.sversion >= 110000);
has_pubgencols = (pset.sversion >= 180000);
@@ -7095,16 +6862,6 @@ describeSubscriptions(const char *pattern, bool verbose)
false, false, false, false, false, false, false, false, false, false,
false, false, false, false, false, false, false};
- if (pset.sversion < 100000)
- {
- char sverbuf[32];
-
- pg_log_error("The server (version %s) does not support subscriptions.",
- formatPGVersionNumber(pset.sversion, false,
- sverbuf, sizeof(sverbuf)));
- return true;
- }
-
initPQExpBuffer(&buf);
printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching subscriptions"));
--
2.50.1 (Apple Git-155)
--CT89ko5pLUrsvFtH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=v2-0004-run-pgindent.patch
^ permalink raw reply [nested|flat] 35+ messages in thread
* Support logical replication of DDLs, take2
@ 2026-04-20 23:14 Masahiko Sawada <[email protected]>
2026-04-27 06:14 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-28 06:32 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
0 siblings, 2 replies; 35+ messages in thread
From: Masahiko Sawada @ 2026-04-20 23:14 UTC (permalink / raw)
To: Vitaly Davydov <[email protected]>; +Cc: Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On Mon, Feb 23, 2026 at 5:21 PM Masahiko Sawada <[email protected]> wrote:
>
>
> One idea I'm experimenting with is that we define an abstract data
> type that can represent a DDL (like CollectedCommand) and write it to
> a new WAL record so that logical decoding processes it. For CREATE
> DDLs, we can use pg_get_xxx_def() function while using a historical
> snapshot to get the DDLs. We would need to implement the codes to
> generate DROP and ALTER DDLs from the data. I believe DROP DDLs would
> not be hard. For ALTER DDLs, we would incur the initial implementation
> costs, but we would not change these codes often.
>
DDL support for logical replication is one of the biggest missing
pieces in logical replication. I'd like to resume this work for PG20.
We made a lot of effort on this feature through 2022 and 2023, but the
development is currently inactive. The last patch was submitted on Jul
18, 2023. I've reviewed the previous patches and discussions, and I
would like to summarize how DDL replication was implemented, the main
reasons it stalled, and propose an alternative design to address those
problems.
The overall idea of the previous patch set was to implement DDL
deparsing and utilize it for DDL replication. It converted a parse
tree into a JSON string. For instance, if a user executes "DROP TABLE
t1", the deparser generates from its parse tree:
{DROPSTMT :objects (("t1")) :removeType 41 :behavior 0 :missing_ok
false :concurrent false}
to:
{"fmt": "DROP TABLE %{objidentity}s", "objidentity": "public.t1"}
This JSON string is self-documenting, meaning someone who gets it can
easily reconstruct the original DDL with schema-qualified object
names. In a dedicated event trigger for logical replication, we
deparsed the parse tree of a DDL, wrote it into a WAL record, and then
the logical decoding processed it similarly to DML changes.
While there are several benefits to the JSON data approach mentioned
in the wiki [1] -- most notably the flexibility to easily remap
schemas (e.g., mapping "schema A" on the publisher to "schema B" on
the subscriber) -- there was a major concern: the huge maintenance
burden. We would need to maintain the JSON serialization code whenever
creating or modifying parse nodes, regardless of whether the changes
were related to DDL replication. IIUC, this was the primary reason the
feature didn't cross the finish line.
Additionally, I think there is another design issue: it is not
output-plugin agnostic. Since the deparsed DDL was written by a
logical-replication-specific event trigger, third-party logical
decoding plugins cannot easily detect DDL events. Ideally, we should
write DDL information into a WAL record natively when
wal_level='logical' (or additionally when a GUC enables DDL events
WAL-logging) so that all decoding plugins can detect them. This also
allows us to test DDL logical decoding with test_decoding without
setting up a full logical replication subscription.
To address these two points, I'd like to propose an alternative
approach: we introduce a new data type, say DDLCommand, that is
self-contained to represent a DDL (like CollectedCommand), and don't
rely on event triggers. It would have the command type (and subtype if
required), the OIDs of the target object and its namespace, and the
OID of the user who executed the DDL. We write it to a new WAL record
at appropriate places during DDL execution, and the logical decoding
layer passes the data to output plugins. That way, any logical
decoding plugin can detect DDL changes, and it's up to the plugins how
to decode the DDL information.
In pgoutput, for CREATE DDLs, we can use the pg_get_xxx_ddl()
functions while using a historical snapshot to get the DDLs, saving
maintenance costs. We would still need to implement the code to
generate DROP and ALTER DDLs from the data. I believe DROP DDLs would
not be hard. For ALTER DDLs, we would incur an initial implementation
cost, but we would not need to change this code often. We can
implement the DDL generation code in a way that improves ddlutils.c.
Also, because DDLCommand is separated from parse nodes, we only need
to change the DDL deparse/replication code when it is actually needed.
Additionally, this approach would eliminate the code around the
two-step process (using DCT_TableDropStart and DCT_TableDropEnd) for
DROP TABLE. While it would miss the flexibility benefits that the JSON
deparsing approach has, I guess it would not be very hard to implement
the mapping in the deparse layer even without the JSON data.
Regarding the publication syntax, previous patches proposed:
CREATE PUBLICATION pub FOR ALL TABLES WITH (ddl = 'table');
While simple, it doesn't support critical enterprise use cases (e.g.,
DWH environments) where users want to replicate CREATE and ALTER, but
explicitly filter out DROP TABLE to prevent accidental data loss. We
should consider introducing publish_ddl options to filter operations:
CREATE PUBLICATION pub FOR ALL TABLES WITH (publish_ddl = 'create, alter');
I have implemented the basic idea with the above changes and it seems
to work well, though the patch is not yet ready to share. I'd like to
resume the discussion to move this project forward. My initial MVP
goal is to support CREATE/ALTER/DROP TABLE, which covers the vast
majority of use cases, and incrementally extend support for other
object types later.
FYI I've experimented with auto-generation approaches too. For
instance, gen_node_support.pl generates C code that converts parse
nodes to the corresponding text representations. Or
gen_node_support.pl generates C code that makes all objects in the
given SQL query text fully-schema qualified. While these ideas are
promising they didn't help reduce the maintenance burden much as the
parse node definitions are already complex and vary on nodes much.
Thank you for taking the time to read through this long email.
Regards,
[1] https://wiki.postgresql.org/wiki/Logical_replication_of_DDLs#JSONB_Benefits
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
@ 2026-04-27 06:14 ` Dilip Kumar <[email protected]>
2026-04-28 06:57 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 21:38 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
1 sibling, 2 replies; 35+ messages in thread
From: Dilip Kumar @ 2026-04-27 06:14 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Apr 21, 2026 at 4:45 AM Masahiko Sawada <[email protected]> wrote:
>
> Hi,
>
> On Mon, Feb 23, 2026 at 5:21 PM Masahiko Sawada <[email protected]> wrote:
> >
> >
> > One idea I'm experimenting with is that we define an abstract data
> > type that can represent a DDL (like CollectedCommand) and write it to
> > a new WAL record so that logical decoding processes it. For CREATE
> > DDLs, we can use pg_get_xxx_def() function while using a historical
> > snapshot to get the DDLs. We would need to implement the codes to
> > generate DROP and ALTER DDLs from the data. I believe DROP DDLs would
> > not be hard. For ALTER DDLs, we would incur the initial implementation
> > costs, but we would not change these codes often.
> >
>
> DDL support for logical replication is one of the biggest missing
> pieces in logical replication. I'd like to resume this work for PG20.
Thanks for working on this.
> We made a lot of effort on this feature through 2022 and 2023, but the
> development is currently inactive. The last patch was submitted on Jul
> 18, 2023. I've reviewed the previous patches and discussions, and I
> would like to summarize how DDL replication was implemented, the main
> reasons it stalled, and propose an alternative design to address those
> problems.
>
> The overall idea of the previous patch set was to implement DDL
> deparsing and utilize it for DDL replication. It converted a parse
> tree into a JSON string. For instance, if a user executes "DROP TABLE
> t1", the deparser generates from its parse tree:
>
> {DROPSTMT :objects (("t1")) :removeType 41 :behavior 0 :missing_ok
> false :concurrent false}
>
> to:
>
> {"fmt": "DROP TABLE %{objidentity}s", "objidentity": "public.t1"}
>
> This JSON string is self-documenting, meaning someone who gets it can
> easily reconstruct the original DDL with schema-qualified object
> names. In a dedicated event trigger for logical replication, we
> deparsed the parse tree of a DDL, wrote it into a WAL record, and then
> the logical decoding processed it similarly to DML changes.
I think there was also a discussion on whether to use JSON vs the
existing infrastructure of converting nodes to strings. Although the
JSON is standard format and might provide more flexibility using
existing format avoid extra maintence burden.
> While there are several benefits to the JSON data approach mentioned
> in the wiki [1] -- most notably the flexibility to easily remap
> schemas (e.g., mapping "schema A" on the publisher to "schema B" on
> the subscriber) -- there was a major concern: the huge maintenance
> burden. We would need to maintain the JSON serialization code whenever
> creating or modifying parse nodes, regardless of whether the changes
> were related to DDL replication. IIUC, this was the primary reason the
> feature didn't cross the finish line.
Do you mean that modifying any existing parse node requires changing
the JSON serialization code? But why do we need to do that if that's
not related to DDL? I don't think I understood this point clearly,
can you explain it or point me to the discussion thread?
> Additionally, I think there is another design issue: it is not
> output-plugin agnostic. Since the deparsed DDL was written by a
> logical-replication-specific event trigger, third-party logical
> decoding plugins cannot easily detect DDL events. Ideally, we should
> write DDL information into a WAL record natively when
> wal_level='logical' (or additionally when a GUC enables DDL events
> WAL-logging) so that all decoding plugins can detect them. This also
> allows us to test DDL logical decoding with test_decoding without
> setting up a full logical replication subscription.
Yeah, that's a valid point, but I think we could separate the event
trigger logic from the decoding plugins so that it's available for any
other output plugins?
> To address these two points, I'd like to propose an alternative
> approach: we introduce a new data type, say DDLCommand, that is
> self-contained to represent a DDL (like CollectedCommand), and don't
> rely on event triggers. It would have the command type (and subtype if
> required), the OIDs of the target object and its namespace, and the
> OID of the user who executed the DDL. We write it to a new WAL record
> at appropriate places during DDL execution, and the logical decoding
> layer passes the data to output plugins. That way, any logical
> decoding plugin can detect DDL changes, and it's up to the plugins how
> to decode the DDL information.
Interesting. I'm trying to figure out exactly when we plan to
construct this new DDLCommand data type? And how would we prepare
this, by converting the internal DDL structures or from parsetree?
> In pgoutput, for CREATE DDLs, we can use the pg_get_xxx_ddl()
> functions while using a historical snapshot to get the DDLs, saving
> maintenance costs. We would still need to implement the code to
> generate DROP and ALTER DDLs from the data. I believe DROP DDLs would
> not be hard. For ALTER DDLs, we would incur an initial implementation
> cost, but we would not need to change this code often. We can
> implement the DDL generation code in a way that improves ddlutils.c.
>
> Also, because DDLCommand is separated from parse nodes, we only need
> to change the DDL deparse/replication code when it is actually needed.
> Additionally, this approach would eliminate the code around the
> two-step process (using DCT_TableDropStart and DCT_TableDropEnd) for
> DROP TABLE. While it would miss the flexibility benefits that the JSON
> deparsing approach has, I guess it would not be very hard to implement
> the mapping in the deparse layer even without the JSON data.
>
> Regarding the publication syntax, previous patches proposed:
>
> CREATE PUBLICATION pub FOR ALL TABLES WITH (ddl = 'table');
>
> While simple, it doesn't support critical enterprise use cases (e.g.,
> DWH environments) where users want to replicate CREATE and ALTER, but
> explicitly filter out DROP TABLE to prevent accidental data loss. We
> should consider introducing publish_ddl options to filter operations:
>
> CREATE PUBLICATION pub FOR ALL TABLES WITH (publish_ddl = 'create, alter');
+1
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-27 06:14 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
@ 2026-04-28 06:57 ` Amit Kapila <[email protected]>
2026-04-28 19:55 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
1 sibling, 1 reply; 35+ messages in thread
From: Amit Kapila @ 2026-04-28 06:57 UTC (permalink / raw)
To: Dilip Kumar <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Apr 27, 2026 at 11:45 AM Dilip Kumar <[email protected]> wrote:
>
> On Tue, Apr 21, 2026 at 4:45 AM Masahiko Sawada <[email protected]> wrote:
> >
> > The overall idea of the previous patch set was to implement DDL
> > deparsing and utilize it for DDL replication. It converted a parse
> > tree into a JSON string. For instance, if a user executes "DROP TABLE
> > t1", the deparser generates from its parse tree:
> >
> > {DROPSTMT :objects (("t1")) :removeType 41 :behavior 0 :missing_ok
> > false :concurrent false}
> >
> > to:
> >
> > {"fmt": "DROP TABLE %{objidentity}s", "objidentity": "public.t1"}
> >
> > This JSON string is self-documenting, meaning someone who gets it can
> > easily reconstruct the original DDL with schema-qualified object
> > names. In a dedicated event trigger for logical replication, we
> > deparsed the parse tree of a DDL, wrote it into a WAL record, and then
> > the logical decoding processed it similarly to DML changes.
>
> I think there was also a discussion on whether to use JSON vs the
> existing infrastructure of converting nodes to strings. Although the
> JSON is standard format and might provide more flexibility using
> existing format avoid extra maintence burden.
>
Yes, the discussion related to node-to-string and the pros and cons of
the deparse approach are detailed in email [1]. I think some of these
points could be also related to the new approach as well.
[1] - https://www.postgresql.org/message-id/OS0PR01MB571684CBF660D05B63B4412C94AB9%40OS0PR01MB5716.jpnprd0...
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-27 06:14 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-28 06:57 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
@ 2026-04-28 19:55 ` Hannu Krosing <[email protected]>
2026-04-29 03:39 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
0 siblings, 1 reply; 35+ messages in thread
From: Hannu Krosing @ 2026-04-28 19:55 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Dilip Kumar <[email protected]>; Masahiko Sawada <[email protected]>; Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
My high-level understanding is, that we should first clearly answer these
two questions
1. What does Logical Decodng infrastructure make available to the decoding
plugin call-backs
2. What the callbacks themselves do in case of DDL
My answer to the 1. is "everything, as it should be the plugin that decides
what it needs". It does not mean that we should always prepack everything
with special logical decoding structures, but there should be a way to get
at anything that is available in the WAL at least. The result is in WAL,
and for DDL it is also in the system tables themselves, in proper
time-travel way.
For 2. I would prefer to "deparse" the DDL from actual system tables at
that snapshot. In logical decoding the system tables are special in that we
keep the actual table content and have real time travel capabilities on
them. This should allow us to use the code we already have in pg_dump for
extracting the "status quo DDL" meaning the DDL for creating everything
from scratch. The main thing missing is DDL for ALTER and DROP which would
need to be added. But that too should be in the plugin, not in the DDL side
.
On Tue, Apr 28, 2026 at 8:57 AM Amit Kapila <[email protected]> wrote:
> On Mon, Apr 27, 2026 at 11:45 AM Dilip Kumar <[email protected]>
> wrote:
> >
> > On Tue, Apr 21, 2026 at 4:45 AM Masahiko Sawada <[email protected]>
> wrote:
> > >
> > > The overall idea of the previous patch set was to implement DDL
> > > deparsing and utilize it for DDL replication. It converted a parse
> > > tree into a JSON string. For instance, if a user executes "DROP TABLE
> > > t1", the deparser generates from its parse tree:
> > >
> > > {DROPSTMT :objects (("t1")) :removeType 41 :behavior 0 :missing_ok
> > > false :concurrent false}
> > >
> > > to:
> > >
> > > {"fmt": "DROP TABLE %{objidentity}s", "objidentity": "public.t1"}
> > >
> > > This JSON string is self-documenting, meaning someone who gets it can
> > > easily reconstruct the original DDL with schema-qualified object
> > > names. In a dedicated event trigger for logical replication, we
> > > deparsed the parse tree of a DDL, wrote it into a WAL record, and then
> > > the logical decoding processed it similarly to DML changes.
> >
> > I think there was also a discussion on whether to use JSON vs the
> > existing infrastructure of converting nodes to strings. Although the
> > JSON is standard format and might provide more flexibility using
> > existing format avoid extra maintence burden.
> >
>
> Yes, the discussion related to node-to-string and the pros and cons of
> the deparse approach are detailed in email [1]. I think some of these
> points could be also related to the new approach as well.
>
> [1] -
> https://www.postgresql.org/message-id/OS0PR01MB571684CBF660D05B63B4412C94AB9%40OS0PR01MB5716.jpnprd0...
>
> --
> With Regards,
> Amit Kapila.
>
>
>
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-27 06:14 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-28 06:57 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 19:55 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
@ 2026-04-29 03:39 ` Dilip Kumar <[email protected]>
2026-04-29 08:07 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
0 siblings, 1 reply; 35+ messages in thread
From: Dilip Kumar @ 2026-04-29 03:39 UTC (permalink / raw)
To: Hannu Krosing <[email protected]>; +Cc: Amit Kapila <[email protected]>; Masahiko Sawada <[email protected]>; Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Apr 29, 2026 at 1:25 AM Hannu Krosing <[email protected]> wrote:
>
> My high-level understanding is, that we should first clearly answer these two questions
>
> 1. What does Logical Decodng infrastructure make available to the decoding plugin call-backs
>
> 2. What the callbacks themselves do in case of DDL
Yeah that makes sense, Hannu.
> My answer to the 1. is "everything, as it should be the plugin that decides what it needs". It does not mean that we should always prepack everything with special logical decoding structures, but there should be a way to get at anything that is available in the WAL at least. The result is in WAL, and for DDL it is also in the system tables themselves, in proper time-travel way.
>
> For 2. I would prefer to "deparse" the DDL from actual system tables at that snapshot. In logical decoding the system tables are special in that we keep the actual table content and have real time travel capabilities on them. This should allow us to use the code we already have in pg_dump for extracting the "status quo DDL" meaning the DDL for creating everything from scratch. The main thing missing is DDL for ALTER and DROP which would need to be added. But that too should be in the plugin, not in the DDL side .
I am trying to understand your idea. If we are trying to deparse from
an actual system table using a snapshot, why don't we just use the
WAL? I mean, the WAL should contain the actual catalog modifications
it has made. Although converting the catalog changes into a deparse
representation of the DDL could be complex no? Another question is
what we would do with those deparsed representations: will we convert
them to SQL on the subscriber and execute, or do something else?
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-27 06:14 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-28 06:57 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 19:55 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
2026-04-29 03:39 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
@ 2026-04-29 08:07 ` Hannu Krosing <[email protected]>
2026-04-29 11:29 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-29 12:10 ` Re: Support logical replication of DDLs, take2 Andres Freund <[email protected]>
0 siblings, 2 replies; 35+ messages in thread
From: Hannu Krosing @ 2026-04-29 08:07 UTC (permalink / raw)
To: Dilip Kumar <[email protected]>; +Cc: Amit Kapila <[email protected]>; Masahiko Sawada <[email protected]>; Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Apr 29, 2026 at 5:39 AM Dilip Kumar <[email protected]> wrote:
> I am trying to understand your idea. If we are trying to deparse from
> an actual system table using a snapshot, why don't we just use the
> WAL? I mean, the WAL should contain the actual catalog modifications
> it has made.
We have the full data in the catalog and we would likely need catalog
queries for any change, even when de-parsing the tree.
And we should not add the extra load on the original DDL side, just as
we don't for DML.
At most we could just serialize the statement tree into the WAL,
though even that may be an overkill if we can get the change from
existing records.
- insert new row in pg_class --> extract the CREATE TABLE (or INDEX, or ...)
- update row in pg_class or insert, update or delete a row in
pg_attribute --> extract ALTER TABLE
- except when it just updates relfilenod --> extract TRUNCATE
- delete row in pg_class --> DROP TABLE
- dml on pg_constraint --> ALTER TABLE
... etc
> Although converting the catalog changes into a deparse
> representation of the DDL could be complex no?
Both de-parsing the tree and converting the catalog change could be complex.
The advantage of using the catalog is that we already have decades of
experience doing this via pg_dump.
> Another question is
> what we would do with those deparsed representations: will we convert
> them to SQL on the subscriber and execute, or do something else?
Current pg_dump approach is logically equivalent to "doing it on the
subscriber", pg_dump is designed to dump schemas from all older
database versions in format that is compatible with the version the
pg_dump is written for.
This brings us back to the uncomfortable discussion of needing to
back-port some changes to older versions contrary to general
PostgreSQL development principles of adding new features to only the
latest version.
Or we could enable exporting the catalog snapshot from logical
replication stream so that subscriber could use that snapshot in a
"callback connection: to extract the catalog state at that snapshot
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-27 06:14 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-28 06:57 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 19:55 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
2026-04-29 03:39 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-29 08:07 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
@ 2026-04-29 11:29 ` Dilip Kumar <[email protected]>
1 sibling, 0 replies; 35+ messages in thread
From: Dilip Kumar @ 2026-04-29 11:29 UTC (permalink / raw)
To: Hannu Krosing <[email protected]>; +Cc: Amit Kapila <[email protected]>; Masahiko Sawada <[email protected]>; Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Apr 29, 2026 at 1:37 PM Hannu Krosing <[email protected]> wrote:
>
> On Wed, Apr 29, 2026 at 5:39 AM Dilip Kumar <[email protected]> wrote:
> > Another question is
> > what we would do with those deparsed representations: will we convert
> > them to SQL on the subscriber and execute, or do something else?
>
> Current pg_dump approach is logically equivalent to "doing it on the
> subscriber", pg_dump is designed to dump schemas from all older
> database versions in format that is compatible with the version the
> pg_dump is written for.
IIUC, you're suggesting a pg_dump-style mechanism for SQL generation
from the catalog. My concern is that pg_dump is snapshot-based, while
decoding is incremental. So how to we generate SQL from the catalog
for incremental changes(like an ALTER TABLE...SET DATA TYPE)?
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-27 06:14 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-28 06:57 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 19:55 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
2026-04-29 03:39 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-29 08:07 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
@ 2026-04-29 12:10 ` Andres Freund <[email protected]>
2026-04-30 14:02 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
1 sibling, 1 reply; 35+ messages in thread
From: Andres Freund @ 2026-04-29 12:10 UTC (permalink / raw)
To: Hannu Krosing <[email protected]>; +Cc: Dilip Kumar <[email protected]>; Amit Kapila <[email protected]>; Masahiko Sawada <[email protected]>; Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On 2026-04-29 10:07:04 +0200, Hannu Krosing wrote:
> On Wed, Apr 29, 2026 at 5:39 AM Dilip Kumar <[email protected]> wrote:
>
> > I am trying to understand your idea. If we are trying to deparse from
> > an actual system table using a snapshot, why don't we just use the
> > WAL? I mean, the WAL should contain the actual catalog modifications
> > it has made.
>
> We have the full data in the catalog and we would likely need catalog
> queries for any change, even when de-parsing the tree.
>
> And we should not add the extra load on the original DDL side, just as
> we don't for DML.
That can't be a relevant cost compared to everything else.
> At most we could just serialize the statement tree into the WAL,
> though even that may be an overkill if we can get the change from
> existing records.
>
> - insert new row in pg_class --> extract the CREATE TABLE (or INDEX, or ...)
> - update row in pg_class or insert, update or delete a row in
> pg_attribute --> extract ALTER TABLE
> - except when it just updates relfilenod --> extract TRUNCATE
> - delete row in pg_class --> DROP TABLE
> - dml on pg_constraint --> ALTER TABLE
>
> ... etc
That doesn't work in the general case, think of
ALTER TABLE ... ALTER COLUMN ... TYPE foo USING (...)
There's a big difference between USING(foo::int8) and USING (pg_size_bytes(foo))
but it's nowhere visible in the WAL.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-27 06:14 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-28 06:57 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 19:55 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
2026-04-29 03:39 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-29 08:07 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
2026-04-29 12:10 ` Re: Support logical replication of DDLs, take2 Andres Freund <[email protected]>
@ 2026-04-30 14:02 ` Hannu Krosing <[email protected]>
2026-05-01 18:40 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 35+ messages in thread
From: Hannu Krosing @ 2026-04-30 14:02 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Dilip Kumar <[email protected]>; Amit Kapila <[email protected]>; Masahiko Sawada <[email protected]>; Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Apr 29, 2026 at 2:10 PM Andres Freund <[email protected]> wrote:
>
> Hi,
>
> On 2026-04-29 10:07:04 +0200, Hannu Krosing wrote:
> > On Wed, Apr 29, 2026 at 5:39 AM Dilip Kumar <[email protected]> wrote:
> >
> > > I am trying to understand your idea. If we are trying to deparse from
> > > an actual system table using a snapshot, why don't we just use the
> > > WAL? I mean, the WAL should contain the actual catalog modifications
> > > it has made.
> >
> > We have the full data in the catalog and we would likely need catalog
> > queries for any change, even when de-parsing the tree.
> >
> > And we should not add the extra load on the original DDL side, just as
> > we don't for DML.
>
> That can't be a relevant cost compared to everything else.
Probably not. But unless we somehow encode "everything" at that point
we will make building different DDL decoders harder down the line.
So why not just save the normally serialised parse tree at this point
and let the decoders decide to do whatever they need.
> > At most we could just serialize the statement tree into the WAL,
> > though even that may be an overkill if we can get the change from
> > existing records.
> >
> > - insert new row in pg_class --> extract the CREATE TABLE (or INDEX, or ...)
> > - update row in pg_class or insert, update or delete a row in
> > pg_attribute --> extract ALTER TABLE
> > - except when it just updates relfilenod --> extract TRUNCATE
> > - delete row in pg_class --> DROP TABLE
> > - dml on pg_constraint --> ALTER TABLE
> >
> > ... etc
>
> That doesn't work in the general case, think of
> ALTER TABLE ... ALTER COLUMN ... TYPE foo USING (...)
>
> There's a big difference between USING(foo::int8) and USING (pg_size_bytes(foo))
> but it's nowhere visible in the WAL.
It can't be a big difference if it is not visible in the WAL.
Currently, we do treat DML exactly this way (or arguably worse).
In the following all the updates are decoded exactly the same
CREATE TABLE t(id int primary key, data text);
INSERT INTO t VALUES(1, 'one');
UPDATE t SET data='one' where id=1;
UPDATE t SET id=id;
UPDATE t SET id=10-9;
UPDATE t SET data='one';
ALL of the above get decoded as "UPDATE t SET data='one' where id=1;"
That is, we do not care how the values got there, as long as the end
result is the same.
And we do not track which fields were actually changed
The only reasons I see why we could not do the same for DDL are
1. it would be significantly more expensive to do so
or
2. we plan to fix some of that for DML as well and to start tracking
more of the intent in DML by extracting that from the statement trees.
--
Hannu
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-27 06:14 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-28 06:57 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 19:55 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
2026-04-29 03:39 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-29 08:07 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
2026-04-29 12:10 ` Re: Support logical replication of DDLs, take2 Andres Freund <[email protected]>
2026-04-30 14:02 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
@ 2026-05-01 18:40 ` Masahiko Sawada <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Masahiko Sawada @ 2026-05-01 18:40 UTC (permalink / raw)
To: Hannu Krosing <[email protected]>; +Cc: Andres Freund <[email protected]>; Dilip Kumar <[email protected]>; Amit Kapila <[email protected]>; Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, Apr 30, 2026 at 7:03 AM Hannu Krosing <[email protected]> wrote:
>
> On Wed, Apr 29, 2026 at 2:10 PM Andres Freund <[email protected]> wrote:
> >
> > Hi,
> >
> > On 2026-04-29 10:07:04 +0200, Hannu Krosing wrote:
> > > On Wed, Apr 29, 2026 at 5:39 AM Dilip Kumar <[email protected]> wrote:
> > >
> > > > I am trying to understand your idea. If we are trying to deparse from
> > > > an actual system table using a snapshot, why don't we just use the
> > > > WAL? I mean, the WAL should contain the actual catalog modifications
> > > > it has made.
> > >
> > > We have the full data in the catalog and we would likely need catalog
> > > queries for any change, even when de-parsing the tree.
> > >
> > > And we should not add the extra load on the original DDL side, just as
> > > we don't for DML.
> >
> > That can't be a relevant cost compared to everything else.
>
> Probably not. But unless we somehow encode "everything" at that point
> we will make building different DDL decoders harder down the line.
>
> So why not just save the normally serialised parse tree at this point
> and let the decoders decide to do whatever they need.
>
> > > At most we could just serialize the statement tree into the WAL,
> > > though even that may be an overkill if we can get the change from
> > > existing records.
> > >
> > > - insert new row in pg_class --> extract the CREATE TABLE (or INDEX, or ...)
> > > - update row in pg_class or insert, update or delete a row in
> > > pg_attribute --> extract ALTER TABLE
> > > - except when it just updates relfilenod --> extract TRUNCATE
> > > - delete row in pg_class --> DROP TABLE
> > > - dml on pg_constraint --> ALTER TABLE
> > >
> > > ... etc
> >
> > That doesn't work in the general case, think of
> > ALTER TABLE ... ALTER COLUMN ... TYPE foo USING (...)
> >
> > There's a big difference between USING(foo::int8) and USING (pg_size_bytes(foo))
> > but it's nowhere visible in the WAL.
>
> It can't be a big difference if it is not visible in the WAL.
If we send the rewritten tuples made during ALTER TABLE execution via
logical replication, there would not be a big difference. However, if
we send only the re-constructed ALTER TABLE statement, there is. I
think that replicating ALTER TABLE should behave the latter because we
might not need table rewrites in more ALTER TABLE cases in newer
PostgreSQL versions.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-27 06:14 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
@ 2026-04-28 21:38 ` Masahiko Sawada <[email protected]>
1 sibling, 0 replies; 35+ messages in thread
From: Masahiko Sawada @ 2026-04-28 21:38 UTC (permalink / raw)
To: Dilip Kumar <[email protected]>; +Cc: Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sun, Apr 26, 2026 at 11:15 PM Dilip Kumar <[email protected]> wrote:
>
> On Tue, Apr 21, 2026 at 4:45 AM Masahiko Sawada <[email protected]> wrote:
> >
>
> > We made a lot of effort on this feature through 2022 and 2023, but the
> > development is currently inactive. The last patch was submitted on Jul
> > 18, 2023. I've reviewed the previous patches and discussions, and I
> > would like to summarize how DDL replication was implemented, the main
> > reasons it stalled, and propose an alternative design to address those
> > problems.
> >
> > The overall idea of the previous patch set was to implement DDL
> > deparsing and utilize it for DDL replication. It converted a parse
> > tree into a JSON string. For instance, if a user executes "DROP TABLE
> > t1", the deparser generates from its parse tree:
> >
> > {DROPSTMT :objects (("t1")) :removeType 41 :behavior 0 :missing_ok
> > false :concurrent false}
> >
> > to:
> >
> > {"fmt": "DROP TABLE %{objidentity}s", "objidentity": "public.t1"}
> >
> > This JSON string is self-documenting, meaning someone who gets it can
> > easily reconstruct the original DDL with schema-qualified object
> > names. In a dedicated event trigger for logical replication, we
> > deparsed the parse tree of a DDL, wrote it into a WAL record, and then
> > the logical decoding processed it similarly to DML changes.
>
> I think there was also a discussion on whether to use JSON vs the
> existing infrastructure of converting nodes to strings. Although the
> JSON is standard format and might provide more flexibility using
> existing format avoid extra maintence burden.
Right, but since the string representation of nodes are major version
dependent, we cannot directly send them to subscribers that might be
different major versions.
>
> > While there are several benefits to the JSON data approach mentioned
> > in the wiki [1] -- most notably the flexibility to easily remap
> > schemas (e.g., mapping "schema A" on the publisher to "schema B" on
> > the subscriber) -- there was a major concern: the huge maintenance
> > burden. We would need to maintain the JSON serialization code whenever
> > creating or modifying parse nodes, regardless of whether the changes
> > were related to DDL replication. IIUC, this was the primary reason the
> > feature didn't cross the finish line.
>
> Do you mean that modifying any existing parse node requires changing
> the JSON serialization code? But why do we need to do that if that's
> not related to DDL? I don't think I understood this point clearly,
> can you explain it or point me to the discussion thread?
IIUC, many parse nodes need to support JSON serialization even if we
only want to support CREATE/ALTER/DROP TABLE commands. This is because
these commands can include expressions (e.g., in DEFAULT clauses or
CHECK constraints), function calls, and column references. We would
need to recursively deparse the entire expression tree.
>
> > Additionally, I think there is another design issue: it is not
> > output-plugin agnostic. Since the deparsed DDL was written by a
> > logical-replication-specific event trigger, third-party logical
> > decoding plugins cannot easily detect DDL events. Ideally, we should
> > write DDL information into a WAL record natively when
> > wal_level='logical' (or additionally when a GUC enables DDL events
> > WAL-logging) so that all decoding plugins can detect them. This also
> > allows us to test DDL logical decoding with test_decoding without
> > setting up a full logical replication subscription.
>
> Yeah, that's a valid point, but I think we could separate the event
> trigger logic from the decoding plugins so that it's available for any
> other output plugins?
Yes, it's possible. But I'm concerned that the operations users (or
plugins) would need to enable DDL events would be quite different from
capturing DMLs. I'm not sure if it's a good user experience or plugin
developer experience that additional steps are required to capture DDL
events while changes of INSERT/UPDATE/DELETE/TRUNCATE are passed from
the logical decoding by default.
>
> > To address these two points, I'd like to propose an alternative
> > approach: we introduce a new data type, say DDLCommand, that is
> > self-contained to represent a DDL (like CollectedCommand), and don't
> > rely on event triggers. It would have the command type (and subtype if
> > required), the OIDs of the target object and its namespace, and the
> > OID of the user who executed the DDL. We write it to a new WAL record
> > at appropriate places during DDL execution, and the logical decoding
> > layer passes the data to output plugins. That way, any logical
> > decoding plugin can detect DDL changes, and it's up to the plugins how
> > to decode the DDL information.
>
> Interesting. I'm trying to figure out exactly when we plan to
> construct this new DDLCommand data type? And how would we prepare
> this, by converting the internal DDL structures or from parsetree?
I'm still unsure what is the best approach but I think we can
construct the DDLCommand data from the internal DDL structures, and
each DDL command can call the function to write the DDL command
information to a WAL record.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
@ 2026-04-28 06:32 ` Amit Kapila <[email protected]>
2026-04-28 21:48 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
1 sibling, 1 reply; 35+ messages in thread
From: Amit Kapila @ 2026-04-28 06:32 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Apr 21, 2026 at 4:45 AM Masahiko Sawada <[email protected]> wrote:
>
> On Mon, Feb 23, 2026 at 5:21 PM Masahiko Sawada <[email protected]> wrote:
> >
> >
> > One idea I'm experimenting with is that we define an abstract data
> > type that can represent a DDL (like CollectedCommand) and write it to
> > a new WAL record so that logical decoding processes it. For CREATE
> > DDLs, we can use pg_get_xxx_def() function while using a historical
> > snapshot to get the DDLs. We would need to implement the codes to
> > generate DROP and ALTER DDLs from the data. I believe DROP DDLs would
> > not be hard. For ALTER DDLs, we would incur the initial implementation
> > costs, but we would not change these codes often.
> >
>
> DDL support for logical replication is one of the biggest missing
> pieces in logical replication. I'd like to resume this work for PG20.
>
> We made a lot of effort on this feature through 2022 and 2023, but the
> development is currently inactive. The last patch was submitted on Jul
> 18, 2023. I've reviewed the previous patches and discussions, and I
> would like to summarize how DDL replication was implemented, the main
> reasons it stalled, and propose an alternative design to address those
> problems.
>
> The overall idea of the previous patch set was to implement DDL
> deparsing and utilize it for DDL replication. It converted a parse
> tree into a JSON string. For instance, if a user executes "DROP TABLE
> t1", the deparser generates from its parse tree:
>
> {DROPSTMT :objects (("t1")) :removeType 41 :behavior 0 :missing_ok
> false :concurrent false}
>
> to:
>
> {"fmt": "DROP TABLE %{objidentity}s", "objidentity": "public.t1"}
>
> This JSON string is self-documenting, meaning someone who gets it can
> easily reconstruct the original DDL with schema-qualified object
> names. In a dedicated event trigger for logical replication, we
> deparsed the parse tree of a DDL, wrote it into a WAL record, and then
> the logical decoding processed it similarly to DML changes.
>
> While there are several benefits to the JSON data approach mentioned
> in the wiki [1] -- most notably the flexibility to easily remap
> schemas (e.g., mapping "schema A" on the publisher to "schema B" on
> the subscriber) -- there was a major concern: the huge maintenance
> burden.
>
Yes, there will be a maintenance cost of JSON-based deparsing
approach. But note that multiple senior people (Alvaro Herrera, Robert
Haas) [1] seems to favor that approach. So, I am not sure we can
conclude to abandon that approach without those people or some other
senior people agreeing to abandon it. To be clear, I am not against
considering a new/different approach for DDL replication but just that
it is not clear that old/existing approach can be ruled out without
more discussion on it,
We would need to maintain the JSON serialization code whenever
> creating or modifying parse nodes, regardless of whether the changes
> were related to DDL replication. IIUC, this was the primary reason the
> feature didn't cross the finish line.
>
> Additionally, I think there is another design issue: it is not
> output-plugin agnostic. Since the deparsed DDL was written by a
> logical-replication-specific event trigger, third-party logical
> decoding plugins cannot easily detect DDL events.
>
Why RmgrId like RM_LOGICALDDLMSG_ID and XLOG_LOGICAL_DDL_MESSAGE wal
info is not sufficient for this? Decoder will add a message like
REORDER_BUFFER_CHANGE_DDL which can be used to detect DDL message, no?
> Ideally, we should
> write DDL information into a WAL record natively when
> wal_level='logical' (or additionally when a GUC enables DDL events
> WAL-logging) so that all decoding plugins can detect them. This also
> allows us to test DDL logical decoding with test_decoding without
> setting up a full logical replication subscription.
>
> To address these two points, I'd like to propose an alternative
> approach: we introduce a new data type, say DDLCommand, that is
> self-contained to represent a DDL (like CollectedCommand), and don't
> rely on event triggers. It would have the command type (and subtype if
> required), the OIDs of the target object and its namespace, and the
> OID of the user who executed the DDL. We write it to a new WAL record
> at appropriate places during DDL execution, and the logical decoding
> layer passes the data to output plugins. That way, any logical
> decoding plugin can detect DDL changes, and it's up to the plugins how
> to decode the DDL information.
>
> In pgoutput, for CREATE DDLs, we can use the pg_get_xxx_ddl()
> functions while using a historical snapshot to get the DDLs, saving
> maintenance costs. We would still need to implement the code to
> generate DROP and ALTER DDLs from the data. I believe DROP DDLs would
> not be hard. For ALTER DDLs, we would incur an initial implementation
> cost, but we would not need to change this code often. We can
> implement the DDL generation code in a way that improves ddlutils.c.
>
> Also, because DDLCommand is separated from parse nodes, we only need
> to change the DDL deparse/replication code when it is actually needed.
> Additionally, this approach would eliminate the code around the
> two-step process (using DCT_TableDropStart and DCT_TableDropEnd) for
> DROP TABLE. While it would miss the flexibility benefits that the JSON
> deparsing approach has, I guess it would not be very hard to implement
> the mapping in the deparse layer even without the JSON data.
>
Possible but the point was flexibility and ease with which users can
implement mapping with JSON approach.
>
> FYI I've experimented with auto-generation approaches too. For
> instance, gen_node_support.pl generates C code that converts parse
> nodes to the corresponding text representations. Or
> gen_node_support.pl generates C code that makes all objects in the
> given SQL query text fully-schema qualified. While these ideas are
> promising they didn't help reduce the maintenance burden much as the
> parse node definitions are already complex and vary on nodes much.
>
Yeah, this is my recollection of a previous attempt for
auto_generating the deparsing code.
[1]: https://www.postgresql.org/message-id/CA%2BTgmoauXRQ3yDZNGTzXv_m1kdUnH1Ww%2BhwKmKUSjtyBh0Em2Q%40mail...
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-28 06:32 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
@ 2026-04-28 21:48 ` Masahiko Sawada <[email protected]>
2026-04-30 04:44 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
0 siblings, 1 reply; 35+ messages in thread
From: Masahiko Sawada @ 2026-04-28 21:48 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Apr 27, 2026 at 11:32 PM Amit Kapila <[email protected]> wrote:
>
> On Tue, Apr 21, 2026 at 4:45 AM Masahiko Sawada <[email protected]> wrote:
> >
> > On Mon, Feb 23, 2026 at 5:21 PM Masahiko Sawada <[email protected]> wrote:
> > >
> > >
> > > One idea I'm experimenting with is that we define an abstract data
> > > type that can represent a DDL (like CollectedCommand) and write it to
> > > a new WAL record so that logical decoding processes it. For CREATE
> > > DDLs, we can use pg_get_xxx_def() function while using a historical
> > > snapshot to get the DDLs. We would need to implement the codes to
> > > generate DROP and ALTER DDLs from the data. I believe DROP DDLs would
> > > not be hard. For ALTER DDLs, we would incur the initial implementation
> > > costs, but we would not change these codes often.
> > >
> >
> > DDL support for logical replication is one of the biggest missing
> > pieces in logical replication. I'd like to resume this work for PG20.
> >
> > We made a lot of effort on this feature through 2022 and 2023, but the
> > development is currently inactive. The last patch was submitted on Jul
> > 18, 2023. I've reviewed the previous patches and discussions, and I
> > would like to summarize how DDL replication was implemented, the main
> > reasons it stalled, and propose an alternative design to address those
> > problems.
> >
> > The overall idea of the previous patch set was to implement DDL
> > deparsing and utilize it for DDL replication. It converted a parse
> > tree into a JSON string. For instance, if a user executes "DROP TABLE
> > t1", the deparser generates from its parse tree:
> >
> > {DROPSTMT :objects (("t1")) :removeType 41 :behavior 0 :missing_ok
> > false :concurrent false}
> >
> > to:
> >
> > {"fmt": "DROP TABLE %{objidentity}s", "objidentity": "public.t1"}
> >
> > This JSON string is self-documenting, meaning someone who gets it can
> > easily reconstruct the original DDL with schema-qualified object
> > names. In a dedicated event trigger for logical replication, we
> > deparsed the parse tree of a DDL, wrote it into a WAL record, and then
> > the logical decoding processed it similarly to DML changes.
> >
> > While there are several benefits to the JSON data approach mentioned
> > in the wiki [1] -- most notably the flexibility to easily remap
> > schemas (e.g., mapping "schema A" on the publisher to "schema B" on
> > the subscriber) -- there was a major concern: the huge maintenance
> > burden.
> >
>
> Yes, there will be a maintenance cost of JSON-based deparsing
> approach. But note that multiple senior people (Alvaro Herrera, Robert
> Haas) [1] seems to favor that approach. So, I am not sure we can
> conclude to abandon that approach without those people or some other
> senior people agreeing to abandon it. To be clear, I am not against
> considering a new/different approach for DDL replication but just that
> it is not clear that old/existing approach can be ruled out without
> more discussion on it,
Thank you for pointing it out. Just to be clear, IIUC what they liked
was to use JSON string representation of DDLs, but not JSON string
representation of DDLs that are deparsed from parse nodes, no? I think
if we do versioning the DDL commands sent to subscribers, we can
support JSON-based DDLs in later versions.
>
> We would need to maintain the JSON serialization code whenever
> > creating or modifying parse nodes, regardless of whether the changes
> > were related to DDL replication. IIUC, this was the primary reason the
> > feature didn't cross the finish line.
> >
> > Additionally, I think there is another design issue: it is not
> > output-plugin agnostic. Since the deparsed DDL was written by a
> > logical-replication-specific event trigger, third-party logical
> > decoding plugins cannot easily detect DDL events.
> >
>
> Why RmgrId like RM_LOGICALDDLMSG_ID and XLOG_LOGICAL_DDL_MESSAGE wal
> info is not sufficient for this? Decoder will add a message like
> REORDER_BUFFER_CHANGE_DDL which can be used to detect DDL message, no?
Right, but I'm not sure this is a good developer experience that
additional steps are required to capture DDL events for other plugins
while changes of INSERT/UPDATE/DELETE/TRUNCATE are passed from the
logical decoding by default.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-28 06:32 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 21:48 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
@ 2026-04-30 04:44 ` Amit Kapila <[email protected]>
2026-04-30 20:40 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 35+ messages in thread
From: Amit Kapila @ 2026-04-30 04:44 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Apr 29, 2026 at 3:19 AM Masahiko Sawada <[email protected]> wrote:
>
> On Mon, Apr 27, 2026 at 11:32 PM Amit Kapila <[email protected]> wrote:
> >
> > Yes, there will be a maintenance cost of JSON-based deparsing
> > approach. But note that multiple senior people (Alvaro Herrera, Robert
> > Haas) [1] seems to favor that approach. So, I am not sure we can
> > conclude to abandon that approach without those people or some other
> > senior people agreeing to abandon it. To be clear, I am not against
> > considering a new/different approach for DDL replication but just that
> > it is not clear that old/existing approach can be ruled out without
> > more discussion on it,
>
> Thank you for pointing it out. Just to be clear, IIUC what they liked
> was to use JSON string representation of DDLs, but not JSON string
> representation of DDLs that are deparsed from parse nodes, no?
>
As per my understanding, we built deparsing stuff with a goal of
supporting DDL replication and Alvaro was the original author of that
work, see [1]. The benefit it provides flexibility in terms of
filtering by decoding plugin, if any, or changing the DDL (like
schema-mapping) during apply. It is not clear to me if we can achive
similar level of flexibility with other approach.
>
> >
> > We would need to maintain the JSON serialization code whenever
> > > creating or modifying parse nodes, regardless of whether the changes
> > > were related to DDL replication. IIUC, this was the primary reason the
> > > feature didn't cross the finish line.
> > >
> > > Additionally, I think there is another design issue: it is not
> > > output-plugin agnostic. Since the deparsed DDL was written by a
> > > logical-replication-specific event trigger, third-party logical
> > > decoding plugins cannot easily detect DDL events.
> > >
> >
> > Why RmgrId like RM_LOGICALDDLMSG_ID and XLOG_LOGICAL_DDL_MESSAGE wal
> > info is not sufficient for this? Decoder will add a message like
> > REORDER_BUFFER_CHANGE_DDL which can be used to detect DDL message, no?
>
> Right, but I'm not sure this is a good developer experience that
> additional steps are required to capture DDL events for other plugins
> while changes of INSERT/UPDATE/DELETE/TRUNCATE are passed from the
> logical decoding by default.
>
Yes, there could probably be additional steps for plugins but they
must be doing a few things already which are defined at publication
level like column lists, row filtering, something related to RI, etc.
To reduce the plugin work, one naive idea is to let the event triggers
be registered at first logical slot creation or may be at init time of
plugin.
[1]: https://www.postgresql.org/message-id/202203162206.7spggyktx63e%40alvherre.pgsql
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-28 06:32 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 21:48 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-30 04:44 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
@ 2026-04-30 20:40 ` Masahiko Sawada <[email protected]>
2026-05-04 12:23 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
0 siblings, 1 reply; 35+ messages in thread
From: Masahiko Sawada @ 2026-04-30 20:40 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Apr 29, 2026 at 9:44 PM Amit Kapila <[email protected]> wrote:
>
> On Wed, Apr 29, 2026 at 3:19 AM Masahiko Sawada <[email protected]> wrote:
> >
> > On Mon, Apr 27, 2026 at 11:32 PM Amit Kapila <[email protected]> wrote:
> > >
> > > Yes, there will be a maintenance cost of JSON-based deparsing
> > > approach. But note that multiple senior people (Alvaro Herrera, Robert
> > > Haas) [1] seems to favor that approach. So, I am not sure we can
> > > conclude to abandon that approach without those people or some other
> > > senior people agreeing to abandon it. To be clear, I am not against
> > > considering a new/different approach for DDL replication but just that
> > > it is not clear that old/existing approach can be ruled out without
> > > more discussion on it,
> >
> > Thank you for pointing it out. Just to be clear, IIUC what they liked
> > was to use JSON string representation of DDLs, but not JSON string
> > representation of DDLs that are deparsed from parse nodes, no?
> >
>
> As per my understanding, we built deparsing stuff with a goal of
> supporting DDL replication and Alvaro was the original author of that
> work, see [1]. The benefit it provides flexibility in terms of
> filtering by decoding plugin, if any, or changing the DDL (like
> schema-mapping) during apply. It is not clear to me if we can achive
> similar level of flexibility with other approach.
I think we can generate the same JSON-string representation of DDLs
from catalog information, it would also require a lot of code, though.
It would be independent from parse nodes and if we implement it as an
option for pg_get_xxx_ddl() functionality it would be able to be
reused by other tools too.
>
> >
> > >
> > > We would need to maintain the JSON serialization code whenever
> > > > creating or modifying parse nodes, regardless of whether the changes
> > > > were related to DDL replication. IIUC, this was the primary reason the
> > > > feature didn't cross the finish line.
> > > >
> > > > Additionally, I think there is another design issue: it is not
> > > > output-plugin agnostic. Since the deparsed DDL was written by a
> > > > logical-replication-specific event trigger, third-party logical
> > > > decoding plugins cannot easily detect DDL events.
> > > >
> > >
> > > Why RmgrId like RM_LOGICALDDLMSG_ID and XLOG_LOGICAL_DDL_MESSAGE wal
> > > info is not sufficient for this? Decoder will add a message like
> > > REORDER_BUFFER_CHANGE_DDL which can be used to detect DDL message, no?
> >
> > Right, but I'm not sure this is a good developer experience that
> > additional steps are required to capture DDL events for other plugins
> > while changes of INSERT/UPDATE/DELETE/TRUNCATE are passed from the
> > logical decoding by default.
> >
>
> Yes, there could probably be additional steps for plugins but they
> must be doing a few things already which are defined at publication
> level like column lists, row filtering, something related to RI, etc.
I think those publication-level features operate at a somewhat
different layer than the fundamental mechanism of capturing DDL
events. Plugins filter rows or columns based on configuration, but the
logical decoding itself guarantees that the DML events are reliably
passed to them. Given that the TRUNCATE in logical replication already
works so, I guess DDL should have the same fundamental guarantee.
it's unclear to me how plugins could reliably manage these event
triggers. While a plugin might create an event trigger during the
startup callback if it doesn't exist, it cannot drop it during the
shutdown callback. We also cannot establish a dependency between an
event trigger and a logical replication slot. We would likely need to
invent a new plugin callback specifically invoked at slot drop time
just to clean it up. Also, if different plugins want to capture DDL
events, they could end up registering different event triggers,
emitting multiple DDL WAL records for the same DDL event.
> To reduce the plugin work, one naive idea is to let the event triggers
> be registered at first logical slot creation or may be at init time of
> plugin.
I think we need to note that replication slot creation and drop are
non-transactional operations. We need to make sure that both logical
slots and the event trigger are not orphaned in error or server crash
cases.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-28 06:32 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 21:48 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-30 04:44 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-30 20:40 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
@ 2026-05-04 12:23 ` Amit Kapila <[email protected]>
2026-05-05 17:36 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 35+ messages in thread
From: Amit Kapila @ 2026-05-04 12:23 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, May 1, 2026 at 2:11 AM Masahiko Sawada <[email protected]> wrote:
>
> On Wed, Apr 29, 2026 at 9:44 PM Amit Kapila <[email protected]> wrote:
> >
> > On Wed, Apr 29, 2026 at 3:19 AM Masahiko Sawada <[email protected]> wrote:
> > >
> > > On Mon, Apr 27, 2026 at 11:32 PM Amit Kapila <[email protected]> wrote:
> > > >
> > > > Yes, there will be a maintenance cost of JSON-based deparsing
> > > > approach. But note that multiple senior people (Alvaro Herrera, Robert
> > > > Haas) [1] seems to favor that approach. So, I am not sure we can
> > > > conclude to abandon that approach without those people or some other
> > > > senior people agreeing to abandon it. To be clear, I am not against
> > > > considering a new/different approach for DDL replication but just that
> > > > it is not clear that old/existing approach can be ruled out without
> > > > more discussion on it,
> > >
> > > Thank you for pointing it out. Just to be clear, IIUC what they liked
> > > was to use JSON string representation of DDLs, but not JSON string
> > > representation of DDLs that are deparsed from parse nodes, no?
> > >
> >
> > As per my understanding, we built deparsing stuff with a goal of
> > supporting DDL replication and Alvaro was the original author of that
> > work, see [1]. The benefit it provides flexibility in terms of
> > filtering by decoding plugin, if any, or changing the DDL (like
> > schema-mapping) during apply. It is not clear to me if we can achive
> > similar level of flexibility with other approach.
>
> I think we can generate the same JSON-string representation of DDLs
> from catalog information, it would also require a lot of code, though.
> It would be independent from parse nodes and if we implement it as an
> option for pg_get_xxx_ddl() functionality it would be able to be
> reused by other tools too.
>
IIRC, this was discussed previously as well but we were not sure if we
can build all (especially some complex ones) without parsetree. See
discussion/emails around [1][2].
> >
> > >
> > > >
> > > > We would need to maintain the JSON serialization code whenever
> > > > > creating or modifying parse nodes, regardless of whether the changes
> > > > > were related to DDL replication. IIUC, this was the primary reason the
> > > > > feature didn't cross the finish line.
> > > > >
> > > > > Additionally, I think there is another design issue: it is not
> > > > > output-plugin agnostic. Since the deparsed DDL was written by a
> > > > > logical-replication-specific event trigger, third-party logical
> > > > > decoding plugins cannot easily detect DDL events.
> > > > >
> > > >
> > > > Why RmgrId like RM_LOGICALDDLMSG_ID and XLOG_LOGICAL_DDL_MESSAGE wal
> > > > info is not sufficient for this? Decoder will add a message like
> > > > REORDER_BUFFER_CHANGE_DDL which can be used to detect DDL message, no?
> > >
> > > Right, but I'm not sure this is a good developer experience that
> > > additional steps are required to capture DDL events for other plugins
> > > while changes of INSERT/UPDATE/DELETE/TRUNCATE are passed from the
> > > logical decoding by default.
> > >
> >
> > Yes, there could probably be additional steps for plugins but they
> > must be doing a few things already which are defined at publication
> > level like column lists, row filtering, something related to RI, etc.
>
> I think those publication-level features operate at a somewhat
> different layer than the fundamental mechanism of capturing DDL
> events. Plugins filter rows or columns based on configuration, but the
> logical decoding itself guarantees that the DML events are reliably
> passed to them. Given that the TRUNCATE in logical replication already
> works so, I guess DDL should have the same fundamental guarantee.
>
> it's unclear to me how plugins could reliably manage these event
> triggers. While a plugin might create an event trigger during the
> startup callback if it doesn't exist, it cannot drop it during the
> shutdown callback. We also cannot establish a dependency between an
> event trigger and a logical replication slot. We would likely need to
> invent a new plugin callback specifically invoked at slot drop time
> just to clean it up. Also, if different plugins want to capture DDL
> events, they could end up registering different event triggers,
> emitting multiple DDL WAL records for the same DDL event.
>
Why would different plugins end up registering different event
triggers? I mean if they are already registered by the first plugin
what is the need to re-register.
[1] - https://www.postgresql.org/message-id/[email protected]
[2] - https://www.postgresql.org/message-id/[email protected]
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-28 06:32 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 21:48 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-30 04:44 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-30 20:40 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-05-04 12:23 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
@ 2026-05-05 17:36 ` Masahiko Sawada <[email protected]>
2026-06-11 07:07 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 35+ messages in thread
From: Masahiko Sawada @ 2026-05-05 17:36 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, May 4, 2026 at 5:23 AM Amit Kapila <[email protected]> wrote:
>
> On Fri, May 1, 2026 at 2:11 AM Masahiko Sawada <[email protected]> wrote:
> >
> > On Wed, Apr 29, 2026 at 9:44 PM Amit Kapila <[email protected]> wrote:
> > >
> > > On Wed, Apr 29, 2026 at 3:19 AM Masahiko Sawada <[email protected]> wrote:
> > > >
> > > > On Mon, Apr 27, 2026 at 11:32 PM Amit Kapila <[email protected]> wrote:
> > > > >
> > > > > Yes, there will be a maintenance cost of JSON-based deparsing
> > > > > approach. But note that multiple senior people (Alvaro Herrera, Robert
> > > > > Haas) [1] seems to favor that approach. So, I am not sure we can
> > > > > conclude to abandon that approach without those people or some other
> > > > > senior people agreeing to abandon it. To be clear, I am not against
> > > > > considering a new/different approach for DDL replication but just that
> > > > > it is not clear that old/existing approach can be ruled out without
> > > > > more discussion on it,
> > > >
> > > > Thank you for pointing it out. Just to be clear, IIUC what they liked
> > > > was to use JSON string representation of DDLs, but not JSON string
> > > > representation of DDLs that are deparsed from parse nodes, no?
> > > >
> > >
> > > As per my understanding, we built deparsing stuff with a goal of
> > > supporting DDL replication and Alvaro was the original author of that
> > > work, see [1]. The benefit it provides flexibility in terms of
> > > filtering by decoding plugin, if any, or changing the DDL (like
> > > schema-mapping) during apply. It is not clear to me if we can achive
> > > similar level of flexibility with other approach.
> >
> > I think we can generate the same JSON-string representation of DDLs
> > from catalog information, it would also require a lot of code, though.
> > It would be independent from parse nodes and if we implement it as an
> > option for pg_get_xxx_ddl() functionality it would be able to be
> > reused by other tools too.
> >
>
> IIRC, this was discussed previously as well but we were not sure if we
> can build all (especially some complex ones) without parsetree. See
> discussion/emails around [1][2].
Right. We would need parsetree somewhat. I think we're able to
generate CREATE and DROP TABLE statements for the particular table
without parsetree. But as for generating CREATE TABLE for the table,
it's going to be a combination of CREATE/ALTER TABLE/INDEX/SEQUENCE
statements, like pg_dump does. For instance, if a user executes
"CREATE TABLE foo (id serial primary key)", we create table, sequence,
and index, but searching system catalogs doesn't tell us these objects
are created in one statement. So we would generate multiple DDLs as
follow:
CREATE TABLE public.foo (id integer NOT NULL);
CREATE SEQUENCE public.foo_id_seq AS integer ...;
ALTER SEQUENCE public.foo OWNED BY foo;
ALTER TABLE public.foo ADD CONSTRAINT foo_pkey PRIMARY KEY (id);
While these queries create the same table as the one on the publisher,
we need to consider whether it's okay to replicate these queries
instead of the oen statement originally executed on the publisher. If
we can use something like pg_get_table_ddl() in DDL replication, that
function would be able to be used also by the initial schema
synchronization.
As for ALTER TABLE, we would need parsetree of ALTER TABLE subcommands.
>
> > >
> > > >
> > > > >
> > > > > We would need to maintain the JSON serialization code whenever
> > > > > > creating or modifying parse nodes, regardless of whether the changes
> > > > > > were related to DDL replication. IIUC, this was the primary reason the
> > > > > > feature didn't cross the finish line.
> > > > > >
> > > > > > Additionally, I think there is another design issue: it is not
> > > > > > output-plugin agnostic. Since the deparsed DDL was written by a
> > > > > > logical-replication-specific event trigger, third-party logical
> > > > > > decoding plugins cannot easily detect DDL events.
> > > > > >
> > > > >
> > > > > Why RmgrId like RM_LOGICALDDLMSG_ID and XLOG_LOGICAL_DDL_MESSAGE wal
> > > > > info is not sufficient for this? Decoder will add a message like
> > > > > REORDER_BUFFER_CHANGE_DDL which can be used to detect DDL message, no?
> > > >
> > > > Right, but I'm not sure this is a good developer experience that
> > > > additional steps are required to capture DDL events for other plugins
> > > > while changes of INSERT/UPDATE/DELETE/TRUNCATE are passed from the
> > > > logical decoding by default.
> > > >
> > >
> > > Yes, there could probably be additional steps for plugins but they
> > > must be doing a few things already which are defined at publication
> > > level like column lists, row filtering, something related to RI, etc.
> >
> > I think those publication-level features operate at a somewhat
> > different layer than the fundamental mechanism of capturing DDL
> > events. Plugins filter rows or columns based on configuration, but the
> > logical decoding itself guarantees that the DML events are reliably
> > passed to them. Given that the TRUNCATE in logical replication already
> > works so, I guess DDL should have the same fundamental guarantee.
> >
> > it's unclear to me how plugins could reliably manage these event
> > triggers. While a plugin might create an event trigger during the
> > startup callback if it doesn't exist, it cannot drop it during the
> > shutdown callback. We also cannot establish a dependency between an
> > event trigger and a logical replication slot. We would likely need to
> > invent a new plugin callback specifically invoked at slot drop time
> > just to clean it up. Also, if different plugins want to capture DDL
> > events, they could end up registering different event triggers,
> > emitting multiple DDL WAL records for the same DDL event.
> >
>
> Why would different plugins end up registering different event
> triggers? I mean if they are already registered by the first plugin
> what is the need to re-register.
>
Since you mentioned column lists and row filtering as examples of what
individual plugins already do in a reply to my point that registering
event triggers could be an additional step for other plugins, I
thought you meant that each plugin registering event triggers is not a
huge cumbersome. I think different plugins don't need to register
different event triggers. We can have the common event triggers to
write logical-DDL WAL and register them when the first logical slot is
created. But as I mentioned, we need to be careful about both the
concurrent slot creation/drop and the fact that slot creation/drop
operations are not transactional. Also, we cannot create event
triggers on the replicas even if a logical slot is created there. If a
failover happens before applying the WAL of creating event triggers,
we would need to somehow make sure that even triggers are created on
the new primary if it has logical slots.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: Support logical replication of DDLs, take2
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-28 06:32 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 21:48 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-30 04:44 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-30 20:40 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-05-04 12:23 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-05-05 17:36 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
@ 2026-06-11 07:07 ` Masahiko Sawada <[email protected]>
0 siblings, 0 replies; 35+ messages in thread
From: Masahiko Sawada @ 2026-06-11 07:07 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Vitaly Davydov <[email protected]>; Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, May 5, 2026 at 10:36 AM Masahiko Sawada <[email protected]> wrote:
>
> On Mon, May 4, 2026 at 5:23 AM Amit Kapila <[email protected]> wrote:
> >
> > On Fri, May 1, 2026 at 2:11 AM Masahiko Sawada <[email protected]> wrote:
> > >
> > > On Wed, Apr 29, 2026 at 9:44 PM Amit Kapila <[email protected]> wrote:
> > > >
> > > > On Wed, Apr 29, 2026 at 3:19 AM Masahiko Sawada <[email protected]> wrote:
> > > > >
> > > > > On Mon, Apr 27, 2026 at 11:32 PM Amit Kapila <[email protected]> wrote:
> > > > > >
> > > > > > Yes, there will be a maintenance cost of JSON-based deparsing
> > > > > > approach. But note that multiple senior people (Alvaro Herrera, Robert
> > > > > > Haas) [1] seems to favor that approach. So, I am not sure we can
> > > > > > conclude to abandon that approach without those people or some other
> > > > > > senior people agreeing to abandon it. To be clear, I am not against
> > > > > > considering a new/different approach for DDL replication but just that
> > > > > > it is not clear that old/existing approach can be ruled out without
> > > > > > more discussion on it,
> > > > >
> > > > > Thank you for pointing it out. Just to be clear, IIUC what they liked
> > > > > was to use JSON string representation of DDLs, but not JSON string
> > > > > representation of DDLs that are deparsed from parse nodes, no?
> > > > >
> > > >
> > > > As per my understanding, we built deparsing stuff with a goal of
> > > > supporting DDL replication and Alvaro was the original author of that
> > > > work, see [1]. The benefit it provides flexibility in terms of
> > > > filtering by decoding plugin, if any, or changing the DDL (like
> > > > schema-mapping) during apply. It is not clear to me if we can achive
> > > > similar level of flexibility with other approach.
> > >
> > > I think we can generate the same JSON-string representation of DDLs
> > > from catalog information, it would also require a lot of code, though.
> > > It would be independent from parse nodes and if we implement it as an
> > > option for pg_get_xxx_ddl() functionality it would be able to be
> > > reused by other tools too.
> > >
> >
> > IIRC, this was discussed previously as well but we were not sure if we
> > can build all (especially some complex ones) without parsetree. See
> > discussion/emails around [1][2].
>
> Right. We would need parsetree somewhat. I think we're able to
> generate CREATE and DROP TABLE statements for the particular table
> without parsetree. But as for generating CREATE TABLE for the table,
> it's going to be a combination of CREATE/ALTER TABLE/INDEX/SEQUENCE
> statements, like pg_dump does. For instance, if a user executes
> "CREATE TABLE foo (id serial primary key)", we create table, sequence,
> and index, but searching system catalogs doesn't tell us these objects
> are created in one statement. So we would generate multiple DDLs as
> follow:
>
> CREATE TABLE public.foo (id integer NOT NULL);
> CREATE SEQUENCE public.foo_id_seq AS integer ...;
> ALTER SEQUENCE public.foo OWNED BY foo;
> ALTER TABLE public.foo ADD CONSTRAINT foo_pkey PRIMARY KEY (id);
>
> While these queries create the same table as the one on the publisher,
> we need to consider whether it's okay to replicate these queries
> instead of the oen statement originally executed on the publisher. If
> we can use something like pg_get_table_ddl() in DDL replication, that
> function would be able to be used also by the initial schema
> synchronization.
>
> As for ALTER TABLE, we would need parsetree of ALTER TABLE subcommands.
>
> >
> > > >
> > > > >
> > > > > >
> > > > > > We would need to maintain the JSON serialization code whenever
> > > > > > > creating or modifying parse nodes, regardless of whether the changes
> > > > > > > were related to DDL replication. IIUC, this was the primary reason the
> > > > > > > feature didn't cross the finish line.
> > > > > > >
> > > > > > > Additionally, I think there is another design issue: it is not
> > > > > > > output-plugin agnostic. Since the deparsed DDL was written by a
> > > > > > > logical-replication-specific event trigger, third-party logical
> > > > > > > decoding plugins cannot easily detect DDL events.
> > > > > > >
> > > > > >
> > > > > > Why RmgrId like RM_LOGICALDDLMSG_ID and XLOG_LOGICAL_DDL_MESSAGE wal
> > > > > > info is not sufficient for this? Decoder will add a message like
> > > > > > REORDER_BUFFER_CHANGE_DDL which can be used to detect DDL message, no?
> > > > >
> > > > > Right, but I'm not sure this is a good developer experience that
> > > > > additional steps are required to capture DDL events for other plugins
> > > > > while changes of INSERT/UPDATE/DELETE/TRUNCATE are passed from the
> > > > > logical decoding by default.
> > > > >
> > > >
> > > > Yes, there could probably be additional steps for plugins but they
> > > > must be doing a few things already which are defined at publication
> > > > level like column lists, row filtering, something related to RI, etc.
> > >
> > > I think those publication-level features operate at a somewhat
> > > different layer than the fundamental mechanism of capturing DDL
> > > events. Plugins filter rows or columns based on configuration, but the
> > > logical decoding itself guarantees that the DML events are reliably
> > > passed to them. Given that the TRUNCATE in logical replication already
> > > works so, I guess DDL should have the same fundamental guarantee.
> > >
> > > it's unclear to me how plugins could reliably manage these event
> > > triggers. While a plugin might create an event trigger during the
> > > startup callback if it doesn't exist, it cannot drop it during the
> > > shutdown callback. We also cannot establish a dependency between an
> > > event trigger and a logical replication slot. We would likely need to
> > > invent a new plugin callback specifically invoked at slot drop time
> > > just to clean it up. Also, if different plugins want to capture DDL
> > > events, they could end up registering different event triggers,
> > > emitting multiple DDL WAL records for the same DDL event.
> > >
> >
> > Why would different plugins end up registering different event
> > triggers? I mean if they are already registered by the first plugin
> > what is the need to re-register.
> >
>
> Since you mentioned column lists and row filtering as examples of what
> individual plugins already do in a reply to my point that registering
> event triggers could be an additional step for other plugins, I
> thought you meant that each plugin registering event triggers is not a
> huge cumbersome. I think different plugins don't need to register
> different event triggers. We can have the common event triggers to
> write logical-DDL WAL and register them when the first logical slot is
> created. But as I mentioned, we need to be careful about both the
> concurrent slot creation/drop and the fact that slot creation/drop
> operations are not transactional. Also, we cannot create event
> triggers on the replicas even if a logical slot is created there. If a
> failover happens before applying the WAL of creating event triggers,
> we would need to somehow make sure that even triggers are created on
> the new primary if it has logical slots.
>
I've reviewed and researched the last proposed DDL deparse patch[1],
and found that it can generate a set of commands for a single command.
For example, deparsing "create table test_serial (a serial)"
generates:
- CREATE SEQUENCE public.test_serial_a_seq CACHE 1 NO CYCLE INCREMENT
BY 1 MINVALUE 1 MAXVALUE 2147483647 START WITH 1 RESTART 1 AS
pg_catalog.int4;
- CREATE TABLE public.test_serial (a pg_catalog.int4 STORAGE PLAIN NOT
NULL DEFAULT pg_catalog.nextval('public.test_serial_a_seq'::pg_catalog.regclass));
- ALTER SEQUENCE public.test_serial_a_seq OWNED BY public.test_serial.a;
And deparsing "create table test_b (a int, b int references test_a
(a))" generates:
- CREATE TABLE public.test_b (a pg_catalog.int4 STORAGE PLAIN, b
pg_catalog.int4 STORAGE PLAIN);
- ALTER TABLE public.test_b ADD CONSTRAINT test_b_b_fkey FOREIGN KEY
(b) REFERENCES public.test_a(a);
The reason is that the patch deparses after the command runs (via an
event trigger at ddl_command_end) and rebuilds the DDL from the
catalog. By that time, the server has already expanded one CREATE
TABLE into several internal sub-commands: transformCreateStmt() turns
a serial column into a CREATE SEQUENCE plus ALTER SEQUENCE ... OWNED
BY, a foreign key into an ALTER TABLE ADD CONSTRAINT, a primary key
into an index, and so on. So the output looks much like pg_dump or the
proposed pg_get_table_ddl() [2], because it also rebuilds from the
catalog, not from the original parse tree.
For a general DDL deparse feature, is it useful to turn one command
into several, or to emit a command that is different from what the
user ran? Since DDL deparsing can be used also for schema-qualifying
DDL commands, I considered it's useful for audit purposes or CDC use
cases. But if the generated DDL command could be different or even
multiple DDL commands are generated from one DDL command, I'm not sure
it's useful for other use cases than DDL replication.
Even in DDL replication use cases, I'm concerned that it might be
confusing users. For example, what if DDL replication has DDL command
filter and users specify it to replicate only 'CREATE TABLE' and not
for 'ALTER TABLE'? Users might expect all replicated DDL commands are
'CREATE TABLE' but it would not be able to replicate some form of
CREATE TABLE without ALTER TABLE or CREATE/ALTER SEQUENCE.
Also, given that the proposed pg_get_table_ddl()[2] has approximately
1800 lines to support generating CREATE TABLE commands for the given
table, does it really make sense to have additional 3000 lines to
support DDL deparsing that works in mostly the same way but based on
parse trees? While the JSON blob idea is flexible and preferable,
considering that two features seem to work in mostly the same way,
I'm not sure it can justify the implementation costs.
I think that DDL deparse should keep the intention and the form of the
original command as much as possible: do not turn one command into
several commands, and do not add options that the user did not write.
I see two benefits:
First, as a feature on its own, this makes DDL deparse usable for pure
schema-qualification. That is useful outside DDL replication too, for
example for audit or CDC. (Some audit cases may instead want the fully
expanded form. We could offer that as an option, but I think the
default should stay faithful to the original command.)
Second, as a building block for DDL replication, keeping the original
command lets the subscriber benefit from improvements in its own
version. Some commands may take a lighter lock, or use better new
defaults. For example, in v19 the default TOAST compression became
lz4, while it was pglz before. If we had DDL replication in v18 and a
user set up a logical replication v18 -> v19, I think they would not
want new tables on v19 to be forced to pglz just because that was the
publisher's default. If a user does want exactly the same options on
the subscriber, we can add an option to include the options that were
not explicitly specified too. But even then I do not think we should
produce multiple commands from one command.
Another point I think we should discuss is how to capture DDL events.
The last patch creates event triggers automatically at CREATE
PUBLICATION. I am not sure this is the best approach:
Third-party logical decoding output plugins do not use PUBLICATION, so
they cannot easily capture DDL events this way.
We could instead create a common event trigger when the first logical
slot is created. But it is not easy to make sure the trigger is
reliably created and dropped, including on replicas (for example, a
slot can exist only on a replica that is later promoted).
I think we can decouple the DDL capture infrastructure from event
triggers, and use it as common infrastructure for both event triggers
and WAL-logging of DDL for replication. That way, I think we can
trigger DDL deparse while not relying on event triggers and minimizing
the code duplication. I'm drafting the patch for this idea and
feedback is very welcome.
Regards,
[1] https://www.postgresql.org/message-id/OS0PR01MB57163E6487EFF7378CB8E17C9438A%40OS0PR01MB5716.jpnprd0...
[2] https://www.postgresql.org/message-id/CANxoLDfjQnhM%3DE6JSyYo9s9OdjqoN8s_3wE5yL%3DkaDu_X8j-dA%40mail...
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 35+ messages in thread
end of thread, other threads:[~2026-06-11 07:07 UTC | newest]
Thread overview: 35+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-07-09 00:14 [PATCH 4/4] rework where incremental sort paths are created Tomas Vondra <[email protected]>
2022-09-14 09:10 [PATCH] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2022-09-14 09:10 [PATCH] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2022-09-14 09:10 [PATCH v4] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2022-09-14 09:10 [PATCH v2] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2022-09-14 09:10 [PATCH v3] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2022-09-14 09:10 [PATCH v1] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2022-09-14 09:10 [PATCH] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2022-09-14 09:10 [PATCH v2] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2022-09-14 09:10 [PATCH v4] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2022-09-14 09:10 [PATCH] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2022-09-14 09:10 [PATCH v4] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2022-09-14 09:10 [PATCH v2] Allow wildcard (%) in extension upgrade paths Sandro Santilli <[email protected]>
2026-01-14 17:37 [PATCH v1 1/3] Rename pg_popcount_avx512.c to pg_popcount_x86_64.c. Nathan Bossart <[email protected]>
2026-01-14 17:37 [PATCH v1 1/3] Rename pg_popcount_avx512.c to pg_popcount_x86_64.c. Nathan Bossart <[email protected]>
2026-01-14 17:37 [PATCH v2 1/4] Rename pg_popcount_avx512.c to pg_popcount_x86.c. Nathan Bossart <[email protected]>
2026-04-17 18:34 [PATCH v2 3/4] psql: bump minimum supported version to v10 Nathan Bossart <[email protected]>
2026-04-20 23:14 Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-27 06:14 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-28 06:57 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 19:55 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
2026-04-29 03:39 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-29 08:07 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
2026-04-29 11:29 ` Re: Support logical replication of DDLs, take2 Dilip Kumar <[email protected]>
2026-04-29 12:10 ` Re: Support logical replication of DDLs, take2 Andres Freund <[email protected]>
2026-04-30 14:02 ` Re: Support logical replication of DDLs, take2 Hannu Krosing <[email protected]>
2026-05-01 18:40 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-28 21:38 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-28 06:32 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-28 21:48 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-04-30 04:44 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-04-30 20:40 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-05-04 12:23 ` Re: Support logical replication of DDLs, take2 Amit Kapila <[email protected]>
2026-05-05 17:36 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
2026-06-11 07:07 ` Re: Support logical replication of DDLs, take2 Masahiko Sawada <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox