Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/spock.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#include "spock_fe.h"
#include "spock_node.h"

#define SPOCK_VERSION "6.0.0-devel"
#define SPOCK_VERSION "6.0.0"
#define SPOCK_VERSION_NUM 60000

#define EXTENSION_NAME "spock"
Expand Down
163 changes: 163 additions & 0 deletions sql/spock--5.0.6--5.0.7.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@

/* spock--5.0.6--5.0.7.sql */

-- complain if script is sourced in psql, rather than via ALTER EXTENSION
\echo Use "ALTER EXTENSION spock UPDATE TO '5.0.7'" to load this file. \quit

CREATE FUNCTION spock.pause_apply_workers()
RETURNS void VOLATILE LANGUAGE c AS 'MODULE_PATHNAME', 'spock_pause_apply_workers';

CREATE FUNCTION spock.resume_apply_workers()
RETURNS void VOLATILE LANGUAGE c AS 'MODULE_PATHNAME', 'spock_resume_apply_workers';

REVOKE EXECUTE ON FUNCTION spock.pause_apply_workers() FROM PUBLIC;
REVOKE EXECUTE ON FUNCTION spock.resume_apply_workers() FROM PUBLIC;

DROP PROCEDURE IF EXISTS spock.wait_for_sync_event(OUT bool, oid, pg_lsn, int);
DROP PROCEDURE IF EXISTS spock.wait_for_sync_event(OUT bool, oid, pg_lsn, int, bool);
DROP PROCEDURE IF EXISTS spock.wait_for_sync_event(OUT bool, name, pg_lsn, int);
DROP PROCEDURE IF EXISTS spock.wait_for_sync_event(OUT bool, name, pg_lsn, int, bool);
CREATE PROCEDURE spock.wait_for_sync_event(
OUT result bool,
origin_id oid,
lsn pg_lsn,
timeout int DEFAULT 0,
wait_if_disabled bool DEFAULT false
) AS $$
DECLARE
target_id oid;
start_time timestamptz := clock_timestamp();
progress_lsn pg_lsn;
sub_is_enabled bool;
sub_slot name;
BEGIN
IF origin_id IS NULL THEN
RAISE EXCEPTION 'Invalid NULL origin_id';
END IF;
target_id := node_id FROM spock.node_info();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Expect no matches after the fix.
rg -nP ':\=\s*[A-Za-z_][A-Za-z0-9_]*\s+FROM\s+' sql/spock--5.0.6--5.0.7.sql

Repository: pgEdge/spock

Length of output: 173


🏁 Script executed:

cat -n sql/spock--5.0.6--5.0.7.sql | head -50

Repository: pgEdge/spock

Length of output: 2380


🏁 Script executed:

cat -n sql/spock--5.0.6--5.0.7.sql | sed -n '25,50p'

Repository: pgEdge/spock

Length of output: 1133


🏁 Script executed:

cat -n sql/spock--5.0.6--5.0.7.sql | sed -n '100,120p'

Repository: pgEdge/spock

Length of output: 937


Fix invalid PL/pgSQL assignment-from-query syntax at lines 37 and 109.

The := ... FROM ... construct is not valid PL/pgSQL and will fail when the procedures are created during the upgrade.

