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/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/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; diff --git a/src/spock.c b/src/spock.c index 9b7907e1..f97791d0 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 @@ -1004,6 +1007,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, @@ -1293,6 +1310,23 @@ _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); + + /* + * 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; @@ -1326,8 +1360,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_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); +} 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; } 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/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); 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();