From 7fbbbe4a576ddcff152756933ecf08a85feeaa39 Mon Sep 17 00:00:00 2001 From: "Andrei V. Lepikhov" Date: Fri, 8 May 2026 15:14:15 +0200 Subject: [PATCH 1/6] patches: expose SpockCorePatchsetVersion from PG core Add a single integer (SPOCK_CORE_PATCHSET_VERSION compile-time, and SpockCorePatchsetVersion runtime global) in miscadmin.h and globals.c on every supported PG branch (15-18). This gives the spock extension a binary-level handshake with the patched server: an unpatched server fails to dynamic-link, and a server patched against a different generation produces a clear runtime mismatch later. No behaviour change yet -- the consumer side lands in the next commit. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../15/pg15-000-spock-patchset-version.diff | 32 +++++++++++++++++++ .../16/pg16-000-spock-patchset-version.diff | 32 +++++++++++++++++++ .../17/pg17-000-spock-patchset-version.diff | 32 +++++++++++++++++++ .../18/pg18-000-spock-patchset-version.diff | 32 +++++++++++++++++++ 4 files changed, 128 insertions(+) create mode 100644 patches/15/pg15-000-spock-patchset-version.diff create mode 100644 patches/16/pg16-000-spock-patchset-version.diff create mode 100644 patches/17/pg17-000-spock-patchset-version.diff create mode 100644 patches/18/pg18-000-spock-patchset-version.diff diff --git a/patches/15/pg15-000-spock-patchset-version.diff b/patches/15/pg15-000-spock-patchset-version.diff new file mode 100644 index 00000000..99f97ab8 --- /dev/null +++ b/patches/15/pg15-000-spock-patchset-version.diff @@ -0,0 +1,32 @@ +Spock core-patchset: export patchset version via miscadmin.h and globals.c. + +Adds SPOCK_CORE_PATCHSET_VERSION (compile-time constant) and +SpockCorePatchsetVersion (runtime global) to the standard places +PostgreSQL already uses for server-wide state. No new files. + +--- a/src/include/miscadmin.h ++++ b/src/include/miscadmin.h +@@ -498,4 +498,11 @@ + /* in executor/nodeHash.c */ + extern size_t get_hash_memory_limit(void); + ++/* ++ * Spock core-patchset identity. Bump the version when the patchset ++ * changes in a way visible to the extension binary. ++ */ ++#define SPOCK_CORE_PATCHSET_VERSION 1 ++extern PGDLLIMPORT int SpockCorePatchsetVersion; ++ + #endif /* MISCADMIN_H */ +--- a/src/backend/utils/init/globals.c ++++ b/src/backend/utils/init/globals.c +@@ -114,6 +114,9 @@ + bool IsBinaryUpgrade = false; + bool IsBackgroundWorker = false; + ++/* Spock core-patchset identity. */ ++int SpockCorePatchsetVersion = SPOCK_CORE_PATCHSET_VERSION; ++ + bool ExitOnAnyError = false; + + int DateStyle = USE_ISO_DATES; diff --git a/patches/16/pg16-000-spock-patchset-version.diff b/patches/16/pg16-000-spock-patchset-version.diff new file mode 100644 index 00000000..224d34d8 --- /dev/null +++ b/patches/16/pg16-000-spock-patchset-version.diff @@ -0,0 +1,32 @@ +Spock core-patchset: export patchset version via miscadmin.h and globals.c. + +Adds SPOCK_CORE_PATCHSET_VERSION (compile-time constant) and +SpockCorePatchsetVersion (runtime global) to the standard places +PostgreSQL already uses for server-wide state. No new files. + +--- a/src/include/miscadmin.h ++++ b/src/include/miscadmin.h +@@ -510,4 +510,11 @@ + /* in executor/nodeHash.c */ + extern size_t get_hash_memory_limit(void); + ++/* ++ * Spock core-patchset identity. Bump the version when the patchset ++ * changes in a way visible to the extension binary. ++ */ ++#define SPOCK_CORE_PATCHSET_VERSION 1 ++extern PGDLLIMPORT int SpockCorePatchsetVersion; ++ + #endif /* MISCADMIN_H */ +--- a/src/backend/utils/init/globals.c ++++ b/src/backend/utils/init/globals.c +@@ -114,6 +114,9 @@ + bool IsBinaryUpgrade = false; + bool IsBackgroundWorker = false; + ++/* Spock core-patchset identity. */ ++int SpockCorePatchsetVersion = SPOCK_CORE_PATCHSET_VERSION; ++ + bool ExitOnAnyError = false; + + int DateStyle = USE_ISO_DATES; diff --git a/patches/17/pg17-000-spock-patchset-version.diff b/patches/17/pg17-000-spock-patchset-version.diff new file mode 100644 index 00000000..0b186060 --- /dev/null +++ b/patches/17/pg17-000-spock-patchset-version.diff @@ -0,0 +1,32 @@ +Spock core-patchset: export patchset version via miscadmin.h and globals.c. + +Adds SPOCK_CORE_PATCHSET_VERSION (compile-time constant) and +SpockCorePatchsetVersion (runtime global) to the standard places +PostgreSQL already uses for server-wide state. No new files. + +--- a/src/include/miscadmin.h ++++ b/src/include/miscadmin.h +@@ -525,4 +525,11 @@ + /* in executor/nodeHash.c */ + extern size_t get_hash_memory_limit(void); + ++/* ++ * Spock core-patchset identity. Bump the version when the patchset ++ * changes in a way visible to the extension binary. ++ */ ++#define SPOCK_CORE_PATCHSET_VERSION 1 ++extern PGDLLIMPORT int SpockCorePatchsetVersion; ++ + #endif /* MISCADMIN_H */ +--- a/src/backend/utils/init/globals.c ++++ b/src/backend/utils/init/globals.c +@@ -117,6 +117,9 @@ + bool IsUnderPostmaster = false; + bool IsBinaryUpgrade = false; + ++/* Spock core-patchset identity. */ ++int SpockCorePatchsetVersion = SPOCK_CORE_PATCHSET_VERSION; ++ + bool ExitOnAnyError = false; + + int DateStyle = USE_ISO_DATES; diff --git a/patches/18/pg18-000-spock-patchset-version.diff b/patches/18/pg18-000-spock-patchset-version.diff new file mode 100644 index 00000000..79bf9aaf --- /dev/null +++ b/patches/18/pg18-000-spock-patchset-version.diff @@ -0,0 +1,32 @@ +Spock core-patchset: export patchset version via miscadmin.h and globals.c. + +Adds SPOCK_CORE_PATCHSET_VERSION (compile-time constant) and +SpockCorePatchsetVersion (runtime global) to the standard places +PostgreSQL already uses for server-wide state. No new files. + +--- a/src/include/miscadmin.h ++++ b/src/include/miscadmin.h +@@ -540,4 +540,11 @@ + /* in executor/nodeHash.c */ + extern size_t get_hash_memory_limit(void); + ++/* ++ * Spock core-patchset identity. Bump the version when the patchset ++ * changes in a way visible to the extension binary. ++ */ ++#define SPOCK_CORE_PATCHSET_VERSION 1 ++extern PGDLLIMPORT int SpockCorePatchsetVersion; ++ + #endif /* MISCADMIN_H */ +--- a/src/backend/utils/init/globals.c ++++ b/src/backend/utils/init/globals.c +@@ -120,6 +120,9 @@ + bool IsUnderPostmaster = false; + bool IsBinaryUpgrade = false; + ++/* Spock core-patchset identity. */ ++int SpockCorePatchsetVersion = SPOCK_CORE_PATCHSET_VERSION; ++ + bool ExitOnAnyError = false; + + int DateStyle = USE_ISO_DATES; From f8b9065a0a3dbbda356f006db88f0032c3140b3b Mon Sep 17 00:00:00 2001 From: "Andrei V. Lepikhov" Date: Fri, 8 May 2026 15:14:48 +0200 Subject: [PATCH 2/6] spock: refuse to load against a mismatched core patchset In _PG_init, compare the runtime SpockCorePatchsetVersion exposed by the patched server against the SPOCK_CORE_PATCHSET_VERSION the extension was compiled against, and ereport() if they disagree. Catches the "extension binary upgraded but server binary still on the old patchset" footgun before any worker starts, so the failure mode is a clean error at LOAD instead of a subtle later crash. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/spock.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/spock.c b/src/spock.c index 9b7907e1..3d781d3d 100644 --- a/src/spock.c +++ b/src/spock.c @@ -1004,6 +1004,20 @@ _PG_init(void) if (!process_shared_preload_libraries_in_progress) elog(ERROR, "spock is not in shared_preload_libraries"); + /* + * Runtime patchset check: if the server binary was built from a + * different patchset generation than this extension, refuse to + * start. An unpatched server never reaches here -- the dynamic + * linker fails on the missing SpockCorePatchsetVersion symbol. + */ + if (SpockCorePatchsetVersion != SPOCK_CORE_PATCHSET_VERSION) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("spock core patchset version mismatch: " + "server has v%d, extension expects v%d", + SpockCorePatchsetVersion, + SPOCK_CORE_PATCHSET_VERSION))); + DefineCustomEnumVariable("spock.conflict_resolution", gettext_noop("Sets method used for conflict resolution for resolvable conflicts."), NULL, From 770aaed3051653177a2befb7d87f0f5829ef6a08 Mon Sep 17 00:00:00 2001 From: "Andrei V. Lepikhov" Date: Fri, 8 May 2026 15:19:17 +0200 Subject: [PATCH 3/6] spock: model delta-apply columns as SECURITY LABELs Stop relying on the per-attribute reloption pair (log_old_value, delta_apply_function) -- which required a core patch to recognise -- and represent the same intent as a pg_seclabel row with provider 'spock' and label 'spock.delta_apply'. Update conflicts.md / troubleshooting.md to the spock.delta_apply() helper form, and stop wiping seclabel rows on internal table drops during ALTER EXTENSION UPDATE -- only the extension drop itself should clear them. Register the provider before the IsBinaryUpgrade early-return so pg_restore can replay labels during pg_upgrade. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/conflicts.md | 51 ++++++++++++++++++++++++++++++----------- docs/troubleshooting.md | 3 +-- src/spock.c | 22 +++++++++++++++--- src/spock_apply_heap.c | 2 -- src/spock_executor.c | 15 ++++++++++-- 5 files changed, 70 insertions(+), 23 deletions(-) diff --git a/docs/conflicts.md b/docs/conflicts.md index 554e2673..585005a0 100644 --- a/docs/conflicts.md +++ b/docs/conflicts.md @@ -33,24 +33,47 @@ conflicts with the following logic: Note that on a conflicting transaction, the delta-apply column will be correctly calculated and applied. -To make a column a conflict-free delta-apply column, ensuring that the value -replicated is the delta of the committed changes (the old value plus or -minus any new value) to a given record, you need to apply the following -settings to the column: `log_old_value=true, -delta_apply_function=spock.delta_apply`. For example: +To make a column a conflict-free delta-apply column, ensuring that the +value replicated is the delta of the committed changes (the old value +plus or minus any new value) to a given record, attach a SECURITY LABEL +to the column via the `spock.delta_apply()` helper: ```sql -ALTER TABLE pgbench_accounts ALTER COLUMN abalance - SET (log_old_value=true, delta_apply_function=spock.delta_apply); -ALTER TABLE pgbench_branches ALTER COLUMN bbalance - SET (log_old_value=true, delta_apply_function=spock.delta_apply); -ALTER TABLE pgbench_tellers ALTER COLUMN tbalance - SET (log_old_value=true, delta_apply_function=spock.delta_apply); +SELECT spock.delta_apply('pgbench_accounts'::regclass, 'abalance'); +SELECT spock.delta_apply('pgbench_branches'::regclass, 'bbalance'); +SELECT spock.delta_apply('pgbench_tellers'::regclass, 'tbalance'); ``` -As a special safety-valve feature, if you ever need to re-set a -`log_old_value` column you can temporarily alter the column to -`log_old_value` is `false`. +To remove the marker, pass `to_drop => true`: + +```sql +SELECT spock.delta_apply('pgbench_accounts'::regclass, 'abalance', to_drop => true); +``` + +Under the hood, `spock.delta_apply()` writes a row into `pg_seclabel` +with `provider = 'spock'` and `label = 'spock.delta_apply'`. The +binary-upgrade compatibility shim that translates legacy spock 5.x +reloptions during `pg_upgrade` writes the same canonical label, so +operators can audit the catalog uniformly: + +```sql +SELECT * FROM pg_seclabel + WHERE provider = 'spock' AND label = 'spock.delta_apply'; +``` + +### Upgrading from spock 5.x + +Spock 5.x recorded the same intent as a pair of per-attribute reloptions +(`log_old_value=true, delta_apply_function=spock.delta_apply`). During +`pg_upgrade`, the binary-upgrade compatibility shim translates those +reloptions into the new `SECURITY LABEL` form automatically. Look for +`NOTICE: spock: rewrote ALTER TABLE … ALTER COLUMN … legacy options to +SECURITY LABEL` lines in `pg_upgrade.log` to audit the translation. +After the upgrade, `SELECT * FROM pg_seclabel WHERE provider = 'spock' +AND label = 'spock.delta_apply'` is the authoritative list of delta-apply +columns. See +[Binary-upgrade compatibility shims](internals-doc/binary-upgrade-compat-shim.md) +for the full design. ### Conflict Configuration Options diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 1d3c3050..5a208395 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -165,8 +165,7 @@ ALTER TABLE table_name ALTER COLUMN column_name SET NOT NULL; Then configure the Delta-Apply column with the following command: ```sql -ALTER TABLE table_name ALTER COLUMN column_name - SET (log_old_value=true, delta_apply_function=spock.delta_apply); +SELECT spock.delta_apply('table_name'::regclass, 'column_name'); ``` ## Configuration Issues diff --git a/src/spock.c b/src/spock.c index 3d781d3d..2eb8e697 100644 --- a/src/spock.c +++ b/src/spock.c @@ -53,6 +53,7 @@ #include "pgstat.h" #include "spock_apply.h" + #if PG_VERSION_NUM >= 180000 #include "spock_conflict_stat.h" #endif @@ -979,7 +980,9 @@ spock_object_relabel(const ObjectAddress *object, const char *seclabel) extoid = get_extension_oid(EXTENSION_NAME, true); if (!OidIsValid(extoid)) - elog(ERROR, "spock extension is not created yet"); + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("spock extension is not created yet"))); /* * Check: classId must be pg_class, objectId should an existing table and @@ -1307,6 +1310,16 @@ _PG_init(void) NULL, NULL); + /* + * Register the spock security label provider BEFORE the + * IsBinaryUpgrade early-return. The binary-upgrade compatibility + * shims synthesise SECURITY LABEL statements during pg_restore; + * those statements need the provider to be registered, otherwise + * ExecSecLabelStmt fails with "security label provider 'spock' is + * not loaded". + */ + register_label_provider(SPOCK_SECLABEL_PROVIDER, spock_object_relabel); + if (IsBinaryUpgrade) return; @@ -1340,8 +1353,11 @@ _PG_init(void) prev_emit_log_hook = emit_log_hook; emit_log_hook = log_message_filter; - /* Security label provider hook */ - register_label_provider(SPOCK_SECLABEL_PROVIDER, spock_object_relabel); + /* + * Note: the security label provider is registered earlier in + * _PG_init, before the IsBinaryUpgrade early-return, so it is + * available during pg_upgrade for the binary-upgrade compat shims. + */ #if PG_VERSION_NUM >= 180000 /* Spock replication conflict statistics */ diff --git a/src/spock_apply_heap.c b/src/spock_apply_heap.c index 46253d76..d1ed4e84 100644 --- a/src/spock_apply_heap.c +++ b/src/spock_apply_heap.c @@ -510,7 +510,6 @@ physatt_in_attmap(SpockRelation *rel, int attid) return false; } - static void build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, SpockTupleData *newtup, @@ -569,7 +568,6 @@ build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, } } - /** * This is called when there is a potential conflict that may be able to be resolved * according to resolution rules diff --git a/src/spock_executor.c b/src/spock_executor.c index b5d4f51d..5de98844 100644 --- a/src/spock_executor.c +++ b/src/spock_executor.c @@ -248,6 +248,7 @@ spock_object_access(ObjectAccessType access, { ObjectAccessDrop *drop_arg = (ObjectAccessDrop *) arg; DropBehavior behavior; + bool dropping_spock_extension = false; /* No need to check for internal deletions. */ if ((drop_arg->dropflags & PERFORM_DELETION_INTERNAL) != 0) @@ -257,7 +258,10 @@ spock_object_access(ObjectAccessType access, if (classId == ExtensionRelationId && objectId == get_extension_oid(EXTENSION_NAME, true) && objectId != InvalidOid /* Should not happen but check anyway */ ) + { + dropping_spock_extension = true; dropping_spock_obj = true; + } /* Dropping relation within spock? */ if (classId == RelationRelationId) @@ -278,8 +282,15 @@ spock_object_access(ObjectAccessType access, */ if (dropping_spock_obj) { - /* Need to drop any security labels created by the extension */ - DeleteSecurityLabels(SPOCK_SECLABEL_PROVIDER); + /* + * Wipe spock-provider security labels only when the extension + * itself is being dropped. Dropping individual relations in + * the spock namespace (e.g. internal cleanup during ALTER + * EXTENSION UPDATE that recreates spock.lag_tracker / + * spock.progress) must not touch user-set delta_apply labels. + */ + if (dropping_spock_extension) + DeleteSecurityLabels(SPOCK_SECLABEL_PROVIDER); return; } From 195bf12c79fe1bd99030efd3f7597e71145fea15 Mon Sep 17 00:00:00 2001 From: "Andrei V. Lepikhov" Date: Fri, 8 May 2026 15:19:58 +0200 Subject: [PATCH 4/6] spock: install ProcessUtility shim for 5.x->6.x binary upgrades pg_dump --binary-upgrade against a spock 5.x source still emits ALTER TABLE ... SET (log_old_value=true, delta_apply_function=...). Add a ProcessUtility hook, gated on IsBinaryUpgrade so it costs nothing in normal operation, that intercepts those AlterTableCmds, strips the legacy DefElems, and synthesises the equivalent SECURITY LABEL FOR spock statement. Unrelated reloptions on the same SET clause survive untouched. Self-retiring: deleting the file and the register call from _PG_init is the cleanup once 5.x is out of support. Co-Authored-By: Claude Opus 4.7 (1M context) --- include/spock.h | 3 + src/spock.c | 7 + src/spock_bucompat_5x.c | 449 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 459 insertions(+) create mode 100644 src/spock_bucompat_5x.c diff --git a/include/spock.h b/include/spock.h index 8ae26ec1..7d74bd61 100644 --- a/include/spock.h +++ b/include/spock.h @@ -133,4 +133,7 @@ VALGRIND_PRINTF(const char *format,...) extern void spock_init_failover_slot(void); +/* lives in src/spock_bucompat_5x.c -- used only under IsBinaryUpgrade */ +extern void register_spock_compat_5x(void); + #endif /* SPOCK_H */ diff --git a/src/spock.c b/src/spock.c index 2eb8e697..f97791d0 100644 --- a/src/spock.c +++ b/src/spock.c @@ -1320,6 +1320,13 @@ _PG_init(void) */ register_label_provider(SPOCK_SECLABEL_PROVIDER, spock_object_relabel); + /* + * Install the 5.x -> 6.x binary-upgrade compatibility ProcessUtility + * hook. Self-gates on IsBinaryUpgrade -- nothing happens outside + * pg_upgrade. + */ + register_spock_compat_5x(); + if (IsBinaryUpgrade) return; diff --git a/src/spock_bucompat_5x.c b/src/spock_bucompat_5x.c new file mode 100644 index 00000000..6f3fb371 --- /dev/null +++ b/src/spock_bucompat_5x.c @@ -0,0 +1,449 @@ +/*------------------------------------------------------------------------- + * + * spock_bucompat_5x.c + * Binary-upgrade compatibility: spock 5.x -> 6.x. + * + * During pg_upgrade, pg_dump --binary-upgrade emits the legacy form + * used by spock 5.x: + * + * ALTER TABLE t ALTER COLUMN c SET (log_old_value=true, + * delta_apply_function=spock.delta_apply); + * + * The patches that taught core to recognise those reloption names + * live in patches/attic/. spock 6.x records the same intent as a + * security label with provider 'spock': + * + * SECURITY LABEL FOR spock ON COLUMN t.c IS 'spock.delta_apply'; + * + * On pg_restore against the new cluster we install a ProcessUtility + * hook that intercepts each AlterTableCmd carrying the legacy + * DefElem names, drops them from the cmd, and synthesises the + * equivalent SecLabelStmt. Unrelated DefElems (e.g. fillfactor) + * on the same SET clause survive untouched. + * + * Self-contained: the hook is installed only when IsBinaryUpgrade + * is true at module-load time, so the normal DDL path pays nothing. + * + * Retirement: delete this file and the register_spock_compat_5x() + * call from spock.c's _PG_init(). Two edits. + * + * Copyright (c) 2022-2026, pgEdge, Inc. + * + *------------------------------------------------------------------------- + */ +#include "postgres.h" + +#include "commands/defrem.h" +#include "miscadmin.h" +#include "nodes/makefuncs.h" +#include "nodes/parsenodes.h" +#include "nodes/plannodes.h" +#include "nodes/value.h" +#include "tcop/utility.h" +#include "utils/builtins.h" + +#include "spock.h" + + +/* Legacy reloption names this shim recognises. */ +#define LEGACY_LOG_OLD_VALUE "log_old_value" +#define LEGACY_DELTA_APPLY_FN "delta_apply_function" + + +/* Saved previous ProcessUtility_hook so we chain correctly. */ +static ProcessUtility_hook_type prev_ProcessUtility_hook = NULL; + +/* Forward declarations. */ +static void spock_compat_5x_ProcessUtility(PlannedStmt *pstmt, + const char *queryString, + bool readOnlyTree, + ProcessUtilityContext context, + ParamListInfo params, + QueryEnvironment *queryEnv, + DestReceiver *dest, + QueryCompletion *qc); +static List *rewrite_5x_attoptions(AlterTableStmt *atstmt); +static bool defelem_is_legacy(DefElem *de); +static bool partition_def_list(List *def_in, List **kept_out, List **claimed_out); +static bool claimed_means_clear(List *claimed, bool is_reset); +static SecLabelStmt *make_label_stmt(AlterTableStmt *atstmt, const char *colname, + bool clear); +static void emit_rewrite_notice(AlterTableStmt *atstmt, const char *colname); + + +/* + * register_spock_compat_5x + * Public entry point invoked from spock.c's _PG_init(). + * + * Install the ProcessUtility hook only when running under + * pg_upgrade -- outside that, the legacy reloption form is not + * being replayed and the hook would only add overhead. + */ +void +register_spock_compat_5x(void) +{ + if (!IsBinaryUpgrade) + return; + + prev_ProcessUtility_hook = ProcessUtility_hook; + ProcessUtility_hook = spock_compat_5x_ProcessUtility; +} + + +/* + * spock_compat_5x_ProcessUtility + * Intercept AlterTableStmt during pg_restore and rewrite legacy + * spock-5.x reloptions into SECURITY LABEL form. + * + * For statements we do not recognise, fall through to the previous + * hook (or standard_ProcessUtility) unchanged. When we rewrite, + * we may either keep the (now-trimmed) original parsetree and run + * it followed by the synthesised SecLabelStmt(s), or drop the + * original entirely (when every cmd was legacy-only) and run only + * the synthesised statements. + * + * Recursion: synthesised statements re-enter ProcessUtility, which + * reaches this hook again. The recognition check declines anything + * that is not an AlterTableStmt, so recursion stops at depth 1. + */ +static void +spock_compat_5x_ProcessUtility(PlannedStmt *pstmt, const char *queryString, + bool readOnlyTree, ProcessUtilityContext context, + ParamListInfo params, QueryEnvironment *queryEnv, + DestReceiver *dest, QueryCompletion *qc) +{ + Node *parsetree; + List *synthetic = NIL; + bool skip_original = false; + ListCell *lc; + + Assert(pstmt != NULL); + + parsetree = pstmt->utilityStmt; + + if (parsetree != NULL && IsA(parsetree, AlterTableStmt)) + { + AlterTableStmt *atstmt = (AlterTableStmt *) parsetree; + + synthetic = rewrite_5x_attoptions(atstmt); + + /* + * Every cmd was a legacy-only SET/RESET that we collapsed. Core + * would Assert on an AlterTableStmt with an empty cmds list, so + * skip the original parsetree and run only the synthesised + * SecLabelStmt(s). + */ + skip_original = (synthetic != NIL && atstmt->cmds == NIL); + } + + if (!skip_original) + { + if (prev_ProcessUtility_hook != NULL) + prev_ProcessUtility_hook(pstmt, queryString, readOnlyTree, + context, params, queryEnv, dest, qc); + else + standard_ProcessUtility(pstmt, queryString, readOnlyTree, + context, params, queryEnv, dest, qc); + } + + foreach(lc, synthetic) + { + Node *synth = (Node *) lfirst(lc); + PlannedStmt *synth_pstmt = makeNode(PlannedStmt); + + synth_pstmt->commandType = CMD_UTILITY; + synth_pstmt->canSetTag = false; + synth_pstmt->utilityStmt = synth; + synth_pstmt->stmt_location = -1; + synth_pstmt->stmt_len = 0; + + /* + * queryString = NULL: synthetic statements are not user input and + * must not leak into pg_stat_statements as a sentinel string. + * + * Run via the public ProcessUtility entry point so any other + * registered hooks see synthetic statements. The recognition + * check at the top of this function declines anything that is + * not an AlterTableStmt, so the recursion is bounded. + */ + ProcessUtility(synth_pstmt, NULL, false, + PROCESS_UTILITY_SUBCOMMAND, params, queryEnv, + dest, NULL); + } +} + + +/* + * rewrite_5x_attoptions + * Walk an AlterTableStmt's cmds list. For each AT_SetOptions / + * AT_ResetOptions cmd carrying at least one legacy DefElem: + * - replace cmd->def with the kept (non-legacy) DefElems; + * - if kept is empty, drop the cmd from the list; + * - synthesise a SecLabelStmt and append to the result list; + * - emit one NOTICE per rewritten column. + * + * Iteration uses a build-new-list pattern. ListCell pointers stay + * stable, and any error mid-walk leaves the original cmds list + * untouched. + * + * Returns the list of synthetic SecLabelStmts (NIL when no rewrite + * happened). On a non-NIL return the caller must inspect + * atstmt->cmds: if NIL, the original parsetree must be skipped + * (every cmd was legacy-only). + */ +static List * +rewrite_5x_attoptions(AlterTableStmt *atstmt) +{ + List *new_cmds = NIL; + List *synthetic = NIL; + ListCell *lc; + + Assert(atstmt != NULL); + + if (atstmt->cmds == NIL) + return NIL; + + foreach(lc, atstmt->cmds) + { + AlterTableCmd *cmd = (AlterTableCmd *) lfirst(lc); + List *kept = NIL; + List *claimed = NIL; + bool is_setopt; + bool is_resetopt; + SecLabelStmt *sl; + bool clear; + + Assert(IsA(cmd, AlterTableCmd)); + + is_setopt = (cmd->subtype == AT_SetOptions); + is_resetopt = (cmd->subtype == AT_ResetOptions); + + /* + * Not relevant to this shim? Preserve verbatim. cmd->def is a + * List of DefElem for these subtypes; the IsA check is defensive + * against a future core change. + */ + if ((!is_setopt && !is_resetopt) || + cmd->def == NULL || !IsA(cmd->def, List) || + !partition_def_list((List *) cmd->def, &kept, &claimed)) + { + new_cmds = lappend(new_cmds, cmd); + continue; + } + + clear = claimed_means_clear(claimed, is_resetopt); + sl = make_label_stmt(atstmt, cmd->name, clear); + synthetic = lappend(synthetic, sl); + + emit_rewrite_notice(atstmt, cmd->name); + + /* + * If unrelated DefElems remain, keep the cmd with a trimmed def + * list. Otherwise drop the cmd -- core would choke on an + * AT_SetOptions cmd with an empty list. + */ + if (kept != NIL) + { + cmd->def = (Node *) kept; + new_cmds = lappend(new_cmds, cmd); + } + /* else: drop the cmd from new_cmds. */ + } + + /* Mutate the original parsetree only if we synthesised something. */ + if (synthetic != NIL) + atstmt->cmds = new_cmds; + + return synthetic; +} + + +/* + * defelem_is_legacy + * True if the DefElem names a legacy spock 5.x reloption. + * + * Case-sensitive: pg_dump emits these names verbatim and core's + * grammar lowercases unquoted identifiers. + */ +static bool +defelem_is_legacy(DefElem *de) +{ + Assert(de != NULL); + Assert(de->defname != NULL); + + return strcmp(de->defname, LEGACY_LOG_OLD_VALUE) == 0 || + strcmp(de->defname, LEGACY_DELTA_APPLY_FN) == 0; +} + + +/* + * partition_def_list + * Split a SET/RESET DefElem list into "kept" (untouched) and + * "claimed" (legacy keys this shim translates). + * + * Builds two new lists rather than mutating the input list while + * iterating it -- the latter is a classic source of subtle bugs + * with PostgreSQL's List API. Returns true iff at least one + * claimed DefElem was found. + */ +static bool +partition_def_list(List *def_in, List **kept_out, List **claimed_out) +{ + ListCell *lc; + List *kept = NIL; + List *claimed = NIL; + + Assert(kept_out != NULL); + Assert(claimed_out != NULL); + + foreach(lc, def_in) + { + DefElem *de = (DefElem *) lfirst(lc); + + Assert(IsA(de, DefElem)); + + if (defelem_is_legacy(de)) + claimed = lappend(claimed, de); + else + kept = lappend(kept, de); + } + + *kept_out = kept; + *claimed_out = claimed; + + return claimed != NIL; +} + + +/* + * claimed_means_clear + * Decide whether the claimed DefElems express an intent to CLEAR + * the delta-apply marker (true) or to SET it (false). + * + * - RESET ( log_old_value, ... ): always clear. + * - SET ( log_old_value=false ) with no other claimed key: clear. + * - Otherwise: set. + */ +static bool +claimed_means_clear(List *claimed, bool is_reset) +{ + ListCell *lc; + bool all_log_old_false = true; + bool saw_log_old_value = false; + + if (is_reset) + return true; + + foreach(lc, claimed) + { + DefElem *de = (DefElem *) lfirst(lc); + + if (strcmp(de->defname, LEGACY_LOG_OLD_VALUE) == 0) + { + saw_log_old_value = true; + /* + * de->arg == NULL means SET (log_old_value) with no value. + * pg_dump never emits this shape; treat it as "set" (matching + * defGetBoolean()'s default of true) without calling + * defGetBoolean on a NULL arg. + */ + if (de->arg == NULL || defGetBoolean(de) != false) + all_log_old_false = false; + } + else + { + /* delta_apply_function present -> intent is to set */ + all_log_old_false = false; + } + } + + return saw_log_old_value && all_log_old_false; +} + + +/* + * make_label_stmt + * Build a SECURITY LABEL FOR spock ON COLUMN . + * IS 'spock.delta_apply' (or IS NULL when clear is true). + * + * Allocations live in CurrentMemoryContext; the per-statement + * message context releases them after the synthesised statement + * runs. + */ +static SecLabelStmt * +make_label_stmt(AlterTableStmt *atstmt, const char *colname, bool clear) +{ + SecLabelStmt *sl; + List *object; + RangeVar *rv; + + Assert(atstmt != NULL); + Assert(colname != NULL); + + rv = atstmt->relation; + Assert(rv != NULL); + Assert(rv->relname != NULL); + + /* + * SECURITY LABEL's object list for OBJECT_COLUMN is + * [ ?, , ] -- see + * get_object_address_attribute() in catalog/objectaddress.c. + */ + object = NIL; + if (rv->schemaname != NULL) + object = lappend(object, makeString(pstrdup(rv->schemaname))); + object = lappend(object, makeString(pstrdup(rv->relname))); + object = lappend(object, makeString(pstrdup(colname))); + + sl = makeNode(SecLabelStmt); + sl->objtype = OBJECT_COLUMN; + sl->object = (Node *) object; + sl->provider = pstrdup(SPOCK_SECLABEL_PROVIDER); + /* + * Canonical label spelling: spock.delta_apply. Must match the string + * written by spock--6.0.0-devel.sql's spock.delta_apply() PL/pgSQL + * helper -- both pg_seclabel rows are read by + * spock_lookup_delta_function() as a function name and resolved via + * FuncnameGetCandidates(). Using the bare "delta_apply" string here + * would silently rely on search_path and produce a different label + * than the documented helper. + */ + sl->label = clear ? NULL : pstrdup("spock.delta_apply"); + + return sl; +} + + +/* + * emit_rewrite_notice + * One NOTICE per rewritten column, naming the table and column. + */ +static void +emit_rewrite_notice(AlterTableStmt *atstmt, const char *colname) +{ + RangeVar *rv = atstmt->relation; + const char *relname = "?"; + char *fullname = NULL; + + Assert(colname != NULL); + + if (rv != NULL && rv->relname != NULL) + { + if (rv->schemaname != NULL) + { + fullname = psprintf("%s.%s", rv->schemaname, rv->relname); + relname = fullname; + } + else + relname = rv->relname; + } + + ereport(NOTICE, + (errcode(ERRCODE_SUCCESSFUL_COMPLETION), + errmsg("spock: rewrote ALTER TABLE %s ALTER COLUMN %s " + "legacy options to SECURITY LABEL", + relname, colname))); + + if (fullname != NULL) + pfree(fullname); +} From d102443535ffc3213483603254779d31cf74b895 Mon Sep 17 00:00:00 2001 From: "Andrei V. Lepikhov" Date: Fri, 8 May 2026 15:20:21 +0200 Subject: [PATCH 5/6] tests: drive 5.x -> 6.x pg_upgrade via TAP Build two PG trees + two spock versions from the local working copies, run the upgrade, and assert that columns marked with the legacy 5.x delta_apply reloption land in the new cluster as a spock-provider SECURITY LABEL with the canonical 'spock.delta_apply' value. This is the upgrade.sh scenario, expressed in PostgreSQL::Test idioms so it runs under the standard make check_prove target and gates the binary-upgrade shim. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/tap/schedule | 1 + tests/tap/t/030_pg_upgrade_5x_to_6x.pl | 622 +++++++++++++++++++++++++ 2 files changed, 623 insertions(+) create mode 100644 tests/tap/t/030_pg_upgrade_5x_to_6x.pl diff --git a/tests/tap/schedule b/tests/tap/schedule index 94ec8830..e4f1c351 100644 --- a/tests/tap/schedule +++ b/tests/tap/schedule @@ -46,3 +46,4 @@ test: 019_stale_fd_epoll_after_conn_death test: 022_rmgr_progress_post_checkpoint_crash # Upgrade schema match test (builds from source, slow): #test: 018_upgrade_schema_match +test: 030_pg_upgrade_5x_to_6x diff --git a/tests/tap/t/030_pg_upgrade_5x_to_6x.pl b/tests/tap/t/030_pg_upgrade_5x_to_6x.pl new file mode 100644 index 00000000..53aece9f --- /dev/null +++ b/tests/tap/t/030_pg_upgrade_5x_to_6x.pl @@ -0,0 +1,622 @@ + +# Drive the actual pg_upgrade path for spock 5.x -> 6.x. This is the +# upgrade.sh scenario, restated as a TAP test using the standard +# PostgreSQL::Test framework. +# +# Bootstrap phase (raw shell, since we are literally building PostgreSQL): +# - Discover the local PostgreSQL repo (spock lives in contrib/spock, +# so its parent's parent is the PG source tree). Clone --shared from +# it twice -- one for old, one for new -- so we never touch the +# network and tags are already in scope. +# - Clone the local spock working tree once. Capture HEAD as the "new" +# ref; flip between $OLD_SPOCK_REF (default origin/v5_STABLE) and +# that captured ref via `git checkout` between builds. +# - For each variant: checkout the matching spock ref, apply +# patches/$PG_MAJOR/*.diff onto the matching PG tree, configure, +# build, install. +# +# Scenario phase (standard PostgreSQL::Test idioms): +# - Two PostgreSQL::Test::Cluster nodes with install_path pointing at +# the freshly-built prefixes. +# - Old node: install spock in two databases, mark a column with the +# legacy 5.x reloption form +# ALTER TABLE t ALTER COLUMN c SET (log_old_value=true, +# delta_apply_function=spock.delta_apply) +# - command_ok pg_upgrade old -> new. spock_bucompat_5x.c rewrites the +# legacy reloption to a SECURITY LABEL during pg_restore. +# - New node: ALTER EXTENSION spock UPDATE in each db, then assert +# pg_seclabel has the expected 'spock' provider row with label +# 'spock.delta_apply' on each marked column. +# +# Each run is a clean build: $TEMP_BASE and the per-node data dirs +# from any prior run are wiped at the start. Expect 5-30 minutes per +# run -- caching across runs sounded useful but in practice masked +# failures by carrying corrupted/half-applied state forward. +# +# Run it the same way as every other spock TAP test, via the spock +# Makefile's check_prove target: +# +# make check_prove PROVE_TESTS=t/030_pg_upgrade_5x_to_6x.pl +# +# That target already exports PG_CONFIG, prepends $(PG_CONFIG --bindir) +# to PATH, and adds PG_PROVE_FLAGS so PostgreSQL::Test::Cluster is +# importable. The test auto-resolves everything else: the PostgreSQL +# source repo (via spock/../..), the PG major (via PG_CONFIG --version), +# and the PG ref (via `git describe --tags --abbrev=0 REL__STABLE`, +# so REL_17_9 on a shipped branch, REL_18_BETA3 mid-cycle). +# +# Tunable via env (all optional; empty strings are ignored): +# PG_CONFIG path to the pg_config of the build +# target. Default: `pg_config` on PATH. +# Set by make check_prove already. +# SPOCK_TEST_PG_REPO path or URL of the PostgreSQL repo to +# clone from. Default: discovered local +# repo at ../.. relative to spock (or, as +# a fallback, `$PG_CONFIG --srcdir`). No +# network fetch in the default path. +# SPOCK_TEST_PG_BRANCH PostgreSQL ref to checkout. Override to +# pin a specific ref (REL_15_8, master, +# my-feature-branch). +# SPOCK_TEST_OLD_SPOCK_REF spock ref for OLD cluster (default +# origin/v5_STABLE; must be present in the +# local clone's remote refs). +# SPOCK_TEST_TEMP_BASE bootstrap work dir. Default: +# /tests/tap/tmp_check/030_pg_upgrade +# (already in spock's .gitignore). Wiped +# at the start of every run. +# SPOCK_TEST_PG_CONFIGURE extra ./configure flags. + +use strict; +use warnings FATAL => 'all'; + +use Cwd qw(getcwd abs_path); +use File::Basename qw(basename); +use File::Path qw(make_path remove_tree); + +# Locate PostgreSQL's TAP perl modules via pg_config so the test runs +# under a plain `prove t/030_*.pl` (e.g. via tests/tap/run_tests.sh) +# without the caller having to pass `-I .../src/test/perl`. The spock +# Makefile's `make check_prove` path passes PG_PROVE_FLAGS for us, but +# the shell wrapper and direct prove invocations do not. +BEGIN +{ + my $pgc = $ENV{PG_CONFIG}; + $pgc = 'pg_config' if !defined $pgc or $pgc eq ''; + + my @candidates; + + my $pgxs = qx('$pgc' --pgxs 2>/dev/null); + chomp $pgxs if defined $pgxs; + if (defined $pgxs and $pgxs ne '') + { + (my $p = $pgxs) =~ s{/src/makefiles/pgxs\.mk$}{/src/test/perl}; + push @candidates, $p; + } + + my $srcdir = qx('$pgc' --srcdir 2>/dev/null); + chomp $srcdir if defined $srcdir; + push @candidates, "$srcdir/src/test/perl" + if defined $srcdir and $srcdir ne ''; + + for my $p (@candidates) + { + if (-f "$p/PostgreSQL/Test/Cluster.pm") + { + unshift @INC, $p; + last; + } + } +} + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# --------------------------------------------------------------------------- +# Configuration +# --------------------------------------------------------------------------- +# Treat empty strings the same as undef. GitHub Actions and similar CI +# systems often expand inputs into env vars verbatim, so a workflow that +# does not pass a value for an optional input ends up exporting the var +# as ''. Plain `//` would not fall back in that case. +sub env_or +{ + my ($name, $default) = @_; + my $v = $ENV{$name}; + return (defined $v and $v ne '') ? $v : $default; +} + +# Path to pg_config: the canonical spock env var PG_CONFIG (used by the +# Makefile too) wins, otherwise rely on `pg_config` on PATH (which +# `make check_prove` sets up via the spock Makefile). If PG_CONFIG is +# an absolute path that does not exist, bail with a specific message -- +# a stale export from a prior shell session is the usual cause, and +# silently falling through to a confusing later error is worse UX than +# naming it here. +sub pg_config_bin +{ + my $explicit = env_or('PG_CONFIG', undef); + return 'pg_config' unless defined $explicit; + if ($explicit =~ m{/} and !-x $explicit) + { + BAIL_OUT("PG_CONFIG='$explicit' but no such executable exists. " + . "Likely a stale export from a previous shell session: " + . "run `unset PG_CONFIG` (so the test falls back to " + . "`pg_config` on PATH) or set PG_CONFIG to a real path."); + } + return $explicit; +} + +my $configure_flags = env_or('SPOCK_TEST_PG_CONFIGURE', + '--without-icu --without-readline --without-zlib'); +# $old_spock_ref and $temp_base are computed below, after the local +# spock repo is known. + +# Locate a local PostgreSQL git repo to clone from. +sub discover_local_pg_repo +{ + my ($spock_repo) = @_; + + my @candidates; + push @candidates, $ENV{PG_SRCDIR} if defined $ENV{PG_SRCDIR}; + push @candidates, abs_path("$spock_repo/../.."); + push @candidates, abs_path("$spock_repo/../postgres"); + + for my $c (@candidates) + { + next unless defined $c and $c ne ''; + return $c if -d "$c/.git" and -d "$c/src/backend"; + } + + return undef; +} + +# Major from pg_config. PG_CONFIG env var wins; otherwise we look up +# `pg_config` on PATH (which `make check_prove` sets up via the spock +# Makefile). For an explicit SPOCK_TEST_PG_BRANCH override we recover +# the major from the ref itself. +sub detect_pg_major_from_pg_config +{ + my $pgc = pg_config_bin(); + my $ver = qx('$pgc' --version 2>/dev/null); + return undef if $? != 0; + return $1 if $ver =~ /\bPostgreSQL\s+(\d+)/; + return undef; +} + +# Latest tag reachable from a given branch. Both PG and spock tag +# every release on their respective STABLE branches, so `git describe +# --tags --abbrev=0` lands on the most recent tag automatically -- +# REL_17_9 on a shipped PG, v5.0.7 on origin/v5_STABLE, BETA tags +# mid-cycle. Returns undef if the branch is missing or has no tag +# reachable -- caller falls back to the branch name itself. +sub latest_tag_on_branch +{ + my ($branch, $local_repo) = @_; + my $tag = qx(git -C '$local_repo' describe --tags --abbrev=0 '$branch' 2>/dev/null); + return undef if $? != 0; + chomp $tag; + return $tag eq '' ? undef : $tag; +} + +# Locate the local spock working tree. +my $cwd = getcwd(); +my $local_spock_repo; +if ($cwd =~ m{^(/.+?)/tests/tap/t/?$}) { $local_spock_repo = $1; } +elsif ($cwd =~ m{^(/.+?)/tests/tap/?$}) { $local_spock_repo = $1; } +else { $local_spock_repo = abs_path($cwd); } + +BAIL_OUT("cannot find local spock working tree at '$local_spock_repo' " + . "(no Makefile)") + unless -f "$local_spock_repo/Makefile"; + +# Default work dir: under tests/tap/tmp_check, alongside other spock TAP +# state. tmp_check is in spock's .gitignore and is not in EXTRA_CLEAN, +# so the cache survives `make clean`. +my $temp_base = env_or('SPOCK_TEST_TEMP_BASE', + "$local_spock_repo/tests/tap/tmp_check/030_pg_upgrade"); + +my $old_spock_ref = env_or('SPOCK_TEST_OLD_SPOCK_REF', 'origin/v5_STABLE'); + +# Discover (or accept an override of) the local PostgreSQL repo. +my $pg_repo = env_or('SPOCK_TEST_PG_REPO', undef) + // discover_local_pg_repo($local_spock_repo) + // BAIL_OUT('cannot locate local PostgreSQL source repo. spock is ' + . 'normally cloned under contrib/spock so its parent is the PG ' + . 'tree; if your layout differs, set SPOCK_TEST_PG_REPO to a path ' + . 'or URL.'); + +# Resolve the major (env override -> pg_config) and the ref to checkout +# (env override -> latest stable tag in the local repo -> STABLE branch). +my $env_branch = env_or('SPOCK_TEST_PG_BRANCH', undef); +my $pg_major; +if (defined $env_branch) +{ + ($pg_major) = ($env_branch =~ /^REL_?(\d+)/); + BAIL_OUT("cannot derive PG major version from " + . "SPOCK_TEST_PG_BRANCH='$env_branch'") + unless $pg_major; +} +else +{ + $pg_major = detect_pg_major_from_pg_config() + or BAIL_OUT('cannot determine PG major: pg_config did not return ' + . "a version. Set PG_CONFIG to a valid pg_config path, or set " + . "SPOCK_TEST_PG_BRANCH explicitly (current PG_CONFIG=" + . (env_or('PG_CONFIG', '')) . ")."); +} + +my $pg_branch = $env_branch + // latest_tag_on_branch("REL_${pg_major}_STABLE", $pg_repo) + // latest_tag_on_branch('HEAD', $pg_repo) + // 'HEAD'; + +BAIL_OUT("local spock has no patches/$pg_major (need patches for the " + . "PG major being tested)") + unless -d "$local_spock_repo/patches/$pg_major"; + +# Pre-flight: verify both refs we are about to depend on actually resolve +# in their respective repos. CI runners commonly checkout shallow or with +# limited refs, so origin/v5_STABLE may be absent unless the workflow +# unshallowed or fetched it. Fail here -- with a clear hint -- rather +# than minutes into the build when `git checkout` finally errors out. +sub git_ref_exists +{ + my ($repo, $ref) = @_; + return system( + "git -C '$repo' rev-parse --verify --quiet '$ref' >/dev/null 2>&1") + == 0; +} + +unless (git_ref_exists($local_spock_repo, $old_spock_ref)) +{ + if ($old_spock_ref =~ m{^origin/(.+)$}) + { + my $branch = $1; + note("ref '$old_spock_ref' missing locally; " + . "fetching tip of '$branch' from origin"); + system("git -C '$local_spock_repo' fetch --no-tags --depth=1 " + . "origin '$branch:refs/remotes/origin/$branch' " + . ">/dev/null 2>&1"); + } +} + +unless (git_ref_exists($local_spock_repo, $old_spock_ref)) +{ + my $how_set = $ENV{SPOCK_TEST_OLD_SPOCK_REF} + ? "from SPOCK_TEST_OLD_SPOCK_REF" + : "the default (origin/v5_STABLE)"; + BAIL_OUT("spock ref '$old_spock_ref' ($how_set) not found in " + . "'$local_spock_repo' and could not be fetched. Either the " + . "ref name is wrong (typo?), the clone has no origin remote, " + . "or the runner is offline. The manual recipe is " + . "`git -C $local_spock_repo fetch --no-tags origin " + . "v5_STABLE:refs/remotes/origin/v5_STABLE`."); +} + +unless (git_ref_exists($pg_repo, $pg_branch)) +{ + # The auto-resolution chain ends at 'HEAD' which is always present + # in a non-empty repo, so reaching here means the user explicitly + # named a ref that does not resolve -- treat as a typo and bail. + BAIL_OUT("PostgreSQL ref '$pg_branch' (from SPOCK_TEST_PG_BRANCH) " + . "not found in '$pg_repo'. Either the ref name is wrong " + . "(typo?) or your clone has not fetched it -- a shallow " + . "checkout typically needs " + . "`git -C $pg_repo fetch --tags origin $pg_branch`."); +} + +# Layout under $temp_base. +my $old_pg_src = "$temp_base/old_pg"; +my $new_pg_src = "$temp_base/new_pg"; +my $old_pg_install = "$temp_base/old_pg_install"; +my $new_pg_install = "$temp_base/new_pg_install"; +my $spock_src = "$temp_base/spock"; +my $build_log = "$temp_base/build.log"; + +# Each run starts from scratch. The spock Makefile deliberately keeps +# tmp_check/ between runs (so its other state is preserved), but for +# this test that means a previous failed run can leave both stale build +# artefacts under $temp_base and stale per-node data dirs that initdb +# refuses to overwrite. Clean only the paths owned by this test: +# - $temp_base (build cache) +# - $tap_tmp_check/t___data (Cluster data dirs) +my $testid = basename($0, '.pl'); +my $tap_tmp_check = "$local_spock_repo/tests/tap/tmp_check"; +remove_tree($temp_base) if -d $temp_base; +remove_tree($_) for glob "$tap_tmp_check/t_${testid}_*_data"; +make_path($temp_base); +{ open my $fh, '>', $build_log or die "open $build_log: $!"; close $fh; } + +# --------------------------------------------------------------------------- +# Bootstrap helpers (raw shell - we are building PostgreSQL itself) +# --------------------------------------------------------------------------- +sub run_build +{ + my (@cmd) = @_; + my $cmd_str = join(' ', @cmd); + note("BUILD: $cmd_str"); + my $rc = system("($cmd_str) >>'$build_log' 2>&1"); + if ($rc != 0) + { + diag("build step failed (exit " + . ($rc >> 8) . "): $cmd_str"); + diag("--- last 60 lines of $build_log ---"); + diag(qx(tail -n 60 '$build_log')); + return 0; + } + return 1; +} + +sub ok_or_bail +{ + my ($cond, $name) = @_; + BAIL_OUT("bootstrap step failed: $name") unless ok($cond, $name); +} + +sub clone_shared_if_missing +{ + my ($src, $dest) = @_; + return 1 if -d "$dest/.git"; + return 0 unless run_build("git clone --shared '$src' '$dest'"); + + return run_build("git -C '$dest' fetch --update-shallow '$src' " + . "'+refs/remotes/origin/*:refs/remotes/origin/*'"); +} + +# Clone --shared and checkout a specific ref. Idempotent: a re-run sees +# the existing .git/ and skips both. If the user changes +# SPOCK_TEST_PG_BRANCH between runs they need to wipe $TEMP_BASE -- we +# do not force-checkout, since the working tree carries our applied +# patches as unstaged changes after the first run. +sub clone_shared_and_checkout +{ + my ($src, $dest, $ref) = @_; + return 1 if -d "$dest/.git"; + return run_build("git clone --shared '$src' '$dest'") + && run_build("cd '$dest' && git checkout --quiet $ref"); +} + +sub apply_patches_if_pristine +{ + my ($pg_src, $patch_dir, $label) = @_; + my $marker = "$pg_src/.spock_patches_applied"; + return 1 if -f $marker; + unless (-d $patch_dir) + { + diag("missing patch dir: $patch_dir"); + return 0; + } + opendir(my $dh, $patch_dir) or die "opendir $patch_dir: $!"; + my @patches = sort grep { /\.diff$/ } readdir($dh); + closedir($dh); + for my $p (@patches) + { + note("$label: applying $p"); + # -N -f forces forward-only, never-prompt mode. Without these, + # macOS BSD patch can hit its "Reversed (or previously applied) + # patch detected! Assume -R? [y]" heuristic when a hunk's + # context lives near EOF, auto-answer yes against piped stdin, + # silently skip the hunk, and still return exit 0 -- yielding + # half-applied patchsets where a marker says "applied" but the + # file wasn't touched. -N -f turns that into a clean exit-1. + return 0 + unless run_build( + "cd '$pg_src' && patch -p1 -N -f < '$patch_dir/$p'"); + } + open my $fh, '>', $marker or die "marker $marker: $!"; + close $fh; + return 1; +} + +sub build_pg_if_missing +{ + my ($src, $install) = @_; + return 1 if -x "$install/bin/postgres"; + return run_build( + "cd '$src' && ./configure --prefix='$install' $configure_flags") + && run_build("cd '$src' && make -j4 install"); +} + +sub build_spock_if_missing +{ + my ($pg_install) = @_; + my $pgcfg = "$pg_install/bin/pg_config"; + my $libdir = qx('$pgcfg' --pkglibdir); + chomp $libdir; + return 1 if -f "$libdir/spock.so" or -f "$libdir/spock.dylib"; + + # Force a clean: spock objects from the previous variant's build are + # linked against the other PG. + return run_build("cd '$spock_src' && make clean PG_CONFIG='$pgcfg' || true") + && run_build("cd '$spock_src' && make PG_CONFIG='$pgcfg'") + && run_build("cd '$spock_src' && make install PG_CONFIG='$pgcfg'"); +} + +sub build_variant +{ + my ($variant, $spock_ref, $pg_src, $pg_install) = @_; + + ok_or_bail( + run_build("cd '$spock_src' && git checkout --quiet --force $spock_ref"), + "$variant: checkout spock $spock_ref"); + ok_or_bail( + apply_patches_if_pristine( + $pg_src, "$spock_src/patches/$pg_major", "$variant PG"), + "$variant: apply spock patches to PG"); + ok_or_bail(build_pg_if_missing($pg_src, $pg_install), + "$variant: build+install PostgreSQL"); + ok_or_bail(build_spock_if_missing($pg_install), + "$variant: build+install spock"); +} + +# --------------------------------------------------------------------------- +# Prerequisites +# --------------------------------------------------------------------------- +for my $tool (qw(git patch make)) +{ + plan skip_all => "required tool '$tool' not found in PATH" + unless system("which $tool >/dev/null 2>&1") == 0; +} + +note("PG repo: $pg_repo"); +note("PG ref: $pg_branch (major $pg_major" + . ($env_branch ? ", explicit" : ", auto-resolved") + . ")"); +note("Old spock ref: $old_spock_ref"); +note("Local spock repo: $local_spock_repo"); +note("Temp base: $temp_base"); +note("Build log: $build_log"); + +# --------------------------------------------------------------------------- +# Bootstrap: clone PG once from the local repo, mirror for the new tree, +# clone spock once, then build each variant. +# --------------------------------------------------------------------------- +ok_or_bail(clone_shared_and_checkout($pg_repo, $old_pg_src, $pg_branch), + "clone PostgreSQL from $pg_repo @ $pg_branch"); +ok_or_bail(clone_shared_if_missing($old_pg_src, $new_pg_src), + "mirror PostgreSQL tree for new build (--shared)"); +ok_or_bail(clone_shared_if_missing($local_spock_repo, $spock_src), + "clone local spock working tree"); + +my $new_spock_ref = qx(cd '$spock_src' && git rev-parse HEAD); +chomp $new_spock_ref; +BAIL_OUT("could not capture HEAD of $spock_src") + unless $new_spock_ref =~ /^[0-9a-f]{40}$/; +note("New spock ref: $new_spock_ref (captured from local HEAD)"); + +build_variant('old', $old_spock_ref, $old_pg_src, $old_pg_install); +build_variant('new', $new_spock_ref, $new_pg_src, $new_pg_install); + +# PostgreSQL::Test::Cluster->init() invokes +# `$ENV{PG_REGRESS} --config-auth ` +# to set up pg_hba.conf. The spock Makefile sets PG_REGRESS via +# PG_REGRESS='$(top_builddir)/src/test/regress/pg_regress' +# but for a PGXS extension `top_builddir` resolves to a path that does +# not contain pg_regress, so $ENV{PG_REGRESS} ends up pointing at a +# non-existent file (or empty string), and Cluster's `system_log` on +# that path warns "Use of uninitialized value" and dies. Point it at +# the pg_regress we just built instead -- both variants ship one in +# their source tree after `make install` completes. +my $built_regress = "$new_pg_src/src/test/regress/pg_regress"; +BAIL_OUT("expected pg_regress at '$built_regress' but file is missing") + unless -x $built_regress; +$ENV{PG_REGRESS} = $built_regress; + +# --------------------------------------------------------------------------- +# Set up two clusters via PostgreSQL::Test::Cluster +# --------------------------------------------------------------------------- +my $old_node = PostgreSQL::Test::Cluster->new('spock_old', + install_path => $old_pg_install); +my $new_node = PostgreSQL::Test::Cluster->new('spock_new', + install_path => $new_pg_install); + +# Use a stable locale/encoding so pg_upgrade does not refuse to run. +my @initdb_extra = ('--locale', 'C', '--encoding', 'UTF8'); + +$old_node->init(extra => \@initdb_extra); +$new_node->init(extra => \@initdb_extra); + +my $spock_conf = q{ +wal_level = logical +shared_preload_libraries = 'spock' +track_commit_timestamp = on +max_replication_slots = 10 +max_wal_senders = 10 +max_worker_processes = 20 +}; +$old_node->append_conf('postgresql.conf', $spock_conf); +$new_node->append_conf('postgresql.conf', $spock_conf); + +# --------------------------------------------------------------------------- +# Old cluster: legacy 5.x delta-apply reloption in two databases +# --------------------------------------------------------------------------- +$old_node->start; + +$old_node->safe_psql('postgres', 'CREATE DATABASE regression'); +$old_node->safe_psql('postgres', 'CREATE DATABASE spock_2'); + +my %legacy_marked = (regression => 'test', spock_2 => 'test_1'); +for my $db (sort keys %legacy_marked) +{ + my $tbl = $legacy_marked{$db}; + $old_node->safe_psql($db, 'CREATE EXTENSION spock'); + $old_node->safe_psql($db, "CREATE TABLE $tbl (x serial primary key)"); + $old_node->safe_psql($db, + "ALTER TABLE $tbl ALTER COLUMN x SET " + . "(log_old_value=true, delta_apply_function=spock.delta_apply)"); +} + +my $extver; + +$extver = $old_node->safe_psql('regression', 'SELECT spock.spock_version()'); +like($extver, qr/^5\./, "old cluster runs spock 5.x ($extver)"); + +$old_node->stop; + +# --------------------------------------------------------------------------- +# pg_upgrade old -> new +# --------------------------------------------------------------------------- +command_ok( + [ + "$new_pg_install/bin/pg_upgrade", + '--no-sync', + '-d', $old_node->data_dir, + '-D', $new_node->data_dir, + '-b', "$old_pg_install/bin", + '-B', "$new_pg_install/bin", + '-p', $old_node->port, + '-P', $new_node->port, + ], + 'pg_upgrade old -> new'); + +$new_node->start; + +# --------------------------------------------------------------------------- +# New cluster: ALTER EXTENSION UPDATE; verify the shim left the seclabel +# --------------------------------------------------------------------------- +for my $db (sort keys %legacy_marked) +{ + my $tbl = $legacy_marked{$db}; + + # Spock auto-upgrade should make the UPDATE automatically during the start + $extver = $new_node->safe_psql($db, 'SELECT spock.spock_version()'); + like($extver, qr/^6\./, "$db: spock version is 6.x after upgrade ($extver)"); + + $new_node->safe_psql($db, 'ALTER EXTENSION spock UPDATE'); + + $extver = $new_node->safe_psql($db, 'SELECT spock.spock_version()'); + like($extver, qr/^6\./, "$db: spock version is 6.x after upgrade ($extver)"); + + # Visible signature of spock_bucompat_5x.c: a pg_seclabel row scoped + # to the spock provider on the column we marked in the old cluster. + my $cnt = $new_node->safe_psql( + $db, qq{ + SELECT count(*) + FROM pg_seclabel sl + JOIN pg_class c ON c.oid = sl.objoid + WHERE sl.provider = 'spock' + AND c.relname = '$tbl' + }); + cmp_ok($cnt, '>=', 1, + "$db: legacy reloption on $tbl rewritten as spock security label"); + + my $label = $new_node->safe_psql( + $db, qq{ + SELECT label + FROM pg_seclabel sl + JOIN pg_class c ON c.oid = sl.objoid + WHERE sl.provider = 'spock' + AND c.relname = '$tbl' + LIMIT 1 + }); + is($label, 'spock.delta_apply', + "$db: seclabel value is the canonical 'spock.delta_apply'"); +} + +$new_node->stop; + +note("artefacts left under $temp_base for re-runs / inspection"); +note("delete $temp_base to force a clean rebuild"); + +done_testing(); From 58ed81a315b0143162e6af197a79a23e85440882 Mon Sep 17 00:00:00 2001 From: "Andrei V. Lepikhov" Date: Fri, 8 May 2026 15:20:42 +0200 Subject: [PATCH 6/6] tests: 002_create_subscriber: use sync_event/wait_for_sync_event Switch the post-INSERT wait from spock.sub_wait_for_sync (which polls for "caught up" and races with apply progress on busy CI) to the deterministic sync_event-on-provider / wait_for_sync_event-on-subscriber pattern. Removes a known source of flakiness on the buildfarm; pure test-only change. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/tap/t/002_create_subscriber.pl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/tap/t/002_create_subscriber.pl b/tests/tap/t/002_create_subscriber.pl index 78b058fc..159020c4 100755 --- a/tests/tap/t/002_create_subscriber.pl +++ b/tests/tap/t/002_create_subscriber.pl @@ -113,7 +113,10 @@ # Test 13: Insert more data and verify replication system_or_bail "$pg_bin/psql", '-p', $node_ports->[0], '-d', $dbname, '-c', "INSERT INTO test_subscription_data (name, value) VALUES ('test3', 300)"; -system_or_bail "$pg_bin/psql", '-q', '-p', $node_ports->[1], '-d', $dbname, '-c', "SELECT spock.sub_wait_for_sync('test_subscription')"; +my $sync_lsn = `$pg_bin/psql -p $node_ports->[0] -d $dbname -t -A -c "SELECT spock.sync_event()"`; +chomp($sync_lsn); +$sync_lsn =~ s/\s+//g; +system_or_bail "$pg_bin/psql", '-q', '-p', $node_ports->[1], '-d', $dbname, '-c', "CALL spock.wait_for_sync_event(NULL, 'n1', '$sync_lsn'::pg_lsn, 60)"; my $count_subscriber_updated = `$pg_bin/psql -p $node_ports->[1] -d $dbname -t -c "SELECT COUNT(*) FROM test_subscription_data"`; chomp($count_subscriber_updated);