Proposed fix
-	target_id := node_id FROM spock.node_info();
+	SELECT node_id INTO target_id FROM spock.node_info();
-	origin_id := node_id FROM spock.node WHERE node_name = origin;
+	SELECT node_id INTO origin_id FROM spock.node WHERE node_name = origin;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
target_id := node_id FROM spock.node_info();
SELECT node_id INTO target_id FROM spock.node_info();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sql/spock--5.0.6--5.0.7.sql` at line 37, Replace the invalid PL/pgSQL
assignment-from-query "target_id := node_id FROM spock.node_info();" with a
proper SELECT ... INTO form; e.g. use "SELECT node_id INTO target_id FROM
spock.node_info();" (or "SELECT INTO STRICT" if you expect exactly one row) for
both occurrences referencing target_id, node_id and spock.node_info() (the
instances around lines 37 and 109).


-- Upfront existence check is skipped when wait_if_disabled is true because
-- the subscription may not yet exist (e.g. a newly added node whose
-- subscriptions are still initializing). The loop below handles both the
-- not-found and disabled cases gracefully in that mode.
IF NOT wait_if_disabled THEN
SELECT sub_enabled, sub_slot_name INTO sub_is_enabled, sub_slot
FROM spock.subscription
WHERE sub_origin = origin_id AND sub_target = target_id;

IF NOT FOUND THEN
RAISE EXCEPTION 'No subscription found for replication % => %',
origin_id, target_id;
END IF;
END IF;

WHILE true LOOP
-- Re-check subscription state each iteration. Also re-fetches
-- sub_slot_name so the loop is self-contained when wait_if_disabled
-- is true and the pre-loop check was skipped.
SELECT sub_enabled, sub_slot_name INTO sub_is_enabled, sub_slot
FROM spock.subscription
WHERE sub_origin = origin_id AND sub_target = target_id;

IF NOT FOUND THEN
IF NOT wait_if_disabled THEN
RAISE EXCEPTION 'No subscription found for replication % => %',
origin_id, target_id;
END IF;
-- Subscription not yet created; fall through to sleep.
ELSIF NOT sub_is_enabled THEN
IF NOT wait_if_disabled THEN
RAISE EXCEPTION 'Subscription % => % has been disabled',
origin_id, target_id;
END IF;
-- Subscription still initializing; fall through to sleep.
ELSE
-- Subscription is enabled; check LSN progress.
-- Uses PostgreSQL's native origin tracking rather than spock.progress
SELECT remote_lsn INTO progress_lsn
FROM pg_replication_origin_status
WHERE external_id = sub_slot;

IF progress_lsn IS NOT NULL AND progress_lsn >= lsn THEN
result = true;
RETURN;
END IF;
END IF;

IF timeout <> 0 AND
EXTRACT(EPOCH FROM (clock_timestamp() - start_time)) >= timeout THEN
result := false;
RETURN;
END IF;

ROLLBACK;
PERFORM pg_sleep(0.2);
END LOOP;
END;
$$ LANGUAGE plpgsql;

CREATE PROCEDURE spock.wait_for_sync_event(
OUT result bool,
origin name,
lsn pg_lsn,
timeout int DEFAULT 0,
wait_if_disabled bool DEFAULT false
) AS $$
DECLARE
origin_id oid;
BEGIN
origin_id := node_id FROM spock.node WHERE node_name = origin;
IF origin_id IS NULL THEN
RAISE EXCEPTION 'Origin node ''%'' not found', origin;
END IF;
CALL spock.wait_for_sync_event(result, origin_id, lsn, timeout, wait_if_disabled);
END;
$$ LANGUAGE plpgsql;

-- spock.sync_event() gained an optional 'transactional' boolean argument
-- (default false). Drop the old zero-arg signature first so the upgrade
-- doesn't leave behind two overloads with overlapping zero-arg resolution.
DROP FUNCTION IF EXISTS spock.sync_event();
CREATE FUNCTION spock.sync_event(transactional boolean DEFAULT false)
RETURNS pg_lsn RETURNS NULL ON NULL INPUT
AS 'MODULE_PATHNAME', 'spock_create_sync_event'
LANGUAGE C VOLATILE;

/*
* Correct the declared type of spock.subscription.sub_skip_schema.
*
* The column was added as text in the 5.0.1--5.0.2 upgrade, but the C code
* has always treated it as text[] on both read and write paths
* (strlist_to_textarray on write, DatumGetArrayTypeP on read). The bytes
* already on disk are therefore a valid ArrayType; only the catalog's type
* label is wrong. ALTER TABLE ... ALTER COLUMN TYPE text[] USING ... is
* not viable here: there is no SQL expression that converts "varlena bytes
* the planner believes are text but are in fact ArrayType internal format"
* back into an ArrayType Datum. Relabel the column in place so SQL-level
* access (SELECT, unnest, etc.) works as users expect, without rewriting
* data.
*/
LOCK TABLE spock.subscription IN ACCESS EXCLUSIVE MODE;

UPDATE pg_catalog.pg_attribute
SET atttypid = 'text[]'::regtype,
attndims = 1
WHERE attrelid = 'spock.subscription'::regclass
AND attname = 'sub_skip_schema'
AND atttypid = 'text'::regtype;

/*
* Drop any pg_statistic rows for the column. Stats sampled when the
* column was labelled text encode varlena bytes with text semantics;
* after the relabel the planner would interpret the same stavalues
* arrays as text[], producing nonsense selectivities (and possibly
* crashing on operators that validate ArrayType structure). ANALYZE
* will repopulate as needed.
*/
DELETE FROM pg_catalog.pg_statistic
WHERE starelid = 'spock.subscription'::regclass
AND staattnum = (
SELECT attnum
FROM pg_catalog.pg_attribute
WHERE attrelid = 'spock.subscription'::regclass
AND attname = 'sub_skip_schema');
6 changes: 6 additions & 0 deletions sql/spock--5.0.7--5.0.8.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/* spock--5.0.7--5.0.8.sql */

-- complain if script is sourced in psql, rather than via ALTER EXTENSION
\echo Use "ALTER EXTENSION spock UPDATE TO '5.0.8'" to load this file. \quit

-- No schema changes in 5.0.8.
156 changes: 26 additions & 130 deletions sql/spock--5.0.6--6.0.0-devel.sql → sql/spock--5.0.8--6.0.0.sql
Original file line number Diff line number Diff line change
@@ -1,12 +1,30 @@
/* spock--5.0.6--6.0.0-devel.sql */
/* spock--5.0.8--6.0.0.sql */

-- complain if script is sourced in psql, rather than via ALTER EXTENSION
\echo Use "ALTER EXTENSION spock UPDATE TO '6.0.0-devel'" to load this file. \quit
\echo Use "ALTER EXTENSION spock UPDATE TO '6.0.0'" to load this file. \quit
Comment on lines +1 to +4
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Retarget this direct upgrade to 5.0.7 -> 6.0.0.

This script still advertises 5.0.8 -> 6.0.0, but the release chain in this PR is 5.0.6 -> 5.0.7 -> 6.0.0. Leaving it here means the shipped upgrade artifact and the new validation path are aimed at the wrong source version.

Based on learnings: Treat intermediate upgrade SQL scripts as part of the upgrade chain and only flag issues as bugs if they affect the final upgrade outcome or are not corrected later in the chain.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sql/spock--5.0.8--6.0.0.sql` around lines 1 - 4, The script header and user
message still target 5.0.8->6.0.0; change both to reflect the correct upgrade
source 5.0.7->6.0.0 by renaming the file comment string "/*
spock--5.0.8--6.0.0.sql */" to "/* spock--5.0.7--6.0.0.sql */" (and if the
actual filename is the old form, rename the file accordingly) and update the
psql echo text that contains "ALTER EXTENSION spock UPDATE TO '6.0.0'" to
clearly reference the 5.0.7 source (e.g., mirror the filename change in the
message so the script and its user-facing guidance consistently indicate 5.0.7
-> 6.0.0).


-- Drop functions removed from the 6.0.0 fresh install (present since 5.0.0 but no longer needed)
DROP FUNCTION IF EXISTS spock.convert_column_to_int8(regclass, smallint);
DROP FUNCTION IF EXISTS spock.convert_sequence_to_snowflake(regclass);

-- Add IMMUTABLE PARALLEL SAFE to md5_agg_sfunc (was missing in earlier definitions)
CREATE OR REPLACE FUNCTION spock.md5_agg_sfunc(text, anyelement)
RETURNS text
AS $$ SELECT md5($1 || $2::text) $$
LANGUAGE sql IMMUTABLE PARALLEL SAFE;

-- Add named parameters to spock_gen_slot_name (originally created without names in 5.0.0)
CREATE OR REPLACE FUNCTION spock.spock_gen_slot_name(
dbname name,
provider_node name,
subscription name
) RETURNS name
AS 'MODULE_PATHNAME'
LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;

DROP VIEW IF EXISTS spock.lag_tracker;
DROP TABLE IF EXISTS spock.progress;

DROP FUNCTION IF EXISTS spock.apply_group_progress;
CREATE FUNCTION spock.apply_group_progress (
OUT dbid oid,
OUT node_id oid,
Expand All @@ -26,23 +44,10 @@ LANGUAGE c AS 'MODULE_PATHNAME', 'get_apply_group_progress';
-- for internal use only.
CREATE VIEW spock.progress AS
SELECT * FROM spock.apply_group_progress()
WHERE dbid = (
SELECT oid FROM pg_database WHERE datname = current_database()
);
WHERE dbid = (
SELECT oid FROM pg_database WHERE datname = current_database()
);

CREATE FUNCTION spock.pause_apply_workers()
RETURNS void
AS 'MODULE_PATHNAME', 'spock_pause_apply_workers'
LANGUAGE C VOLATILE;

REVOKE ALL ON FUNCTION spock.pause_apply_workers() FROM PUBLIC;

CREATE FUNCTION spock.resume_apply_workers()
RETURNS void
AS 'MODULE_PATHNAME', 'spock_resume_apply_workers'
LANGUAGE C VOLATILE;

REVOKE ALL ON FUNCTION spock.resume_apply_workers() FROM PUBLIC;

-- Read peer progress (ros.remote_lsn) for all peer subscriptions.
-- Called while apply workers are paused and the slot's snapshot is imported.
Expand Down Expand Up @@ -85,7 +90,7 @@ BEGIN

RAISE NOTICE 'SPOCK cswp slot=% v_lsn=%', p_slot_name, v_lsn;

-- Header row: lsn + snapshot only.
-- Header row: lsn only (snapshot managed by C caller).
lsn := v_lsn;
snapshot := v_snap;
RETURN NEXT;
Expand Down Expand Up @@ -323,120 +328,11 @@ END;
$$ LANGUAGE plpgsql STRICT VOLATILE;


-- spock.sync_event() gained an optional 'transactional' boolean argument
-- (default false). Drop the old zero-arg signature first so the upgrade
-- doesn't leave behind two overloads with overlapping zero-arg resolution.
DROP FUNCTION IF EXISTS spock.sync_event();
CREATE FUNCTION spock.sync_event(transactional boolean DEFAULT false)
RETURNS pg_lsn RETURNS NULL ON NULL INPUT
AS 'MODULE_PATHNAME', 'spock_create_sync_event'
LANGUAGE C VOLATILE;

DROP PROCEDURE IF EXISTS spock.wait_for_sync_event(OUT bool, oid, pg_lsn, int);
DROP PROCEDURE IF EXISTS spock.wait_for_sync_event(OUT bool, oid, pg_lsn, int, bool);
DROP PROCEDURE IF EXISTS spock.wait_for_sync_event(OUT bool, name, pg_lsn, int);
DROP PROCEDURE IF EXISTS spock.wait_for_sync_event(OUT bool, name, pg_lsn, int, bool);
CREATE PROCEDURE spock.wait_for_sync_event(
OUT result bool,
origin_id oid,
lsn pg_lsn,
timeout int DEFAULT 0,
wait_if_disabled bool DEFAULT false
) AS $$
DECLARE
target_id oid;
start_time timestamptz := clock_timestamp();
progress_lsn pg_lsn;
sub_is_enabled bool;
sub_slot name;
BEGIN
IF origin_id IS NULL THEN
RAISE EXCEPTION 'Invalid NULL origin_id';
END IF;
target_id := node_id FROM spock.node_info();

-- Upfront existence check is skipped when wait_if_disabled is true because
-- the subscription may not yet exist (e.g. a newly added node whose
-- subscriptions are still initializing). The loop below handles both the
-- not-found and disabled cases gracefully in that mode.
IF NOT wait_if_disabled THEN
SELECT sub_enabled, sub_slot_name INTO sub_is_enabled, sub_slot
FROM spock.subscription
WHERE sub_origin = origin_id AND sub_target = target_id;

IF NOT FOUND THEN
RAISE EXCEPTION 'No subscription found for replication % => %',
origin_id, target_id;
END IF;
END IF;

WHILE true LOOP
-- Re-check subscription state each iteration. Also re-fetches
-- sub_slot_name so the loop is self-contained when wait_if_disabled
-- is true and the pre-loop check was skipped.
SELECT sub_enabled, sub_slot_name INTO sub_is_enabled, sub_slot
FROM spock.subscription
WHERE sub_origin = origin_id AND sub_target = target_id;

IF NOT FOUND THEN
IF NOT wait_if_disabled THEN
RAISE EXCEPTION 'No subscription found for replication % => %',
origin_id, target_id;
END IF;
-- Subscription not yet created; fall through to sleep.
ELSIF NOT sub_is_enabled THEN
IF NOT wait_if_disabled THEN
RAISE EXCEPTION 'Subscription % => % has been disabled',
origin_id, target_id;
END IF;
-- Subscription still initializing; fall through to sleep.
ELSE
-- Subscription is enabled; check LSN progress.
-- Uses PostgreSQL's native origin tracking rather than spock.progress
SELECT remote_lsn INTO progress_lsn
FROM pg_replication_origin_status
WHERE external_id = sub_slot;

IF progress_lsn IS NOT NULL AND progress_lsn >= lsn THEN
result = true;
RETURN;
END IF;
END IF;

IF timeout <> 0 AND
EXTRACT(EPOCH FROM (clock_timestamp() - start_time)) >= timeout THEN
result := false;
RETURN;
END IF;

ROLLBACK;
PERFORM pg_sleep(0.2);
END LOOP;
END;
$$ LANGUAGE plpgsql;

CREATE PROCEDURE spock.wait_for_sync_event(
OUT result bool,
origin name,
lsn pg_lsn,
timeout int DEFAULT 0,
wait_if_disabled bool DEFAULT false
) AS $$
DECLARE
origin_id oid;
BEGIN
origin_id := node_id FROM spock.node WHERE node_name = origin;
IF origin_id IS NULL THEN
RAISE EXCEPTION 'Origin node ''%'' not found', origin;
END IF;
CALL spock.wait_for_sync_event(result, origin_id, lsn, timeout, wait_if_disabled);
END;
$$ LANGUAGE plpgsql;

CREATE FUNCTION spock.sub_alter_options(
subscription_name name,
options jsonb
)
RETURNS boolean
AS 'MODULE_PATHNAME', 'spock_alter_subscription_options'
LANGUAGE C STRICT VOLATILE;

File renamed without changes.
2 changes: 1 addition & 1 deletion tests/regress/expected/init_1.out
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ CREATE EXTENSION IF NOT EXISTS spock;
List of installed extensions
Name | Version | Default version | Schema | Description
-------+-------------+-----------------+--------+--------------------------------
spock | 6.0.0-devel | 6.0.0-devel | spock | PostgreSQL Logical Replication
spock | 6.0.0 | 6.0.0 | spock | PostgreSQL Logical Replication
(1 row)

SELECT * FROM spock.node_create(node_name := 'test_provider', dsn := (SELECT provider_dsn FROM spock_regress_variables()) || ' user=super');
Expand Down
2 changes: 1 addition & 1 deletion tests/regress/expected/init_fail.out
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ SET client_min_messages = 'warning';
\set VERBOSITY terse
DO $$
BEGIN
CREATE EXTENSION IF NOT EXISTS spock VERSION '6.0.0-devel';
CREATE EXTENSION IF NOT EXISTS spock VERSION '6.0.0';
END;
$$;
ALTER EXTENSION spock UPDATE;
Expand Down
2 changes: 1 addition & 1 deletion tests/regress/sql/init_fail.sql
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ SET client_min_messages = 'warning';

DO $$
BEGIN
CREATE EXTENSION IF NOT EXISTS spock VERSION '6.0.0-devel';
CREATE EXTENSION IF NOT EXISTS spock VERSION '6.0.0';
END;
$$;
ALTER EXTENSION spock UPDATE;
Expand Down
Loading
Loading