From dbbd425904a6765d39eccdfaa652a966a51ef29a Mon Sep 17 00:00:00 2001 From: Mason Sharp Date: Fri, 17 Apr 2026 13:12:25 -0700 Subject: [PATCH 1/9] Revert "Rename delta_apply related variables." This reverts commit 1bea9e8e34ece97cf6f0dc3016ee2f83d04b36a7. --- include/spock.h | 2 +- include/spock_relcache.h | 4 ++-- src/spock.c | 2 +- src/spock_apply_heap.c | 6 +++--- src/spock_executor.c | 4 ++-- src/spock_relcache.c | 28 ++++++++++++++-------------- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/include/spock.h b/include/spock.h index 2d96941f..457e79fc 100644 --- a/include/spock.h +++ b/include/spock.h @@ -28,7 +28,7 @@ #define SPOCK_VERSION_NUM 60000 #define EXTENSION_NAME "spock" -#define SPOCK_SECLABEL_PROVIDER "spock" +#define spock_SECLABEL_PROVIDER "spock" #define REPLICATION_ORIGIN_ALL "all" diff --git a/include/spock_relcache.h b/include/spock_relcache.h index a9a689de..0ae29657 100644 --- a/include/spock_relcache.h +++ b/include/spock_relcache.h @@ -48,8 +48,8 @@ typedef struct SpockRelation /* Additional cache, only valid as long as relation mapping is. */ bool hasTriggers; - Oid *delta_apply_functions; - bool has_delta_columns; + Oid *delta_functions; + bool has_delta_apply; } SpockRelation; extern void spock_relation_cache_update(uint32 remoteid, diff --git a/src/spock.c b/src/spock.c index 8def4d39..a5cb19c1 100644 --- a/src/spock.c +++ b/src/spock.c @@ -1278,7 +1278,7 @@ _PG_init(void) emit_log_hook = log_message_filter; /* Security label provider hook */ - register_label_provider(SPOCK_SECLABEL_PROVIDER, spock_object_relabel); + register_label_provider(spock_SECLABEL_PROVIDER, spock_object_relabel); #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..1a790d73 100644 --- a/src/spock_apply_heap.c +++ b/src/spock_apply_heap.c @@ -532,7 +532,7 @@ build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, int remoteattnum = rel->attmap[attidx]; Assert(remoteattnum < tupdesc->natts); - if (rel->delta_apply_functions[remoteattnum] == InvalidOid) + if (rel->delta_functions[remoteattnum] == InvalidOid) { deltatup->values[remoteattnum] = 0xdeadbeef; deltatup->nulls[remoteattnum] = true; @@ -560,7 +560,7 @@ build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, &loc_isnull); Assert(!loc_isnull); - result = OidFunctionCall3Coll(rel->delta_apply_functions[remoteattnum], + result = OidFunctionCall3Coll(rel->delta_functions[remoteattnum], InvalidOid, oldtup->values[remoteattnum], newtup->values[remoteattnum], loc_value); deltatup->values[remoteattnum] = result; @@ -638,7 +638,7 @@ spock_handle_conflict_and_apply(SpockRelation *rel, EState *estate, xmin, local_origin_found, local_origin, local_ts, idxused); - if (rel->has_delta_columns) + if (rel->has_delta_apply) { SpockTupleData deltatup; HeapTuple currenttuple; diff --git a/src/spock_executor.c b/src/spock_executor.c index b5d4f51d..8e1df23f 100644 --- a/src/spock_executor.c +++ b/src/spock_executor.c @@ -215,7 +215,7 @@ DeleteSecurityLabels(const char *provider) Assert(!isnull); provider = TextDatumGetCString(datum); - if (strcmp(provider, SPOCK_SECLABEL_PROVIDER) != 0) + if (strcmp(provider, spock_SECLABEL_PROVIDER) != 0) continue; CatalogTupleDelete(pg_seclabel, &htup->t_self); @@ -279,7 +279,7 @@ spock_object_access(ObjectAccessType access, if (dropping_spock_obj) { /* Need to drop any security labels created by the extension */ - DeleteSecurityLabels(SPOCK_SECLABEL_PROVIDER); + DeleteSecurityLabels(spock_SECLABEL_PROVIDER); return; } diff --git a/src/spock_relcache.c b/src/spock_relcache.c index 182957e5..34049b36 100644 --- a/src/spock_relcache.c +++ b/src/spock_relcache.c @@ -55,13 +55,13 @@ relcache_free_entry(SpockRelation *entry) if (entry->attmap) pfree(entry->attmap); - if (entry->delta_apply_functions) - pfree(entry->delta_apply_functions); + if (entry->delta_functions) + pfree(entry->delta_functions); entry->natts = 0; entry->reloid = InvalidOid; entry->rel = NULL; - entry->has_delta_columns = false; + entry->has_delta_apply = false; } @@ -117,7 +117,7 @@ spock_relation_open(uint32 remoteid, LOCKMODE lockmode) ObjectAddressSubSet(object, RelationRelationId, RelationGetRelid(entry->rel), entry->attmap[i] + 1); - seclabel = GetSecurityLabel(&object, SPOCK_SECLABEL_PROVIDER); + seclabel = GetSecurityLabel(&object, spock_SECLABEL_PROVIDER); if (seclabel != NULL) { Form_pg_attribute att; @@ -132,21 +132,21 @@ spock_relation_open(uint32 remoteid, LOCKMODE lockmode) entry->nspname, entry->relname, entry->attnames[i], seclabel); - entry->delta_apply_functions[entry->attmap[i]] = dfunc; - Assert(entry->delta_apply_functions[entry->attmap[i]] != InvalidOid); - entry->has_delta_columns = true; + entry->delta_functions[entry->attmap[i]] = dfunc; + Assert(entry->delta_functions[entry->attmap[i]] != InvalidOid); + entry->has_delta_apply = true; } else { /* Main case */ - entry->delta_apply_functions[entry->attmap[i]] = InvalidOid; + entry->delta_functions[entry->attmap[i]] = InvalidOid; } } relinfo = makeNode(ResultRelInfo); InitResultRelInfo(relinfo, entry->rel, 1, NULL, 0); entry->reloid = RelationGetRelid(entry->rel); - if (entry->has_delta_columns) + if (entry->has_delta_apply) { /* * It looks like a hack — which, in fact, it is. @@ -232,8 +232,8 @@ spock_relation_cache_update(uint32 remoteid, char *schemaname, entry->attrtypmods[i] = attrtypmods[i]; } entry->attmap = palloc(natts * sizeof(int)); - entry->has_delta_columns = false; - entry->delta_apply_functions = (Oid *) palloc0(entry->natts * sizeof(Oid)); + entry->has_delta_apply = false; + entry->delta_functions = (Oid *) palloc0(entry->natts * sizeof(Oid)); MemoryContextSwitchTo(oldcontext); /* XXX Should we validate the relation against local schema here? */ @@ -270,8 +270,8 @@ spock_relation_cache_updater(SpockRemoteRel *remoterel) for (i = 0; i < remoterel->natts; i++) entry->attnames[i] = pstrdup(remoterel->attnames[i]); entry->attmap = palloc(remoterel->natts * sizeof(int)); - entry->has_delta_columns = false; - entry->delta_apply_functions = (Oid *) palloc0(entry->natts * sizeof(Oid)); + entry->has_delta_apply = false; + entry->delta_functions = (Oid *) palloc0(entry->natts * sizeof(Oid)); MemoryContextSwitchTo(oldcontext); /* XXX Should we validate the relation against local schema here? */ @@ -454,7 +454,7 @@ get_replication_identity(Relation rel) ObjectAddressSubSet(object, RelationRelationId, RelationGetRelid(rel), i + 1); - seclabel = GetSecurityLabel(&object, SPOCK_SECLABEL_PROVIDER); + seclabel = GetSecurityLabel(&object, spock_SECLABEL_PROVIDER); if (seclabel != NULL) { From 2ed4b8ba4f98d0f4a317f6e94c231f513d4f54c7 Mon Sep 17 00:00:00 2001 From: Mason Sharp Date: Fri, 17 Apr 2026 13:15:44 -0700 Subject: [PATCH 2/9] Revert "Disallow the delta apply feature on nullable columns." This reverts commit 7d9337be299ce1d505addbcdb4ef60016f0b2bcc. --- sql/spock--5.0.6--6.0.0-devel.sql | 17 +------- sql/spock--6.0.0-devel.sql | 17 +------- src/spock_apply_heap.c | 61 +++++++++++++++++----------- tests/regress/expected/autoddl.out | 2 +- tests/regress/expected/basic.out | 6 +-- tests/regress/expected/infofuncs.out | 2 +- tests/regress/sql/autoddl.sql | 2 +- tests/regress/sql/basic.sql | 6 +-- tests/regress/sql/infofuncs.sql | 2 +- tests/tap/t/012_delta_apply.pl | 11 ++--- 10 files changed, 52 insertions(+), 74 deletions(-) diff --git a/sql/spock--5.0.6--6.0.0-devel.sql b/sql/spock--5.0.6--6.0.0-devel.sql index f0078a09..e4a47829 100644 --- a/sql/spock--5.0.6--6.0.0-devel.sql +++ b/sql/spock--5.0.6--6.0.0-devel.sql @@ -246,7 +246,7 @@ BEGIN * Find proper delta_apply function for the column type or ERROR */ - SELECT t.typname,t.typinput,t.typoutput, a.attnotnull + SELECT t.typname,t.typinput,t.typoutput FROM pg_catalog.pg_attribute a, pg_type t WHERE a.attrelid = rel AND a.attname = att_name AND (a.atttypid = t.oid) INTO attdata; @@ -254,21 +254,6 @@ BEGIN RAISE EXCEPTION 'column % does not exist in the table %', att_name, rel; END IF; - IF (attdata.attnotnull = false) THEN - /* - * TODO: Here is a case where the table has different constraints on nodes. - * Using prepared transactions, we might be sure this operation will finish - * if only each node satisfies the rule. But we need to add support for 2PC - * commit beforehand. - */ - RAISE NOTICE USING - MESSAGE = format('delta_apply feature can not be applied to nullable column %L of the table %I', - att_name, rel), - HINT = 'Set NOT NULL constraint on the column', - ERRCODE = 'object_not_in_prerequisite_state'; - RETURN false; - END IF; - SELECT typname FROM pg_type WHERE typname IN ('int2','int4','int8','float4','float8','numeric','money') AND typinput = attdata.typinput AND typoutput = attdata.typoutput diff --git a/sql/spock--6.0.0-devel.sql b/sql/spock--6.0.0-devel.sql index 35cbebe1..41267599 100644 --- a/sql/spock--6.0.0-devel.sql +++ b/sql/spock--6.0.0-devel.sql @@ -897,7 +897,7 @@ BEGIN * Find proper delta_apply function for the column type or ERROR */ - SELECT t.typname,t.typinput,t.typoutput, a.attnotnull + SELECT t.typname,t.typinput,t.typoutput FROM pg_catalog.pg_attribute a, pg_type t WHERE a.attrelid = rel AND a.attname = att_name AND (a.atttypid = t.oid) INTO attdata; @@ -905,21 +905,6 @@ BEGIN RAISE EXCEPTION 'column % does not exist in the table %', att_name, rel; END IF; - IF (attdata.attnotnull = false) THEN - /* - * TODO: Here is a case where the table has different constraints on nodes. - * Using prepared transactions, we might be sure this operation will finish - * if only each node satisfies the rule. But we need to add support for 2PC - * commit beforehand. - */ - RAISE NOTICE USING - MESSAGE = format('delta_apply feature can not be applied to nullable column %L of the table %I', - att_name, rel), - HINT = 'Set NOT NULL constraint on the column', - ERRCODE = 'object_not_in_prerequisite_state'; - RETURN false; - END IF; - SELECT typname FROM pg_type WHERE typname IN ('int2','int4','int8','float4','float8','numeric','money') AND typinput = attdata.typinput AND typoutput = attdata.typoutput diff --git a/src/spock_apply_heap.c b/src/spock_apply_heap.c index 1a790d73..d477185b 100644 --- a/src/spock_apply_heap.c +++ b/src/spock_apply_heap.c @@ -541,31 +541,44 @@ build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, } /* - * This shouldn't happen: creating delta_apply column we must check that - * NOT NULL constraint is set on this column or reject. But just to - * survive in case of a bug we complain and send the apply worker to - * exception behavior path way. + * Column is marked LOG_OLD_VALUE=true. We use that as flag to apply + * the delta between the remote old and new instead of the plain new + * value. + * + * To perform the actual delta math we need the functions behind the + * '+' and '-' operators for the data type. + * + * XXX: This is currently hardcoded for the builtin data types we + * support. Ideally we would lookup those operators in the system + * cache, but that isn't straight forward and we get into all sorts of + * trouble when it comes to user defined data types and the search + * path. */ - if (oldtup->nulls[remoteattnum] || newtup->nulls[remoteattnum]) - ereport(ERROR, - (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), - errmsg("delta apply column can't operate NULL values"), - errdetail("attribute %d for remote tuple is %s, and for the local tuple is %s", - remoteattnum + 1, - newtup->nulls[remoteattnum] ? "NULL" : "NOT NULL", - oldtup->nulls[remoteattnum] ? "NULL" : "NOT NULL" - ))); - - loc_value = heap_getattr(TTS_TUP(localslot), remoteattnum + 1, tupdesc, - &loc_isnull); - Assert(!loc_isnull); - - result = OidFunctionCall3Coll(rel->delta_functions[remoteattnum], - InvalidOid, oldtup->values[remoteattnum], - newtup->values[remoteattnum], loc_value); - deltatup->values[remoteattnum] = result; - deltatup->nulls[remoteattnum] = false; - deltatup->changed[remoteattnum] = true; + + if (oldtup->nulls[remoteattnum]) + { + /* + * This is a special case. Columns for delta apply need to be + * marked NOT NULL. We use this as a flag + * to force plain NEW value application. This is useful in case a + * server ever gets out of sync. + */ + deltatup->values[remoteattnum] = newtup->values[remoteattnum]; + deltatup->nulls[remoteattnum] = false; + deltatup->changed[remoteattnum] = true; + } + else + { + loc_value = heap_getattr(TTS_TUP(localslot), remoteattnum + 1, tupdesc, + &loc_isnull); + + result = OidFunctionCall3Coll(rel->delta_functions[remoteattnum], + InvalidOid, oldtup->values[remoteattnum], + newtup->values[remoteattnum], loc_value); + deltatup->values[remoteattnum] = result; + deltatup->nulls[remoteattnum] = false; + deltatup->changed[remoteattnum] = true; + } } } diff --git a/tests/regress/expected/autoddl.out b/tests/regress/expected/autoddl.out index 2005d958..4ee88cd6 100644 --- a/tests/regress/expected/autoddl.out +++ b/tests/regress/expected/autoddl.out @@ -103,7 +103,7 @@ INFO: DDL statement replicated. \c :provider_dsn \set VERBOSITY terse -- Check propagation of security labels -CREATE TABLE slabel1 (x integer NOT NULL, y text PRIMARY KEY); +CREATE TABLE slabel1 (x integer, y text PRIMARY KEY); INFO: DDL statement replicated. SELECT spock.delta_apply('slabel1', 'x'); INFO: DDL statement replicated. diff --git a/tests/regress/expected/basic.out b/tests/regress/expected/basic.out index 78ef5101..32b095d3 100644 --- a/tests/regress/expected/basic.out +++ b/tests/regress/expected/basic.out @@ -217,8 +217,8 @@ DROP FUNCTION call_fn; -- Remember, these tests still check intra-node behaviour. -- -- A label creation checks -CREATE TABLE slabel (x integer NOT NULL, y text PRIMARY KEY); -CREATE TABLE slabel_ri (x integer NOT NULL, y text, z text); +CREATE TABLE slabel (x integer, y text PRIMARY KEY); +CREATE TABLE slabel_ri (x integer NOT NULL, y text); CREATE UNIQUE INDEX slabel_ri_idx ON slabel_ri(x); ALTER TABLE slabel_ri REPLICA IDENTITY USING INDEX slabel_ri_idx; SELECT spock.delta_apply('slabel', 'x'); @@ -289,7 +289,7 @@ SELECT objname, label FROM pg_seclabels; (0 rows) ALTER TABLE slabel DROP COLUMN x; -ALTER TABLE slabel ADD COLUMN x numeric NOT NULL; +ALTER TABLE slabel ADD COLUMN x numeric; SELECT spock.delta_apply('slabel', 'x', false); WARNING: delta_apply setting has not been propagated to other spock nodes delta_apply diff --git a/tests/regress/expected/infofuncs.out b/tests/regress/expected/infofuncs.out index b2813db0..5e33bef4 100644 --- a/tests/regress/expected/infofuncs.out +++ b/tests/regress/expected/infofuncs.out @@ -21,7 +21,7 @@ WHERE extname = 'spock'; (1 row) -- Check that security label is cleaned up on the extension drop -CREATE TABLE slabel (x money NOT NULL, y text PRIMARY KEY); +CREATE TABLE slabel (x money, y text PRIMARY KEY); SELECT spock.delta_apply('slabel', 'x', false); delta_apply ------------- diff --git a/tests/regress/sql/autoddl.sql b/tests/regress/sql/autoddl.sql index d426ca88..241a62af 100644 --- a/tests/regress/sql/autoddl.sql +++ b/tests/regress/sql/autoddl.sql @@ -68,7 +68,7 @@ DROP TABLE test_383 CASCADE; \c :provider_dsn \set VERBOSITY terse -- Check propagation of security labels -CREATE TABLE slabel1 (x integer NOT NULL, y text PRIMARY KEY); +CREATE TABLE slabel1 (x integer, y text PRIMARY KEY); SELECT spock.delta_apply('slabel1', 'x'); SELECT spock.delta_apply('slabel1', 'y'); -- ERROR diff --git a/tests/regress/sql/basic.sql b/tests/regress/sql/basic.sql index 3bdc63a1..ef6ea088 100644 --- a/tests/regress/sql/basic.sql +++ b/tests/regress/sql/basic.sql @@ -118,8 +118,8 @@ DROP FUNCTION call_fn; -- -- A label creation checks -CREATE TABLE slabel (x integer NOT NULL, y text PRIMARY KEY); -CREATE TABLE slabel_ri (x integer NOT NULL, y text, z text); +CREATE TABLE slabel (x integer, y text PRIMARY KEY); +CREATE TABLE slabel_ri (x integer NOT NULL, y text); CREATE UNIQUE INDEX slabel_ri_idx ON slabel_ri(x); ALTER TABLE slabel_ri REPLICA IDENTITY USING INDEX slabel_ri_idx; @@ -147,7 +147,7 @@ SELECT spock.delta_apply('slabel', 'x', false); ALTER TABLE slabel ALTER COLUMN x TYPE text; -- just warn SELECT objname, label FROM pg_seclabels; ALTER TABLE slabel DROP COLUMN x; -ALTER TABLE slabel ADD COLUMN x numeric NOT NULL; +ALTER TABLE slabel ADD COLUMN x numeric; SELECT spock.delta_apply('slabel', 'x', false); ALTER TABLE slabel DROP COLUMN x; SELECT objname, label FROM pg_seclabels; diff --git a/tests/regress/sql/infofuncs.sql b/tests/regress/sql/infofuncs.sql index f21daf66..c0b19cdb 100644 --- a/tests/regress/sql/infofuncs.sql +++ b/tests/regress/sql/infofuncs.sql @@ -10,7 +10,7 @@ FROM pg_extension WHERE extname = 'spock'; -- Check that security label is cleaned up on the extension drop -CREATE TABLE slabel (x money NOT NULL, y text PRIMARY KEY); +CREATE TABLE slabel (x money, y text PRIMARY KEY); SELECT spock.delta_apply('slabel', 'x', false); SELECT objname, label FROM pg_seclabels; diff --git a/tests/tap/t/012_delta_apply.pl b/tests/tap/t/012_delta_apply.pl index 49213e2e..1f7b0aef 100644 --- a/tests/tap/t/012_delta_apply.pl +++ b/tests/tap/t/012_delta_apply.pl @@ -1,6 +1,6 @@ use strict; use warnings; -use Test::More tests => 14; +use Test::More tests => 13; use IPC::Run; use lib '.'; use lib 't'; @@ -30,8 +30,8 @@ psql_or_bail(3, 'SELECT spock.wait_slot_confirm_lsn(NULL, NULL)'); psql_or_bail(1, "SELECT spock.delta_apply('t1', 'x');"); -#psql_or_bail(2, "SELECT spock.delta_apply('t1', 'x');"); -#psql_or_bail(3, "SELECT spock.delta_apply('t1', 'x');"); +psql_or_bail(2, "SELECT spock.delta_apply('t1', 'x');"); +psql_or_bail(3, "SELECT spock.delta_apply('t1', 'x');"); psql_or_bail(3, q( CREATE PROCEDURE counter_change(relname name, attname name, @@ -98,10 +98,5 @@ BEGIN ok($result2 eq $result3, "Equality of the data on N2 and N3 is confirmed"); print STDERR "DEBUGGING. Results: $result1 | $result2 | $result3\n"; -# Remove delta_apply from the column x and set it on the column y -psql_or_bail(1, "SELECT spock.delta_apply('t1', 'x', true)"); -$result1 = scalar_query(1, "SELECT spock.delta_apply('t1', 'y')"); # ERROR -print STDERR "DEBUGGING. Result: $result1\n"; -ok($result1 eq 'f', "delta_apply can't be used with nullable columns"); # Cleanup will be handled by SpockTest.pm END block # No need for done_testing() when using a test plan From f7e1fb2a305225e6733f62c6b4d45623dbb19edf Mon Sep 17 00:00:00 2001 From: Mason Sharp Date: Fri, 17 Apr 2026 13:17:04 -0700 Subject: [PATCH 3/9] Revert "Let the security label-based delta apply feature be a first-class citizen." This reverts commit 730cac094458a1206bbcf9070615cd62bfd4048d. --- include/spock_relcache.h | 2 - sql/spock--5.0.6--6.0.0-devel.sql | 107 ------------------------------ src/spock_autoddl.c | 6 +- src/spock_relcache.c | 41 ------------ src/spock_repset.c | 3 +- tests/tap/schedule | 1 - tests/tap/t/012_delta_apply.pl | 102 ---------------------------- 7 files changed, 3 insertions(+), 259 deletions(-) delete mode 100644 tests/tap/t/012_delta_apply.pl diff --git a/include/spock_relcache.h b/include/spock_relcache.h index 0ae29657..528f99b3 100644 --- a/include/spock_relcache.h +++ b/include/spock_relcache.h @@ -69,8 +69,6 @@ extern void spock_relation_cache_reset(void); extern Oid spock_lookup_delta_function(char *fname, Oid typeoid); -extern Oid get_replication_identity(Relation rel); - struct SpockTupleData; #endif /* SPOCK_RELCACHE_H */ diff --git a/sql/spock--5.0.6--6.0.0-devel.sql b/sql/spock--5.0.6--6.0.0-devel.sql index e4a47829..cd6dd8f6 100644 --- a/sql/spock--5.0.6--6.0.0-devel.sql +++ b/sql/spock--5.0.6--6.0.0-devel.sql @@ -210,113 +210,6 @@ RETURNS void AS 'MODULE_PATHNAME', 'spock_reset_subscription_stats' LANGUAGE C CALLED ON NULL INPUT VOLATILE; --- Set delta_apply security label on specific column -CREATE FUNCTION spock.delta_apply( - rel regclass, - att_name name, - to_drop boolean DEFAULT false -) RETURNS boolean AS $$ -DECLARE - label text; - atttype name; - attdata record; - sqlstring text; - status boolean; - relreplident char (1); - ctypname name; -BEGIN - - /* - * regclass input type guarantees we see this table, no 'not found' check - * is needed. - */ - SELECT c.relreplident FROM pg_class c WHERE oid = rel INTO relreplident; - /* - * Allow only DEFAULT type of replica identity. FULL type means we have - * already requested delta_apply feature on this table. - * Avoid INDEX type because indexes may have different names on the nodes and - * it would be better to stay paranoid than afraid of consequences. - */ - IF (relreplident <> 'd' AND relreplident <> 'f') - THEN - RAISE EXCEPTION 'spock can apply delta_apply feature to the DEFAULT replica identity type only. This table holds "%" idenity', relreplident; - END IF; - - /* - * Find proper delta_apply function for the column type or ERROR - */ - - SELECT t.typname,t.typinput,t.typoutput - FROM pg_catalog.pg_attribute a, pg_type t - WHERE a.attrelid = rel AND a.attname = att_name AND (a.atttypid = t.oid) - INTO attdata; - IF NOT FOUND THEN - RAISE EXCEPTION 'column % does not exist in the table %', att_name, rel; - END IF; - - SELECT typname FROM pg_type WHERE - typname IN ('int2','int4','int8','float4','float8','numeric','money') AND - typinput = attdata.typinput AND typoutput = attdata.typoutput - INTO ctypname; - IF NOT FOUND THEN - RAISE EXCEPTION 'type "%" can not be used in delta_apply conflict resolution', - attdata.typname; - END IF; - - -- - -- Create security label on the column - -- - IF (to_drop = true) THEN - sqlstring := format('SECURITY LABEL FOR spock ON COLUMN %I.%I IS NULL;' , - rel, att_name); - ELSE - sqlstring := format('SECURITY LABEL FOR spock ON COLUMN %I.%I IS %L;' , - rel, att_name, 'spock.delta_apply'); - END IF; - - EXECUTE sqlstring; - - /* - * Auto replication will propagate security label if needed. Just warn if it's - * not - the structure sync pg_dump call would copy security labels, isn't it? - */ - SELECT pg_catalog.current_setting('spock.enable_ddl_replication') INTO status; - IF EXISTS (SELECT 1 FROM spock.local_node) AND status = false THEN - raise WARNING 'delta_apply setting has not been propagated to other spock nodes'; - END IF; - - IF EXISTS (SELECT 1 FROM pg_catalog.pg_seclabel - WHERE objoid = rel AND classoid = 'pg_class'::regclass AND - provider = 'spock') THEN - /* - * Call it each time to trigger relcache invalidation callback that causes - * refresh of the SpockRelation entry and guarantees actual state of the - * delta_apply columns. - */ - EXECUTE format('ALTER TABLE %I REPLICA IDENTITY FULL', rel); - ELSIF EXISTS (SELECT 1 FROM pg_catalog.pg_class c - WHERE c.oid = rel AND c.relreplident = 'f') THEN - /* - * Have removed he last security label. Revert this spock hack change, - * if needed. - */ - EXECUTE format('ALTER TABLE %I REPLICA IDENTITY DEFAULT', rel); - END IF; - - RETURN true; -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); diff --git a/src/spock_autoddl.c b/src/spock_autoddl.c index cb4e37b3..53a937dd 100644 --- a/src/spock_autoddl.c +++ b/src/spock_autoddl.c @@ -21,7 +21,7 @@ #include "commands/defrem.h" #include "commands/extension.h" -#include "commands/seclabel.h" + #include "tcop/utility.h" #include "utils/builtins.h" #include "utils/lsyscache.h" @@ -30,7 +30,6 @@ #include "spock_autoddl.h" #include "spock_executor.h" #include "spock_queue.h" -#include "spock_relcache.h" #include "spock_repset.h" #include "spock_node.h" #include "spock_output_plugin.h" @@ -282,8 +281,7 @@ add_ddl_to_repset(Node *parsetree) } if (!OidIsValid(targetrel->rd_replidindex) && - (repset->replicate_update || repset->replicate_delete) && - !OidIsValid(get_replication_identity(targetrel))) + (repset->replicate_update || repset->replicate_delete)) { table_close(targetrel, NoLock); return; diff --git a/src/spock_relcache.c b/src/spock_relcache.c index 34049b36..3de050b4 100644 --- a/src/spock_relcache.c +++ b/src/spock_relcache.c @@ -428,44 +428,3 @@ spock_lookup_delta_function(char *fname, Oid typeoid) return funcoid; } - -/* - * Detect if the FULL identity is just covering delta_apply - * feature underpinned by PK. - * This is quite a rare case. Don't afraid overhead. - */ -Oid -get_replication_identity(Relation rel) -{ - TupleDesc tupDesc = RelationGetDescr(rel); - int i = 0; - - if (OidIsValid(rel->rd_replidindex)) - return rel->rd_replidindex; - - if (rel->rd_rel->relreplident != REPLICA_IDENTITY_FULL || - !OidIsValid(rel->rd_pkindex)) - return InvalidOid; - - for (i = 0; i < tupDesc->natts; i++) - { - char *seclabel; - ObjectAddress object; - - ObjectAddressSubSet(object, RelationRelationId, - RelationGetRelid(rel), i + 1); - seclabel = GetSecurityLabel(&object, spock_SECLABEL_PROVIDER); - - if (seclabel != NULL) - { - /* - * Relation has at least on security label on it. Treat it as a - * delta_apply feature. - */ - pfree(seclabel); - return rel->rd_pkindex; - } - } - - return InvalidOid; -} diff --git a/src/spock_repset.c b/src/spock_repset.c index c3539dbf..ae926bde 100644 --- a/src/spock_repset.c +++ b/src/spock_repset.c @@ -45,7 +45,6 @@ #include "spock_dependency.h" #include "spock_node.h" #include "spock_queue.h" -#include "spock_relcache.h" #include "spock_repset.h" #include "spock.h" #include "spock_compat.h" @@ -1131,7 +1130,7 @@ replication_set_add_table(Oid setid, Oid reloid, List *att_list, if (targetrel->rd_indexvalid == 0) RelationGetIndexList(targetrel); - if (!OidIsValid(get_replication_identity(targetrel)) && + if (!OidIsValid(targetrel->rd_replidindex) && (repset->replicate_update || repset->replicate_delete)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), diff --git a/tests/tap/schedule b/tests/tap/schedule index a7894dea..7699c126 100644 --- a/tests/tap/schedule +++ b/tests/tap/schedule @@ -34,7 +34,6 @@ test: 014_pgdump_restore_conflict # break # fi # done -test: 012_delta_apply test: 013_exception_handling # test: 014_rolling_upgrade test: 015_skip_lsn diff --git a/tests/tap/t/012_delta_apply.pl b/tests/tap/t/012_delta_apply.pl deleted file mode 100644 index 1f7b0aef..00000000 --- a/tests/tap/t/012_delta_apply.pl +++ /dev/null @@ -1,102 +0,0 @@ -use strict; -use warnings; -use Test::More tests => 13; -use IPC::Run; -use lib '.'; -use lib 't'; -use SpockTest qw(create_cluster destroy_cluster system_or_bail get_test_config cross_wire psql_or_bail scalar_query); - -my ($result1, $result2, $result3, $lsn1, $lsn2); - -create_cluster(3, 'Create initial 3-node Spock test cluster'); -cross_wire(3, ['n1', 'n2', 'n3'], 'Cross-wire nodes'); -# Get cluster configuration -my $config = get_test_config(); -my $node_count = $config->{node_count}; -my $node_ports = $config->{node_ports}; -my $host = $config->{host}; -my $dbname = $config->{db_name}; -my $db_user = $config->{db_user}; -my $db_password = $config->{db_password}; -my $pg_bin = $config->{pg_bin}; - -psql_or_bail(1, qq( - CREATE TABLE t1 (id integer PRIMARY KEY, x integer NOT NULL, y integer); - INSERT INTO t1 (id, x) VALUES (1,42); -)); - -psql_or_bail(1, 'SELECT spock.wait_slot_confirm_lsn(NULL, NULL)'); -psql_or_bail(2, 'SELECT spock.wait_slot_confirm_lsn(NULL, NULL)'); -psql_or_bail(3, 'SELECT spock.wait_slot_confirm_lsn(NULL, NULL)'); - -psql_or_bail(1, "SELECT spock.delta_apply('t1', 'x');"); -psql_or_bail(2, "SELECT spock.delta_apply('t1', 'x');"); -psql_or_bail(3, "SELECT spock.delta_apply('t1', 'x');"); - -psql_or_bail(3, q( -CREATE PROCEDURE counter_change(relname name, attname name, - value integer, cycles integer) -AS $$ -DECLARE - i integer := 0; -BEGIN - WHILE i < cycles LOOP - EXECUTE format('UPDATE %I SET %s = %s + %L;', relname, attname, attname, value); - -- raise WARNING '[%] iteration % from %', value, i, cycles; - COMMIT; - i := i + 1; - - END LOOP; - raise NOTICE '[%] FINISH: iteration % from %', value, i, cycles; -END; -$$ LANGUAGE plpgsql;)); - -psql_or_bail(1, 'SELECT spock.wait_slot_confirm_lsn(NULL, NULL)'); -psql_or_bail(2, 'SELECT spock.wait_slot_confirm_lsn(NULL, NULL)'); -psql_or_bail(3, 'SELECT spock.wait_slot_confirm_lsn(NULL, NULL)'); - -$result1 = scalar_query(1,"SELECT * FROM pg_catalog.pg_seclabels"); -$result2 = scalar_query(2,"SELECT * FROM pg_catalog.pg_seclabels"); -$result3 = scalar_query(3,"SELECT * FROM pg_catalog.pg_seclabels"); -print STDERR "DEBUGGING. Spock nodes IDs: \n$result1 $result2 $result3\n"; - -my ($pgbench_stdout1, $pgbench_stderr1) = ('', ''); -my $phandle1 = IPC::Run::start( - [ - 'psql', - '-c', "CALL counter_change('t1', 'x', -1, 10000)", - '-h', $host, '-p', $node_ports->[1], '-U', $db_user, $dbname - ], - '>' => \$pgbench_stdout1, - '2>' => \$pgbench_stderr1); -#$phandle1->pump(); - -psql_or_bail(1, "CALL counter_change('t1', 'x', +1, 10001)"); - -$phandle1->finish; -is($phandle1->full_result(0), 0, "alternative run successfull"); - -print STDERR "##### output of psql #####\n"; -print STDERR "$pgbench_stdout1"; -print STDERR "$pgbench_stderr1"; -print STDERR "##### end of output #####\n"; - -# Wait for sync ... -$lsn1 = scalar_query(1, "SELECT spock.sync_event()"); -$lsn2 = scalar_query(2, "SELECT spock.sync_event()"); -psql_or_bail(1, "CALL spock.wait_for_sync_event(true, 'n2', '$lsn2'::pg_lsn, 600)"); -psql_or_bail(2, "CALL spock.wait_for_sync_event(true, 'n1', '$lsn1'::pg_lsn, 600)"); -psql_or_bail(3, "CALL spock.wait_for_sync_event(true, 'n1', '$lsn1'::pg_lsn, 600)"); -psql_or_bail(3, "CALL spock.wait_for_sync_event(true, 'n2', '$lsn2'::pg_lsn, 600)"); - -$result1 = scalar_query(1, "SELECT x FROM t1"); -$result2 = scalar_query(2, "SELECT x FROM t1"); -$result3 = scalar_query(3, "SELECT x FROM t1"); - -ok($result1 eq '43', "Data on the node N1 has correct value"); -ok($result1 eq $result3, "Equality of the data on N1 and N3 is confirmed"); -ok($result2 eq $result3, "Equality of the data on N2 and N3 is confirmed"); -print STDERR "DEBUGGING. Results: $result1 | $result2 | $result3\n"; - -# Cleanup will be handled by SpockTest.pm END block -# No need for done_testing() when using a test plan From b3f7b75709e57f3c3b271faa7a50ab8efbba6169 Mon Sep 17 00:00:00 2001 From: Mason Sharp Date: Fri, 17 Apr 2026 13:17:28 -0700 Subject: [PATCH 4/9] Revert "Remove the core attribute options." This reverts commit 589b1af3f7a1c65b7310cd510e1906c3cb14fe57. --- Makefile | 3 + include/spock_relcache.h | 2 + patches/15/pg15-015-attoptions.diff | 194 +++++++++++++++++++++++++++ patches/16/pg16-015-attoptions.diff | 194 +++++++++++++++++++++++++++ patches/17/pg17-015-attoptions.diff | 194 +++++++++++++++++++++++++++ patches/18/pg18-015-attoptions.diff | 199 ++++++++++++++++++++++++++++ src/spock_apply_heap.c | 27 +++- src/spock_relcache.c | 64 ++++++--- 8 files changed, 853 insertions(+), 24 deletions(-) create mode 100644 patches/15/pg15-015-attoptions.diff create mode 100644 patches/16/pg16-015-attoptions.diff create mode 100644 patches/17/pg17-015-attoptions.diff create mode 100644 patches/18/pg18-015-attoptions.diff diff --git a/Makefile b/Makefile index f8d5cc4d..000138d7 100644 --- a/Makefile +++ b/Makefile @@ -30,6 +30,9 @@ ifdef SPOCK_RANDOM_DELAYS PG_CPPFLAGS += -DSPOCK_RANDOM_DELAYS endif SHLIB_LINK += $(libpq) $(filter -lintl, $(LIBS)) +ifdef NO_LOG_OLD_VALUE +PG_CPPFLAGS += -DNO_LOG_OLD_VALUE +endif REGRESS := __placeholder__ EXTRA_CLEAN += $(control_path) spock_compat.bc \ diff --git a/include/spock_relcache.h b/include/spock_relcache.h index 528f99b3..889537a7 100644 --- a/include/spock_relcache.h +++ b/include/spock_relcache.h @@ -44,6 +44,8 @@ typedef struct SpockRelation Oid idxoid; Relation rel; int *attmap; + bool has_delta_columns; + Oid *delta_apply_functions; /* Additional cache, only valid as long as relation mapping is. */ bool hasTriggers; diff --git a/patches/15/pg15-015-attoptions.diff b/patches/15/pg15-015-attoptions.diff new file mode 100644 index 00000000..7b63311a --- /dev/null +++ b/patches/15/pg15-015-attoptions.diff @@ -0,0 +1,194 @@ +diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c +index 5b696043c5..16ec985928 100644 +--- a/src/backend/access/common/reloptions.c ++++ b/src/backend/access/common/reloptions.c +@@ -168,6 +168,15 @@ static relopt_bool boolRelOpts[] = + }, + true + }, ++ { ++ { ++ "log_old_value", ++ "Add old value of attribute to WAL for logical decoding", ++ RELOPT_KIND_ATTRIBUTE, ++ ShareUpdateExclusiveLock ++ }, ++ false ++ }, + /* list terminator */ + {{NULL}} + }; +@@ -548,6 +557,19 @@ static relopt_enum enumRelOpts[] = + + static relopt_string stringRelOpts[] = + { ++ { ++ { ++ "delta_apply_function", ++ "Function called to perform delta conflict avoidance", ++ RELOPT_KIND_ATTRIBUTE, ++ ShareUpdateExclusiveLock ++ }, ++ -1, ++ true, ++ NULL, ++ NULL, ++ NULL ++ }, + /* list terminator */ + {{NULL}} + }; +@@ -2076,7 +2098,9 @@ attribute_reloptions(Datum reloptions, bool validate) + { + static const relopt_parse_elt tab[] = { + {"n_distinct", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct)}, +- {"n_distinct_inherited", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct_inherited)} ++ {"n_distinct_inherited", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct_inherited)}, ++ {"log_old_value", RELOPT_TYPE_BOOL, offsetof(AttributeOpts, log_old_value)}, ++ {"delta_apply_function", RELOPT_TYPE_STRING, offsetof(AttributeOpts, delta_apply_function)} + }; + + return (bytea *) build_reloptions(reloptions, validate, +diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c +index 88ab5f99c8..161333c5e0 100644 +--- a/src/backend/access/heap/heapam.c ++++ b/src/backend/access/heap/heapam.c +@@ -66,6 +66,7 @@ + #include "storage/smgr.h" + #include "storage/spin.h" + #include "storage/standby.h" ++#include "utils/attoptcache.h" + #include "utils/datum.h" + #include "utils/inval.h" + #include "utils/lsyscache.h" +@@ -86,6 +87,7 @@ static void check_lock_if_inplace_updateable_rel(Relation relation, + HeapTuple newtup); + static void check_inplace_rel_lock(HeapTuple oldtup); + #endif ++static Bitmapset *HeapDetermineLogOldColumns(Relation relation); + static Bitmapset *HeapDetermineColumnsInfo(Relation relation, + Bitmapset *interesting_cols, + Bitmapset *external_cols, +@@ -117,6 +119,7 @@ static void index_delete_sort(TM_IndexDeleteOp *delstate); + static int bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate); + static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup); + static HeapTuple ExtractReplicaIdentity(Relation rel, HeapTuple tup, bool key_required, ++ Bitmapset *logged_old_attrs, + bool *copy); + + +@@ -2919,7 +2922,7 @@ l1: + * Compute replica identity tuple before entering the critical section so + * we don't PANIC upon a memory allocation failure. + */ +- old_key_tuple = ExtractReplicaIdentity(relation, &tp, true, &old_key_copied); ++ old_key_tuple = ExtractReplicaIdentity(relation, &tp, true, NULL, &old_key_copied); + + /* + * If this is the first possibly-multixact-able operation in the current +@@ -3150,6 +3153,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, + Bitmapset *id_attrs; + Bitmapset *interesting_attrs; + Bitmapset *modified_attrs; ++ Bitmapset *logged_old_attrs; + ItemId lp; + HeapTupleData oldtup; + HeapTuple heaptup; +@@ -3267,6 +3271,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, + modified_attrs = HeapDetermineColumnsInfo(relation, interesting_attrs, + id_attrs, &oldtup, + newtup, &id_has_external); ++ logged_old_attrs = HeapDetermineLogOldColumns(relation); + + /* + * If we're not updating any "key" column, we can grab a weaker lock type. +@@ -3538,6 +3543,7 @@ l2: + bms_free(key_attrs); + bms_free(id_attrs); + bms_free(modified_attrs); ++ bms_free(logged_old_attrs); + bms_free(interesting_attrs); + return result; + } +@@ -3874,6 +3880,7 @@ l2: + old_key_tuple = ExtractReplicaIdentity(relation, &oldtup, + bms_overlap(modified_attrs, id_attrs) || + id_has_external, ++ logged_old_attrs, + &old_key_copied); + + /* NO EREPORT(ERROR) from here till changes are logged */ +@@ -4023,6 +4030,7 @@ l2: + bms_free(key_attrs); + bms_free(id_attrs); + bms_free(modified_attrs); ++ bms_free(logged_old_attrs); + bms_free(interesting_attrs); + + return TM_Ok; +@@ -4195,6 +4203,26 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2, + } + } + ++static Bitmapset * ++HeapDetermineLogOldColumns(Relation relation) ++{ ++ int attnum; ++ Bitmapset *logged_cols = NULL; ++ TupleDesc tupdesc = RelationGetDescr(relation); ++ AttributeOpts *aopt; ++ ++ for (attnum = 1; attnum <= tupdesc->natts; attnum++) ++ { ++ aopt = get_attribute_options(relation->rd_id, attnum); ++ if (aopt != NULL && aopt->log_old_value) ++ logged_cols = bms_add_member(logged_cols, ++ attnum - ++ FirstLowInvalidHeapAttributeNumber); ++ } ++ ++ return logged_cols; ++} ++ + /* + * Check which columns are being updated. + * +@@ -8913,6 +8941,7 @@ log_heap_new_cid(Relation relation, HeapTuple tup) + */ + static HeapTuple + ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, ++ Bitmapset *logged_old_attrs, + bool *copy) + { + TupleDesc desc = RelationGetDescr(relation); +@@ -8945,13 +8974,16 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, + } + + /* if the key isn't required and we're only logging the key, we're done */ +- if (!key_required) ++ if (!key_required && logged_old_attrs == NULL) + return NULL; + + /* find out the replica identity columns */ + idattrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_IDENTITY_KEY); + ++ /* merge the columns that are marked LOG_OLD_VALUE */ ++ idattrs = bms_union(idattrs, logged_old_attrs); ++ + /* + * If there's no defined replica identity columns, treat as !key_required. + * (This case should not be reachable from heap_update, since that should +diff --git a/src/include/utils/attoptcache.h b/src/include/utils/attoptcache.h +index ee37af9500..98b48a8fd8 100644 +--- a/src/include/utils/attoptcache.h ++++ b/src/include/utils/attoptcache.h +@@ -21,6 +21,8 @@ typedef struct AttributeOpts + int32 vl_len_; /* varlena header (do not touch directly!) */ + float8 n_distinct; + float8 n_distinct_inherited; ++ bool log_old_value; ++ Oid delta_apply_function; + } AttributeOpts; + + extern AttributeOpts *get_attribute_options(Oid spcid, int attnum); diff --git a/patches/16/pg16-015-attoptions.diff b/patches/16/pg16-015-attoptions.diff new file mode 100644 index 00000000..ffea2382 --- /dev/null +++ b/patches/16/pg16-015-attoptions.diff @@ -0,0 +1,194 @@ +diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c +index 469de9bb49..0119e5aa7c 100644 +--- a/src/backend/access/common/reloptions.c ++++ b/src/backend/access/common/reloptions.c +@@ -168,6 +168,15 @@ static relopt_bool boolRelOpts[] = + }, + true + }, ++ { ++ { ++ "log_old_value", ++ "Add old value of attribute to WAL for logical decoding", ++ RELOPT_KIND_ATTRIBUTE, ++ ShareUpdateExclusiveLock ++ }, ++ false ++ }, + /* list terminator */ + {{NULL}} + }; +@@ -548,6 +557,19 @@ static relopt_enum enumRelOpts[] = + + static relopt_string stringRelOpts[] = + { ++ { ++ { ++ "delta_apply_function", ++ "Function called to perform delta conflict avoidance", ++ RELOPT_KIND_ATTRIBUTE, ++ ShareUpdateExclusiveLock ++ }, ++ -1, ++ true, ++ NULL, ++ NULL, ++ NULL ++ }, + /* list terminator */ + {{NULL}} + }; +@@ -2072,7 +2094,9 @@ attribute_reloptions(Datum reloptions, bool validate) + { + static const relopt_parse_elt tab[] = { + {"n_distinct", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct)}, +- {"n_distinct_inherited", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct_inherited)} ++ {"n_distinct_inherited", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct_inherited)}, ++ {"log_old_value", RELOPT_TYPE_BOOL, offsetof(AttributeOpts, log_old_value)}, ++ {"delta_apply_function", RELOPT_TYPE_STRING, offsetof(AttributeOpts, delta_apply_function)} + }; + + return (bytea *) build_reloptions(reloptions, validate, +diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c +index 723e34e464..b2f845211d 100644 +--- a/src/backend/access/heap/heapam.c ++++ b/src/backend/access/heap/heapam.c +@@ -67,6 +67,7 @@ + #include "storage/smgr.h" + #include "storage/spin.h" + #include "storage/standby.h" ++#include "utils/attoptcache.h" + #include "utils/datum.h" + #include "utils/inval.h" + #include "utils/lsyscache.h" +@@ -87,6 +88,7 @@ static void check_lock_if_inplace_updateable_rel(Relation relation, + HeapTuple newtup); + static void check_inplace_rel_lock(HeapTuple oldtup); + #endif ++static Bitmapset *HeapDetermineLogOldColumns(Relation relation); + static Bitmapset *HeapDetermineColumnsInfo(Relation relation, + Bitmapset *interesting_cols, + Bitmapset *external_cols, +@@ -121,6 +123,7 @@ static void index_delete_sort(TM_IndexDeleteOp *delstate); + static int bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate); + static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup); + static HeapTuple ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, ++ Bitmapset *logged_old_attrs, + bool *copy); + + +@@ -2780,7 +2783,7 @@ l1: + * Compute replica identity tuple before entering the critical section so + * we don't PANIC upon a memory allocation failure. + */ +- old_key_tuple = ExtractReplicaIdentity(relation, &tp, true, &old_key_copied); ++ old_key_tuple = ExtractReplicaIdentity(relation, &tp, true, NULL, &old_key_copied); + + /* + * If this is the first possibly-multixact-able operation in the current +@@ -3013,6 +3016,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, + Bitmapset *id_attrs; + Bitmapset *interesting_attrs; + Bitmapset *modified_attrs; ++ Bitmapset *logged_old_attrs; + ItemId lp; + HeapTupleData oldtup; + HeapTuple heaptup; +@@ -3135,6 +3139,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, + modified_attrs = HeapDetermineColumnsInfo(relation, interesting_attrs, + id_attrs, &oldtup, + newtup, &id_has_external); ++ logged_old_attrs = HeapDetermineLogOldColumns(relation); + + /* + * If we're not updating any "key" column, we can grab a weaker lock type. +@@ -3409,6 +3414,7 @@ l2: + bms_free(key_attrs); + bms_free(id_attrs); + bms_free(modified_attrs); ++ bms_free(logged_old_attrs); + bms_free(interesting_attrs); + return result; + } +@@ -3758,6 +3764,7 @@ l2: + old_key_tuple = ExtractReplicaIdentity(relation, &oldtup, + bms_overlap(modified_attrs, id_attrs) || + id_has_external, ++ logged_old_attrs, + &old_key_copied); + + /* NO EREPORT(ERROR) from here till changes are logged */ +@@ -3924,6 +3931,7 @@ l2: + bms_free(key_attrs); + bms_free(id_attrs); + bms_free(modified_attrs); ++ bms_free(logged_old_attrs); + bms_free(interesting_attrs); + + return TM_Ok; +@@ -4096,6 +4104,26 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2, + } + } + ++static Bitmapset * ++HeapDetermineLogOldColumns(Relation relation) ++{ ++ int attnum; ++ Bitmapset *logged_cols = NULL; ++ TupleDesc tupdesc = RelationGetDescr(relation); ++ AttributeOpts *aopt; ++ ++ for (attnum = 1; attnum <= tupdesc->natts; attnum++) ++ { ++ aopt = get_attribute_options(relation->rd_id, attnum); ++ if (aopt != NULL && aopt->log_old_value) ++ logged_cols = bms_add_member(logged_cols, ++ attnum - ++ FirstLowInvalidHeapAttributeNumber); ++ } ++ ++ return logged_cols; ++} ++ + /* + * Check which columns are being updated. + * +@@ -9051,6 +9079,7 @@ log_heap_new_cid(Relation relation, HeapTuple tup) + */ + static HeapTuple + ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, ++ Bitmapset *logged_old_attrs, + bool *copy) + { + TupleDesc desc = RelationGetDescr(relation); +@@ -9083,13 +9112,16 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, + } + + /* if the key isn't required and we're only logging the key, we're done */ +- if (!key_required) ++ if (!key_required && logged_old_attrs == NULL) + return NULL; + + /* find out the replica identity columns */ + idattrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_IDENTITY_KEY); + ++ /* merge the columns that are marked LOG_OLD_VALUE */ ++ idattrs = bms_union(idattrs, logged_old_attrs); ++ + /* + * If there's no defined replica identity columns, treat as !key_required. + * (This case should not be reachable from heap_update, since that should +diff --git a/src/include/utils/attoptcache.h b/src/include/utils/attoptcache.h +index e4119b6aa2..6354a98157 100644 +--- a/src/include/utils/attoptcache.h ++++ b/src/include/utils/attoptcache.h +@@ -21,6 +21,8 @@ typedef struct AttributeOpts + int32 vl_len_; /* varlena header (do not touch directly!) */ + float8 n_distinct; + float8 n_distinct_inherited; ++ bool log_old_value; ++ Oid delta_apply_function; + } AttributeOpts; + + extern AttributeOpts *get_attribute_options(Oid attrelid, int attnum); diff --git a/patches/17/pg17-015-attoptions.diff b/patches/17/pg17-015-attoptions.diff new file mode 100644 index 00000000..df156695 --- /dev/null +++ b/patches/17/pg17-015-attoptions.diff @@ -0,0 +1,194 @@ +diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c +index d6eb5d85599..4ef2e55cc44 100644 +--- a/src/backend/access/common/reloptions.c ++++ b/src/backend/access/common/reloptions.c +@@ -166,6 +166,15 @@ static relopt_bool boolRelOpts[] = + }, + true + }, ++ { ++ { ++ "log_old_value", ++ "Add old value of attribute to WAL for logical decoding", ++ RELOPT_KIND_ATTRIBUTE, ++ ShareUpdateExclusiveLock ++ }, ++ false ++ }, + /* list terminator */ + {{NULL}} + }; +@@ -546,6 +555,19 @@ static relopt_enum enumRelOpts[] = + + static relopt_string stringRelOpts[] = + { ++ { ++ { ++ "delta_apply_function", ++ "Function called to perform delta conflict avoidance", ++ RELOPT_KIND_ATTRIBUTE, ++ ShareUpdateExclusiveLock ++ }, ++ -1, ++ true, ++ NULL, ++ NULL, ++ NULL ++ }, + /* list terminator */ + {{NULL}} + }; +@@ -2070,7 +2092,9 @@ attribute_reloptions(Datum reloptions, bool validate) + { + static const relopt_parse_elt tab[] = { + {"n_distinct", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct)}, +- {"n_distinct_inherited", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct_inherited)} ++ {"n_distinct_inherited", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct_inherited)}, ++ {"log_old_value", RELOPT_TYPE_BOOL, offsetof(AttributeOpts, log_old_value)}, ++ {"delta_apply_function", RELOPT_TYPE_STRING, offsetof(AttributeOpts, delta_apply_function)} + }; + + return (bytea *) build_reloptions(reloptions, validate, +diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c +index 95e3be524a7..708d55b2618 100644 +--- a/src/backend/access/heap/heapam.c ++++ b/src/backend/access/heap/heapam.c +@@ -64,6 +64,7 @@ + #include "storage/predicate.h" + #include "storage/procarray.h" + #include "storage/standby.h" ++#include "utils/attoptcache.h" + #include "utils/datum.h" + #include "utils/injection_point.h" + #include "utils/inval.h" +@@ -85,6 +86,7 @@ static void check_lock_if_inplace_updateable_rel(Relation relation, + HeapTuple newtup); + static void check_inplace_rel_lock(HeapTuple oldtup); + #endif ++static Bitmapset *HeapDetermineLogOldColumns(Relation relation); + static Bitmapset *HeapDetermineColumnsInfo(Relation relation, + Bitmapset *interesting_cols, + Bitmapset *external_cols, +@@ -121,6 +123,7 @@ static void index_delete_sort(TM_IndexDeleteOp *delstate); + static int bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate); + static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup); + static HeapTuple ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, ++ Bitmapset *logged_old_attrs, + bool *copy); + + +@@ -2938,7 +2941,7 @@ l1: + * Compute replica identity tuple before entering the critical section so + * we don't PANIC upon a memory allocation failure. + */ +- old_key_tuple = ExtractReplicaIdentity(relation, &tp, true, &old_key_copied); ++ old_key_tuple = ExtractReplicaIdentity(relation, &tp, true, NULL, &old_key_copied); + + /* + * If this is the first possibly-multixact-able operation in the current +@@ -3171,6 +3174,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, + Bitmapset *id_attrs; + Bitmapset *interesting_attrs; + Bitmapset *modified_attrs; ++ Bitmapset *logged_old_attrs; + ItemId lp; + HeapTupleData oldtup; + HeapTuple heaptup; +@@ -3338,6 +3342,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, + modified_attrs = HeapDetermineColumnsInfo(relation, interesting_attrs, + id_attrs, &oldtup, + newtup, &id_has_external); ++ logged_old_attrs = HeapDetermineLogOldColumns(relation); + + /* + * If we're not updating any "key" column, we can grab a weaker lock type. +@@ -3612,6 +3617,7 @@ l2: + bms_free(key_attrs); + bms_free(id_attrs); + bms_free(modified_attrs); ++ bms_free(logged_old_attrs); + bms_free(interesting_attrs); + return result; + } +@@ -3961,6 +3967,7 @@ l2: + old_key_tuple = ExtractReplicaIdentity(relation, &oldtup, + bms_overlap(modified_attrs, id_attrs) || + id_has_external, ++ logged_old_attrs, + &old_key_copied); + + /* NO EREPORT(ERROR) from here till changes are logged */ +@@ -4127,6 +4134,7 @@ l2: + bms_free(key_attrs); + bms_free(id_attrs); + bms_free(modified_attrs); ++ bms_free(logged_old_attrs); + bms_free(interesting_attrs); + + return TM_Ok; +@@ -4299,6 +4307,26 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2, + } + } + ++static Bitmapset * ++HeapDetermineLogOldColumns(Relation relation) ++{ ++ int attnum; ++ Bitmapset *logged_cols = NULL; ++ TupleDesc tupdesc = RelationGetDescr(relation); ++ AttributeOpts *aopt; ++ ++ for (attnum = 1; attnum <= tupdesc->natts; attnum++) ++ { ++ aopt = get_attribute_options(relation->rd_id, attnum); ++ if (aopt != NULL && aopt->log_old_value) ++ logged_cols = bms_add_member(logged_cols, ++ attnum - ++ FirstLowInvalidHeapAttributeNumber); ++ } ++ ++ return logged_cols; ++} ++ + /* + * Check which columns are being updated. + * +@@ -9076,6 +9104,7 @@ log_heap_new_cid(Relation relation, HeapTuple tup) + */ + static HeapTuple + ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, ++ Bitmapset *logged_old_attrs, + bool *copy) + { + TupleDesc desc = RelationGetDescr(relation); +@@ -9108,13 +9137,16 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, + } + + /* if the key isn't required and we're only logging the key, we're done */ +- if (!key_required) ++ if (!key_required && logged_old_attrs == NULL) + return NULL; + + /* find out the replica identity columns */ + idattrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_IDENTITY_KEY); + ++ /* merge the columns that are marked LOG_OLD_VALUE */ ++ idattrs = bms_union(idattrs, logged_old_attrs); ++ + /* + * If there's no defined replica identity columns, treat as !key_required. + * (This case should not be reachable from heap_update, since that should +diff --git a/src/include/utils/attoptcache.h b/src/include/utils/attoptcache.h +index a1a9bfc0fb9..e9b6dfab474 100644 +--- a/src/include/utils/attoptcache.h ++++ b/src/include/utils/attoptcache.h +@@ -21,6 +21,8 @@ typedef struct AttributeOpts + int32 vl_len_; /* varlena header (do not touch directly!) */ + float8 n_distinct; + float8 n_distinct_inherited; ++ bool log_old_value; ++ Oid delta_apply_function; + } AttributeOpts; + + extern AttributeOpts *get_attribute_options(Oid attrelid, int attnum); diff --git a/patches/18/pg18-015-attoptions.diff b/patches/18/pg18-015-attoptions.diff new file mode 100644 index 00000000..dd42114d --- /dev/null +++ b/patches/18/pg18-015-attoptions.diff @@ -0,0 +1,199 @@ +diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c +index 50747c16396..6cdbbe1d0dd 100644 +--- a/src/backend/access/common/reloptions.c ++++ b/src/backend/access/common/reloptions.c +@@ -166,6 +166,15 @@ static relopt_bool boolRelOpts[] = + }, + true + }, ++ { ++ { ++ "log_old_value", ++ "Add old value of attribute to WAL for logical decoding", ++ RELOPT_KIND_ATTRIBUTE, ++ ShareUpdateExclusiveLock ++ }, ++ false ++ }, + /* list terminator */ + {{NULL}} + }; +@@ -557,6 +566,19 @@ static relopt_enum enumRelOpts[] = + + static relopt_string stringRelOpts[] = + { ++ { ++ { ++ "delta_apply_function", ++ "Function called to perform delta conflict avoidance", ++ RELOPT_KIND_ATTRIBUTE, ++ ShareUpdateExclusiveLock ++ }, ++ -1, ++ true, ++ NULL, ++ NULL, ++ NULL ++ }, + /* list terminator */ + {{NULL}} + }; +@@ -2106,7 +2128,9 @@ attribute_reloptions(Datum reloptions, bool validate) + { + static const relopt_parse_elt tab[] = { + {"n_distinct", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct)}, +- {"n_distinct_inherited", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct_inherited)} ++ {"n_distinct_inherited", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct_inherited)}, ++ {"log_old_value", RELOPT_TYPE_BOOL, offsetof(AttributeOpts, log_old_value)}, ++ {"delta_apply_function", RELOPT_TYPE_STRING, offsetof(AttributeOpts, delta_apply_function)} + }; + + return (bytea *) build_reloptions(reloptions, validate, +diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c +index 0dcd6ee817e..dcf26167cae 100644 +--- a/src/backend/access/heap/heapam.c ++++ b/src/backend/access/heap/heapam.c +@@ -48,6 +48,7 @@ + #include "storage/lmgr.h" + #include "storage/predicate.h" + #include "storage/procarray.h" ++#include "utils/attoptcache.h" + #include "utils/datum.h" + #include "utils/injection_point.h" + #include "utils/inval.h" +@@ -67,6 +68,7 @@ static void check_lock_if_inplace_updateable_rel(Relation relation, + HeapTuple newtup); + static void check_inplace_rel_lock(HeapTuple oldtup); + #endif ++static Bitmapset *HeapDetermineLogOldColumns(Relation relation); + static Bitmapset *HeapDetermineColumnsInfo(Relation relation, + Bitmapset *interesting_cols, + Bitmapset *external_cols, +@@ -104,6 +106,7 @@ static void index_delete_sort(TM_IndexDeleteOp *delstate); + static int bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate); + static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup); + static HeapTuple ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, ++ Bitmapset *logged_old_attrs, + bool *copy); + + +@@ -3016,7 +3019,7 @@ l1: + * Compute replica identity tuple before entering the critical section so + * we don't PANIC upon a memory allocation failure. + */ +- old_key_tuple = ExtractReplicaIdentity(relation, &tp, true, &old_key_copied); ++ old_key_tuple = ExtractReplicaIdentity(relation, &tp, true, NULL, &old_key_copied); + + /* + * If this is the first possibly-multixact-able operation in the current +@@ -3249,6 +3252,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, + Bitmapset *id_attrs; + Bitmapset *interesting_attrs; + Bitmapset *modified_attrs; ++ Bitmapset *logged_old_attrs; + ItemId lp; + HeapTupleData oldtup; + HeapTuple heaptup; +@@ -3419,6 +3423,12 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, + id_attrs, &oldtup, + newtup, &id_has_external); + ++ if (!IsCatalogRelationOid(relation->rd_id)) ++ logged_old_attrs = HeapDetermineLogOldColumns(relation); ++ else ++ /* No need to log old values for catalog tables */ ++ logged_old_attrs = NULL; ++ + /* + * If we're not updating any "key" column, we can grab a weaker lock type. + * This allows for more concurrency when we are running simultaneously +@@ -3692,6 +3702,7 @@ l2: + bms_free(key_attrs); + bms_free(id_attrs); + bms_free(modified_attrs); ++ bms_free(logged_old_attrs); + bms_free(interesting_attrs); + return result; + } +@@ -4041,6 +4052,7 @@ l2: + old_key_tuple = ExtractReplicaIdentity(relation, &oldtup, + bms_overlap(modified_attrs, id_attrs) || + id_has_external, ++ logged_old_attrs, + &old_key_copied); + + /* NO EREPORT(ERROR) from here till changes are logged */ +@@ -4207,6 +4219,7 @@ l2: + bms_free(key_attrs); + bms_free(id_attrs); + bms_free(modified_attrs); ++ bms_free(logged_old_attrs); + bms_free(interesting_attrs); + + return TM_Ok; +@@ -4379,6 +4392,26 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2, + } + } + ++static Bitmapset * ++HeapDetermineLogOldColumns(Relation relation) ++{ ++ int attnum; ++ Bitmapset *logged_cols = NULL; ++ TupleDesc tupdesc = RelationGetDescr(relation); ++ AttributeOpts *aopt; ++ ++ for (attnum = 1; attnum <= tupdesc->natts; attnum++) ++ { ++ aopt = get_attribute_options(relation->rd_id, attnum); ++ if (aopt != NULL && aopt->log_old_value) ++ logged_cols = bms_add_member(logged_cols, ++ attnum - ++ FirstLowInvalidHeapAttributeNumber); ++ } ++ ++ return logged_cols; ++} ++ + /* + * Check which columns are being updated. + * +@@ -9132,6 +9165,7 @@ log_heap_new_cid(Relation relation, HeapTuple tup) + */ + static HeapTuple + ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, ++ Bitmapset *logged_old_attrs, + bool *copy) + { + TupleDesc desc = RelationGetDescr(relation); +@@ -9164,13 +9198,16 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, + } + + /* if the key isn't required and we're only logging the key, we're done */ +- if (!key_required) ++ if (!key_required && logged_old_attrs == NULL) + return NULL; + + /* find out the replica identity columns */ + idattrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_IDENTITY_KEY); + ++ /* merge the columns that are marked LOG_OLD_VALUE */ ++ idattrs = bms_union(idattrs, logged_old_attrs); ++ + /* + * If there's no defined replica identity columns, treat as !key_required. + * (This case should not be reachable from heap_update, since that should +diff --git a/src/include/utils/attoptcache.h b/src/include/utils/attoptcache.h +index f684a772af5..6c965fede13 100644 +--- a/src/include/utils/attoptcache.h ++++ b/src/include/utils/attoptcache.h +@@ -21,6 +21,8 @@ typedef struct AttributeOpts + int32 vl_len_; /* varlena header (do not touch directly!) */ + float8 n_distinct; + float8 n_distinct_inherited; ++ bool log_old_value; ++ Oid delta_apply_function; + } AttributeOpts; + + extern AttributeOpts *get_attribute_options(Oid attrelid, int attnum); diff --git a/src/spock_apply_heap.c b/src/spock_apply_heap.c index d477185b..afd1dbb3 100644 --- a/src/spock_apply_heap.c +++ b/src/spock_apply_heap.c @@ -98,9 +98,11 @@ typedef struct ApplyExecState #define TTS_TUP(slot) (((HeapTupleTableSlot *)slot)->tuple) +#ifndef NO_LOG_OLD_VALUE static void build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, SpockTupleData *newtup, SpockTupleData *deltatup, TupleTableSlot *localslot); +#endif static bool physatt_in_attmap(SpockRelation *rel, int attid); /* @@ -511,6 +513,8 @@ physatt_in_attmap(SpockRelation *rel, int attid) } +#ifndef NO_LOG_OLD_VALUE + static void build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, SpockTupleData *newtup, @@ -532,7 +536,8 @@ build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, int remoteattnum = rel->attmap[attidx]; Assert(remoteattnum < tupdesc->natts); - if (rel->delta_functions[remoteattnum] == InvalidOid) + if (rel->delta_apply_functions[remoteattnum] == InvalidOid && + rel->delta_functions[remoteattnum] == InvalidOid) { deltatup->values[remoteattnum] = 0xdeadbeef; deltatup->nulls[remoteattnum] = true; @@ -559,7 +564,8 @@ build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, { /* * This is a special case. Columns for delta apply need to be - * marked NOT NULL. We use this as a flag + * marked NOT NULL and LOG_OLD_VALUE=true. During this remote + * UPDATE LOG_OLD_VALUE setting was false. We use this as a flag * to force plain NEW value application. This is useful in case a * server ever gets out of sync. */ @@ -567,7 +573,7 @@ build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, deltatup->nulls[remoteattnum] = false; deltatup->changed[remoteattnum] = true; } - else + else if (rel->delta_functions[remoteattnum] != InvalidOid) { loc_value = heap_getattr(TTS_TUP(localslot), remoteattnum + 1, tupdesc, &loc_isnull); @@ -579,8 +585,21 @@ build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, deltatup->nulls[remoteattnum] = false; deltatup->changed[remoteattnum] = true; } + else + { + loc_value = heap_getattr(TTS_TUP(localslot), remoteattnum + 1, tupdesc, + &loc_isnull); + + result = OidFunctionCall3Coll(rel->delta_apply_functions[remoteattnum], + InvalidOid, oldtup->values[remoteattnum], + newtup->values[remoteattnum], loc_value); + deltatup->values[remoteattnum] = result; + deltatup->nulls[remoteattnum] = false; + deltatup->changed[remoteattnum] = true; + } } } +#endif /* NO_LOG_OLD_VALUE */ /** @@ -651,7 +670,7 @@ spock_handle_conflict_and_apply(SpockRelation *rel, EState *estate, xmin, local_origin_found, local_origin, local_ts, idxused); - if (rel->has_delta_apply) + if (rel->has_delta_columns || rel->has_delta_apply) { SpockTupleData deltatup; HeapTuple currenttuple; diff --git a/src/spock_relcache.c b/src/spock_relcache.c index 3de050b4..0a80bed8 100644 --- a/src/spock_relcache.c +++ b/src/spock_relcache.c @@ -55,12 +55,15 @@ relcache_free_entry(SpockRelation *entry) if (entry->attmap) pfree(entry->attmap); + if (entry->delta_apply_functions) + pfree(entry->delta_apply_functions); if (entry->delta_functions) pfree(entry->delta_functions); entry->natts = 0; entry->reloid = InvalidOid; entry->rel = NULL; + entry->has_delta_columns = false; entry->has_delta_apply = false; } @@ -95,16 +98,44 @@ spock_relation_open(uint32 remoteid, LOCKMODE lockmode) if (unlikely(entry->rel == NULL)) return NULL; - + desc = RelationGetDescr(entry->rel); for (i = 0; i < entry->natts; i++) { ObjectAddress object; char *seclabel; - TupleDesc desc; - desc = RelationGetDescr(entry->rel); entry->attmap[i] = tupdesc_get_att_by_name(desc, entry->attnames[i]); + /* + * If we find attribute options for this column and the + * delta_apply_function is set, lookup the oid for it. + */ + att = TupleDescAttr(desc, entry->attmap[i]); + aopt = get_attribute_options(entry->rel->rd_id, + entry->attmap[i] + 1); + if (aopt != NULL && aopt->delta_apply_function != 0) + { + char *fname; + Form_pg_attribute att; + Oid dfunc; + + fname = pstrdup(GET_STRING_RELOPTION(aopt, + delta_apply_function)); + dfunc = spock_lookup_delta_function(fname, att->atttypid); + pfree(fname); + + if (dfunc == InvalidOid) + elog(ERROR, "SPOCK: column %s.%s.%s is configured for " + "delta_apply_function %s - function not found", + entry->nspname, entry->relname, + entry->attnames[i], + GET_STRING_RELOPTION(aopt, delta_apply_function)); + + + entry->has_delta_columns = true; + entry->delta_apply_functions[entry->attmap[i]] = dfunc; + } + if (entry->rel->rd_rel->relreplident != REPLICA_IDENTITY_FULL) continue; @@ -114,27 +145,16 @@ spock_relation_open(uint32 remoteid, LOCKMODE lockmode) * * XXX: What about non-existing columns on remote side? */ - ObjectAddressSubSet(object, RelationRelationId, - RelationGetRelid(entry->rel), - entry->attmap[i] + 1); + object.classId = RelationRelationId; + object.objectId = RelationGetRelid(entry->rel); + object.objectSubId = entry->attmap[i] + 1; seclabel = GetSecurityLabel(&object, spock_SECLABEL_PROVIDER); if (seclabel != NULL) { - Form_pg_attribute att; - Oid dfunc; - - att = TupleDescAttr(desc, entry->attmap[i]); - dfunc = spock_lookup_delta_function(seclabel, att->atttypid); - - if (dfunc == InvalidOid) - elog(ERROR, "SPOCK: column %s.%s.%s is configured for " - "delta_apply function %s - function not found", - entry->nspname, entry->relname, - entry->attnames[i], seclabel); - - entry->delta_functions[entry->attmap[i]] = dfunc; - Assert(entry->delta_functions[entry->attmap[i]] != InvalidOid); entry->has_delta_apply = true; + entry->delta_functions[entry->attmap[i]] = + spock_lookup_delta_function(seclabel, att->atttypid); + Assert(entry->delta_functions[entry->attmap[i]] != InvalidOid); } else { @@ -232,6 +252,8 @@ spock_relation_cache_update(uint32 remoteid, char *schemaname, entry->attrtypmods[i] = attrtypmods[i]; } entry->attmap = palloc(natts * sizeof(int)); + entry->has_delta_columns = false; + entry->delta_apply_functions = palloc0(natts * sizeof(Oid)); entry->has_delta_apply = false; entry->delta_functions = (Oid *) palloc0(entry->natts * sizeof(Oid)); MemoryContextSwitchTo(oldcontext); @@ -270,6 +292,8 @@ spock_relation_cache_updater(SpockRemoteRel *remoterel) for (i = 0; i < remoterel->natts; i++) entry->attnames[i] = pstrdup(remoterel->attnames[i]); entry->attmap = palloc(remoterel->natts * sizeof(int)); + entry->has_delta_columns = false; + entry->delta_apply_functions = palloc0(remoterel->natts * sizeof(Oid)); entry->has_delta_apply = false; entry->delta_functions = (Oid *) palloc0(entry->natts * sizeof(Oid)); MemoryContextSwitchTo(oldcontext); From 6bf0b9cc513b7461426dca18953cf708b0affe05 Mon Sep 17 00:00:00 2001 From: Mason Sharp Date: Fri, 17 Apr 2026 13:18:01 -0700 Subject: [PATCH 5/9] Revert "Use security labels to implement the delta_apply feature." This reverts commit 43e3857007e62247f2b8e6072de9afd8aec602e6. --- include/spock_relcache.h | 3 -- sql/spock--6.0.0-devel.sql | 76 +++++----------------------- src/spock_apply_heap.c | 17 +------ src/spock_relcache.c | 64 +++-------------------- tests/regress/expected/autoddl.out | 15 +++--- tests/regress/expected/basic.out | 14 ++--- tests/regress/expected/infofuncs.out | 6 +-- tests/regress/sql/basic.sql | 5 -- 8 files changed, 35 insertions(+), 165 deletions(-) diff --git a/include/spock_relcache.h b/include/spock_relcache.h index 889537a7..8bdfa83b 100644 --- a/include/spock_relcache.h +++ b/include/spock_relcache.h @@ -49,9 +49,6 @@ typedef struct SpockRelation /* Additional cache, only valid as long as relation mapping is. */ bool hasTriggers; - - Oid *delta_functions; - bool has_delta_apply; } SpockRelation; extern void spock_relation_cache_update(uint32 remoteid, diff --git a/sql/spock--6.0.0-devel.sql b/sql/spock--6.0.0-devel.sql index 41267599..674cd2ef 100644 --- a/sql/spock--6.0.0-devel.sql +++ b/sql/spock--6.0.0-devel.sql @@ -750,33 +750,19 @@ CREATE FUNCTION spock.terminate_active_transactions() RETURNS bool -- Generic delta apply functions for all numeric data types -- ---- CREATE FUNCTION spock.delta_apply(int2, int2, int2) -RETURNS int2 -AS 'MODULE_PATHNAME', 'delta_apply_int2' -LANGUAGE C; +RETURNS int2 LANGUAGE c AS 'MODULE_PATHNAME', 'delta_apply_int2'; CREATE FUNCTION spock.delta_apply(int4, int4, int4) -RETURNS int4 -AS 'MODULE_PATHNAME', 'delta_apply_int4' -LANGUAGE C; +RETURNS int4 LANGUAGE c AS 'MODULE_PATHNAME', 'delta_apply_int4'; CREATE FUNCTION spock.delta_apply(int8, int8, int8) -RETURNS int8 -AS 'MODULE_PATHNAME', 'delta_apply_int8' -LANGUAGE C; +RETURNS int8 LANGUAGE c AS 'MODULE_PATHNAME', 'delta_apply_int8'; CREATE FUNCTION spock.delta_apply(float4, float4, float4) -RETURNS float4 -AS 'MODULE_PATHNAME', 'delta_apply_float4' -LANGUAGE C; +RETURNS float4 LANGUAGE c AS 'MODULE_PATHNAME', 'delta_apply_float4'; CREATE FUNCTION spock.delta_apply(float8, float8, float8) -RETURNS float8 -AS 'MODULE_PATHNAME', 'delta_apply_float8' -LANGUAGE C; +RETURNS float8 LANGUAGE c AS 'MODULE_PATHNAME', 'delta_apply_float8'; CREATE FUNCTION spock.delta_apply(numeric, numeric, numeric) -RETURNS numeric -AS 'MODULE_PATHNAME', 'delta_apply_numeric' -LANGUAGE C; +RETURNS numeric LANGUAGE c AS 'MODULE_PATHNAME', 'delta_apply_numeric'; CREATE FUNCTION spock.delta_apply(money, money, money) -RETURNS money -AS 'MODULE_PATHNAME', 'delta_apply_money' -LANGUAGE C; +RETURNS money LANGUAGE c AS 'MODULE_PATHNAME', 'delta_apply_money'; -- ---- -- Function to control REPAIR mode @@ -871,31 +857,13 @@ DECLARE label text; atttype name; attdata record; + ctypname name; sqlstring text; status boolean; - relreplident char (1); - ctypname name; BEGIN - - /* - * regclass input type guarantees we see this table, no 'not found' check - * is needed. - */ - SELECT c.relreplident FROM pg_class c WHERE oid = rel INTO relreplident; - /* - * Allow only DEFAULT type of replica identity. FULL type means we have - * already requested delta_apply feature on this table. - * Avoid INDEX type because indexes may have different names on the nodes and - * it would be better to stay paranoid than afraid of consequences. - */ - IF (relreplident <> 'd' AND relreplident <> 'f') - THEN - RAISE EXCEPTION 'spock can apply delta_apply feature to the DEFAULT replica identity type only. This table holds "%" idenity', relreplident; - END IF; - - /* - * Find proper delta_apply function for the column type or ERROR - */ + -- + -- Find proper delta_apply function for the column type or ERROR + -- SELECT t.typname,t.typinput,t.typoutput FROM pg_catalog.pg_attribute a, pg_type t @@ -921,8 +889,8 @@ BEGIN sqlstring := format('SECURITY LABEL FOR spock ON COLUMN %I.%I IS NULL;' , rel, att_name); ELSE - sqlstring := format('SECURITY LABEL FOR spock ON COLUMN %I.%I IS %L;' , - rel, att_name, 'spock.delta_apply'); + sqlstring := format('SECURITY LABEL FOR spock ON COLUMN %I.%I IS %L;' , + rel, att_name, 'delta_apply'); END IF; EXECUTE sqlstring; @@ -936,24 +904,6 @@ BEGIN raise WARNING 'delta_apply setting has not been propagated to other spock nodes'; END IF; - IF EXISTS (SELECT 1 FROM pg_catalog.pg_seclabel - WHERE objoid = rel AND classoid = 'pg_class'::regclass AND - provider = 'spock') THEN - /* - * Call it each time to trigger relcache invalidation callback that causes - * refresh of the SpockRelation entry and guarantees actual state of the - * delta_apply columns. - */ - EXECUTE format('ALTER TABLE %I REPLICA IDENTITY FULL', rel); - ELSIF EXISTS (SELECT 1 FROM pg_catalog.pg_class c - WHERE c.oid = rel AND c.relreplident = 'f') THEN - /* - * Have removed he last security label. Revert this spock hack change, - * if needed. - */ - EXECUTE format('ALTER TABLE %I REPLICA IDENTITY DEFAULT', rel); - END IF; - RETURN true; END; $$ LANGUAGE plpgsql STRICT VOLATILE; diff --git a/src/spock_apply_heap.c b/src/spock_apply_heap.c index afd1dbb3..6eb833b7 100644 --- a/src/spock_apply_heap.c +++ b/src/spock_apply_heap.c @@ -536,8 +536,7 @@ build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, int remoteattnum = rel->attmap[attidx]; Assert(remoteattnum < tupdesc->natts); - if (rel->delta_apply_functions[remoteattnum] == InvalidOid && - rel->delta_functions[remoteattnum] == InvalidOid) + if (rel->delta_apply_functions[remoteattnum] == InvalidOid) { deltatup->values[remoteattnum] = 0xdeadbeef; deltatup->nulls[remoteattnum] = true; @@ -573,18 +572,6 @@ build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, deltatup->nulls[remoteattnum] = false; deltatup->changed[remoteattnum] = true; } - else if (rel->delta_functions[remoteattnum] != InvalidOid) - { - loc_value = heap_getattr(TTS_TUP(localslot), remoteattnum + 1, tupdesc, - &loc_isnull); - - result = OidFunctionCall3Coll(rel->delta_functions[remoteattnum], - InvalidOid, oldtup->values[remoteattnum], - newtup->values[remoteattnum], loc_value); - deltatup->values[remoteattnum] = result; - deltatup->nulls[remoteattnum] = false; - deltatup->changed[remoteattnum] = true; - } else { loc_value = heap_getattr(TTS_TUP(localslot), remoteattnum + 1, tupdesc, @@ -670,7 +657,7 @@ spock_handle_conflict_and_apply(SpockRelation *rel, EState *estate, xmin, local_origin_found, local_origin, local_ts, idxused); - if (rel->has_delta_columns || rel->has_delta_apply) + if (rel->has_delta_columns) { SpockTupleData deltatup; HeapTuple currenttuple; diff --git a/src/spock_relcache.c b/src/spock_relcache.c index 0a80bed8..da51e3b3 100644 --- a/src/spock_relcache.c +++ b/src/spock_relcache.c @@ -16,7 +16,7 @@ #include "catalog/namespace.h" #include "catalog/pg_trigger.h" -#include "commands/seclabel.h" + #include "utils/attoptcache.h" #include "utils/builtins.h" #include "utils/catcache.h" @@ -57,14 +57,11 @@ relcache_free_entry(SpockRelation *entry) pfree(entry->attmap); if (entry->delta_apply_functions) pfree(entry->delta_apply_functions); - if (entry->delta_functions) - pfree(entry->delta_functions); entry->natts = 0; entry->reloid = InvalidOid; entry->rel = NULL; entry->has_delta_columns = false; - entry->has_delta_apply = false; } @@ -101,8 +98,7 @@ spock_relation_open(uint32 remoteid, LOCKMODE lockmode) desc = RelationGetDescr(entry->rel); for (i = 0; i < entry->natts; i++) { - ObjectAddress object; - char *seclabel; + AttributeOpts *aopt; entry->attmap[i] = tupdesc_get_att_by_name(desc, entry->attnames[i]); @@ -110,15 +106,15 @@ spock_relation_open(uint32 remoteid, LOCKMODE lockmode) * If we find attribute options for this column and the * delta_apply_function is set, lookup the oid for it. */ - att = TupleDescAttr(desc, entry->attmap[i]); aopt = get_attribute_options(entry->rel->rd_id, entry->attmap[i] + 1); if (aopt != NULL && aopt->delta_apply_function != 0) { - char *fname; - Form_pg_attribute att; - Oid dfunc; + char *fname; + Form_pg_attribute att; + Oid dfunc; + att = TupleDescAttr(desc, entry->attmap[i]); fname = pstrdup(GET_STRING_RELOPTION(aopt, delta_apply_function)); dfunc = spock_lookup_delta_function(fname, att->atttypid); @@ -135,54 +131,12 @@ spock_relation_open(uint32 remoteid, LOCKMODE lockmode) entry->has_delta_columns = true; entry->delta_apply_functions[entry->attmap[i]] = dfunc; } - - if (entry->rel->rd_rel->relreplident != REPLICA_IDENTITY_FULL) - continue; - - /* - * Read security labels for each attname. For each such an attribute - * choose corresponding delta function. - * - * XXX: What about non-existing columns on remote side? - */ - object.classId = RelationRelationId; - object.objectId = RelationGetRelid(entry->rel); - object.objectSubId = entry->attmap[i] + 1; - seclabel = GetSecurityLabel(&object, spock_SECLABEL_PROVIDER); - if (seclabel != NULL) - { - entry->has_delta_apply = true; - entry->delta_functions[entry->attmap[i]] = - spock_lookup_delta_function(seclabel, att->atttypid); - Assert(entry->delta_functions[entry->attmap[i]] != InvalidOid); - } - else - { - /* Main case */ - entry->delta_functions[entry->attmap[i]] = InvalidOid; - } } relinfo = makeNode(ResultRelInfo); InitResultRelInfo(relinfo, entry->rel, 1, NULL, 0); entry->reloid = RelationGetRelid(entry->rel); - if (entry->has_delta_apply) - { - /* - * It looks like a hack — which, in fact, it is. - * We assume that delta_apply may be used for the DEFAULT identity - * only and will be immediately removed after altering the table. - * Also, if an ERROR happens here we will stay with an inconsistent - * value of the relreplident field. But it is just a cache ... - */ - relinfo->ri_RelationDesc->rd_rel->relreplident = REPLICA_IDENTITY_DEFAULT; - relinfo->ri_RelationDesc->rd_indexvalid = false; - entry->idxoid = RelationGetReplicaIndex(relinfo->ri_RelationDesc); - Assert(entry->idxoid != InvalidOid); - relinfo->ri_RelationDesc->rd_rel->relreplident = REPLICA_IDENTITY_FULL; - } - else - entry->idxoid = RelationGetReplicaIndex(relinfo->ri_RelationDesc); + entry->idxoid = RelationGetReplicaIndex(relinfo->ri_RelationDesc); /* Cache trigger info. */ entry->hasTriggers = false; @@ -254,8 +208,6 @@ spock_relation_cache_update(uint32 remoteid, char *schemaname, entry->attmap = palloc(natts * sizeof(int)); entry->has_delta_columns = false; entry->delta_apply_functions = palloc0(natts * sizeof(Oid)); - entry->has_delta_apply = false; - entry->delta_functions = (Oid *) palloc0(entry->natts * sizeof(Oid)); MemoryContextSwitchTo(oldcontext); /* XXX Should we validate the relation against local schema here? */ @@ -294,8 +246,6 @@ spock_relation_cache_updater(SpockRemoteRel *remoterel) entry->attmap = palloc(remoterel->natts * sizeof(int)); entry->has_delta_columns = false; entry->delta_apply_functions = palloc0(remoterel->natts * sizeof(Oid)); - entry->has_delta_apply = false; - entry->delta_functions = (Oid *) palloc0(entry->natts * sizeof(Oid)); MemoryContextSwitchTo(oldcontext); /* XXX Should we validate the relation against local schema here? */ diff --git a/tests/regress/expected/autoddl.out b/tests/regress/expected/autoddl.out index 4ee88cd6..3cc17c96 100644 --- a/tests/regress/expected/autoddl.out +++ b/tests/regress/expected/autoddl.out @@ -106,7 +106,6 @@ INFO: DDL statement replicated. CREATE TABLE slabel1 (x integer, y text PRIMARY KEY); INFO: DDL statement replicated. SELECT spock.delta_apply('slabel1', 'x'); -INFO: DDL statement replicated. INFO: DDL statement replicated. delta_apply ------------- @@ -118,7 +117,6 @@ ERROR: type "text" can not be used in delta_apply conflict resolution SELECT spock.delta_apply('slabel1', 'z'); -- ERROR ERROR: column z does not exist in the table slabel1 SELECT spock.delta_apply('slabel1', 'x'); -- repeating call do nothing -INFO: DDL statement replicated. INFO: DDL statement replicated. delta_apply ------------- @@ -126,9 +124,9 @@ INFO: DDL statement replicated. (1 row) SELECT objname, label FROM pg_seclabels; - objname | label ------------+------------------- - slabel1.x | spock.delta_apply + objname | label +-----------+------------- + slabel1.x | delta_apply (1 row) -- Wait for the apply worker to process the security label before checking. @@ -141,14 +139,13 @@ CALL spock.wait_for_sync_event(NULL, 'test_provider', :'sync_lsn', 60); (1 row) SELECT objname, label FROM pg_seclabels; - objname | label ------------+------------------- - slabel1.x | spock.delta_apply + objname | label +-----------+------------- + slabel1.x | delta_apply (1 row) \c :provider_dsn SELECT spock.delta_apply('slabel1', 'x', true); -INFO: DDL statement replicated. INFO: DDL statement replicated. delta_apply ------------- diff --git a/tests/regress/expected/basic.out b/tests/regress/expected/basic.out index 32b095d3..b65c726d 100644 --- a/tests/regress/expected/basic.out +++ b/tests/regress/expected/basic.out @@ -218,9 +218,6 @@ DROP FUNCTION call_fn; -- -- A label creation checks CREATE TABLE slabel (x integer, y text PRIMARY KEY); -CREATE TABLE slabel_ri (x integer NOT NULL, y text); -CREATE UNIQUE INDEX slabel_ri_idx ON slabel_ri(x); -ALTER TABLE slabel_ri REPLICA IDENTITY USING INDEX slabel_ri_idx; SELECT spock.delta_apply('slabel', 'x'); WARNING: delta_apply setting has not been propagated to other spock nodes delta_apply @@ -239,15 +236,12 @@ WARNING: delta_apply setting has not been propagated to other spock nodes t (1 row) -SELECT spock.delta_apply('slabel_ri', 'x'); -- ERROR -ERROR: spock can apply delta_apply feature to the DEFAULT replica identity type only. This table holds "i" idenity SELECT objname, label FROM pg_seclabels; - objname | label -----------+------------------- - slabel.x | spock.delta_apply + objname | label +----------+------------- + slabel.x | delta_apply (1 row) -DROP TABLE slabel_ri CASCADE; -- Short round trip to check that subscriber has no security labels \c :subscriber_dsn SELECT objname, label FROM pg_seclabels; @@ -282,7 +276,7 @@ WARNING: delta_apply setting has not been propagated to other spock nodes (1 row) ALTER TABLE slabel ALTER COLUMN x TYPE text; -- just warn -WARNING: the alter column statement removes spock security label 'spock.delta_apply' on this column +WARNING: the alter column statement removes spock security label 'delta_apply' on this column SELECT objname, label FROM pg_seclabels; objname | label ---------+------- diff --git a/tests/regress/expected/infofuncs.out b/tests/regress/expected/infofuncs.out index 5e33bef4..1a7bf836 100644 --- a/tests/regress/expected/infofuncs.out +++ b/tests/regress/expected/infofuncs.out @@ -29,9 +29,9 @@ SELECT spock.delta_apply('slabel', 'x', false); (1 row) SELECT objname, label FROM pg_seclabels; - objname | label -----------+------------------- - slabel.x | spock.delta_apply + objname | label +----------+------------- + slabel.x | delta_apply (1 row) DROP EXTENSION spock; diff --git a/tests/regress/sql/basic.sql b/tests/regress/sql/basic.sql index ef6ea088..cdf16fe4 100644 --- a/tests/regress/sql/basic.sql +++ b/tests/regress/sql/basic.sql @@ -119,17 +119,12 @@ DROP FUNCTION call_fn; -- A label creation checks CREATE TABLE slabel (x integer, y text PRIMARY KEY); -CREATE TABLE slabel_ri (x integer NOT NULL, y text); -CREATE UNIQUE INDEX slabel_ri_idx ON slabel_ri(x); -ALTER TABLE slabel_ri REPLICA IDENTITY USING INDEX slabel_ri_idx; SELECT spock.delta_apply('slabel', 'x'); SELECT spock.delta_apply('slabel', 'y'); -- ERROR SELECT spock.delta_apply('slabel', 'z'); -- ERROR SELECT spock.delta_apply('slabel', 'x'); -- repeating call do nothing -SELECT spock.delta_apply('slabel_ri', 'x'); -- ERROR SELECT objname, label FROM pg_seclabels; -DROP TABLE slabel_ri CASCADE; -- Short round trip to check that subscriber has no security labels \c :subscriber_dsn From 988752da55645a826f11b07780ec12b68995eb1d Mon Sep 17 00:00:00 2001 From: Mason Sharp Date: Fri, 17 Apr 2026 13:18:37 -0700 Subject: [PATCH 6/9] Revert "Replicate SECURITY LABEL commands." This reverts commit 9bc41d7aaf167aad1ee8c2b7e89e969f8c4b259c. --- sql/spock--6.0.0-devel.sql | 41 ++++++++---------- src/spock_apply.c | 5 +-- src/spock_autoddl.c | 5 +-- tests/regress/expected/autoddl.out | 67 +----------------------------- tests/regress/expected/basic.out | 25 +++++------ tests/regress/sql/autoddl.sql | 25 +---------- tests/regress/sql/basic.sql | 6 --- 7 files changed, 32 insertions(+), 142 deletions(-) diff --git a/sql/spock--6.0.0-devel.sql b/sql/spock--6.0.0-devel.sql index 674cd2ef..b42d11fc 100644 --- a/sql/spock--6.0.0-devel.sql +++ b/sql/spock--6.0.0-devel.sql @@ -854,13 +854,22 @@ CREATE FUNCTION spock.delta_apply( to_drop boolean DEFAULT false ) RETURNS boolean AS $$ DECLARE - label text; - atttype name; - attdata record; - ctypname name; - sqlstring text; - status boolean; + label text; + atttype name; + attdata record; + ctypname name; BEGIN + IF (to_drop = true) THEN + DELETE FROM pg_seclabel WHERE objoid = rel AND + classoid = 'pg_class'::regclass AND + objsubid > 0 AND provider = 'spock'; + IF NOT FOUND THEN + RETURN false; + END IF; + + RETURN true; + END IF; + -- -- Find proper delta_apply function for the column type or ERROR -- @@ -885,24 +894,8 @@ BEGIN -- -- Create security label on the column -- - IF (to_drop = true) THEN - sqlstring := format('SECURITY LABEL FOR spock ON COLUMN %I.%I IS NULL;' , - rel, att_name); - ELSE - sqlstring := format('SECURITY LABEL FOR spock ON COLUMN %I.%I IS %L;' , - rel, att_name, 'delta_apply'); - END IF; - - EXECUTE sqlstring; - - /* - * Auto replication will propagate security label if needed. Just warn if it's - * not - the structure sync pg_dump call would copy security labels, isn't it? - */ - SELECT pg_catalog.current_setting('spock.enable_ddl_replication') INTO status; - IF EXISTS (SELECT 1 FROM spock.local_node) AND status = false THEN - raise WARNING 'delta_apply setting has not been propagated to other spock nodes'; - END IF; + EXECUTE format('SECURITY LABEL FOR spock ON COLUMN %I.%I IS %L' , + rel, att_name, 'delta_apply'); RETURN true; END; diff --git a/src/spock_apply.c b/src/spock_apply.c index 6d35b242..8cdc4e5e 100644 --- a/src/spock_apply.c +++ b/src/spock_apply.c @@ -3647,12 +3647,9 @@ spock_execute_sql_command(char *cmdstr, char *role, bool isTopLevel) /* * check if it's a DDL statement. we only do this for * in_spock_replicate_ddl_command - * SECURITY LABEL command is not a DDL, just an utility one. Hence, let - * spock execute this command. */ if (in_spock_replicate_ddl_command && - GetCommandLogLevel(command->stmt) != LOGSTMT_DDL && - !IsA(command->stmt, SecLabelStmt)) + GetCommandLogLevel(command->stmt) != LOGSTMT_DDL) { ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), diff --git a/src/spock_autoddl.c b/src/spock_autoddl.c index 53a937dd..17eb1001 100644 --- a/src/spock_autoddl.c +++ b/src/spock_autoddl.c @@ -342,9 +342,8 @@ autoddl_can_proceed(Node *parsetree, ProcessUtilityContext context, */ return false; - /* Only process DDL statements and SECURITY LABEL's */ - if (GetCommandLogLevel(parsetree) != LOGSTMT_DDL && - !IsA(parsetree, SecLabelStmt)) + /* Only process DDL statements */ + if (GetCommandLogLevel(parsetree) != LOGSTMT_DDL) return false; /* If DDL replication is disabled, do nothing */ diff --git a/tests/regress/expected/autoddl.out b/tests/regress/expected/autoddl.out index 3cc17c96..09cc8039 100644 --- a/tests/regress/expected/autoddl.out +++ b/tests/regress/expected/autoddl.out @@ -100,74 +100,9 @@ WARNING: This DDL statement will not be replicated. DROP TABLE test_383 CASCADE; NOTICE: drop cascades to table test_383 membership in replication set default_insert_only INFO: DDL statement replicated. -\c :provider_dsn -\set VERBOSITY terse --- Check propagation of security labels -CREATE TABLE slabel1 (x integer, y text PRIMARY KEY); -INFO: DDL statement replicated. -SELECT spock.delta_apply('slabel1', 'x'); -INFO: DDL statement replicated. - delta_apply -------------- - t -(1 row) - -SELECT spock.delta_apply('slabel1', 'y'); -- ERROR -ERROR: type "text" can not be used in delta_apply conflict resolution -SELECT spock.delta_apply('slabel1', 'z'); -- ERROR -ERROR: column z does not exist in the table slabel1 -SELECT spock.delta_apply('slabel1', 'x'); -- repeating call do nothing -INFO: DDL statement replicated. - delta_apply -------------- - t -(1 row) - -SELECT objname, label FROM pg_seclabels; - objname | label ------------+------------- - slabel1.x | delta_apply -(1 row) - --- Wait for the apply worker to process the security label before checking. -SELECT spock.sync_event() AS sync_lsn \gset \c :subscriber_dsn -CALL spock.wait_for_sync_event(NULL, 'test_provider', :'sync_lsn', 60); - result --------- - t -(1 row) - -SELECT objname, label FROM pg_seclabels; - objname | label ------------+------------- - slabel1.x | delta_apply -(1 row) - -\c :provider_dsn -SELECT spock.delta_apply('slabel1', 'x', true); -INFO: DDL statement replicated. - delta_apply -------------- - t -(1 row) - --- Wait for the apply worker to process the removal before checking. -SELECT spock.sync_event() AS sync_lsn \gset -\c :subscriber_dsn -CALL spock.wait_for_sync_event(NULL, 'test_provider', :'sync_lsn', 60); - result --------- - t -(1 row) - -SELECT objname, label FROM pg_seclabels; - objname | label ----------+------- -(0 rows) - -\c :provider_dsn -- Reset the configuration to the default value +\c :provider_dsn ALTER SYSTEM SET spock.enable_ddl_replication = 'off'; WARNING: This DDL statement will not be replicated. ALTER SYSTEM SET spock.include_ddl_repset = 'off'; diff --git a/tests/regress/expected/basic.out b/tests/regress/expected/basic.out index b65c726d..7cbab6d8 100644 --- a/tests/regress/expected/basic.out +++ b/tests/regress/expected/basic.out @@ -219,7 +219,6 @@ DROP FUNCTION call_fn; -- A label creation checks CREATE TABLE slabel (x integer, y text PRIMARY KEY); SELECT spock.delta_apply('slabel', 'x'); -WARNING: delta_apply setting has not been propagated to other spock nodes delta_apply ------------- t @@ -230,7 +229,6 @@ ERROR: type "text" can not be used in delta_apply conflict resolution SELECT spock.delta_apply('slabel', 'z'); -- ERROR ERROR: column z does not exist in the table slabel SELECT spock.delta_apply('slabel', 'x'); -- repeating call do nothing -WARNING: delta_apply setting has not been propagated to other spock nodes delta_apply ------------- t @@ -242,26 +240,25 @@ SELECT objname, label FROM pg_seclabels; slabel.x | delta_apply (1 row) --- Short round trip to check that subscriber has no security labels -\c :subscriber_dsn -SELECT objname, label FROM pg_seclabels; - objname | label ----------+------- -(0 rows) - -\c :provider_dsn -- Label drop checks SELECT spock.delta_apply('slabel', 'x', true); -WARNING: delta_apply setting has not been propagated to other spock nodes delta_apply ------------- t (1 row) SELECT spock.delta_apply('slabel', 'y', true); -ERROR: type "text" can not be used in delta_apply conflict resolution + delta_apply +------------- + f +(1 row) + SELECT spock.delta_apply('slabel', 'z', true); -ERROR: column z does not exist in the table slabel + delta_apply +------------- + f +(1 row) + SELECT objname, label FROM pg_seclabels; objname | label ---------+------- @@ -269,7 +266,6 @@ SELECT objname, label FROM pg_seclabels; -- Dependencies SELECT spock.delta_apply('slabel', 'x', false); -WARNING: delta_apply setting has not been propagated to other spock nodes delta_apply ------------- t @@ -285,7 +281,6 @@ SELECT objname, label FROM pg_seclabels; ALTER TABLE slabel DROP COLUMN x; ALTER TABLE slabel ADD COLUMN x numeric; SELECT spock.delta_apply('slabel', 'x', false); -WARNING: delta_apply setting has not been propagated to other spock nodes delta_apply ------------- t diff --git a/tests/regress/sql/autoddl.sql b/tests/regress/sql/autoddl.sql index 241a62af..e6e18fcb 100644 --- a/tests/regress/sql/autoddl.sql +++ b/tests/regress/sql/autoddl.sql @@ -65,33 +65,10 @@ CREATE INDEX test_383_y_idx ON test_383 (a); CLUSTER test_383 USING test_383_y_idx; -- Should not be replicated DROP TABLE test_383 CASCADE; -\c :provider_dsn -\set VERBOSITY terse --- Check propagation of security labels -CREATE TABLE slabel1 (x integer, y text PRIMARY KEY); - -SELECT spock.delta_apply('slabel1', 'x'); -SELECT spock.delta_apply('slabel1', 'y'); -- ERROR -SELECT spock.delta_apply('slabel1', 'z'); -- ERROR -SELECT spock.delta_apply('slabel1', 'x'); -- repeating call do nothing -SELECT objname, label FROM pg_seclabels; - --- Wait for the apply worker to process the security label before checking. -SELECT spock.sync_event() AS sync_lsn \gset -\c :subscriber_dsn -CALL spock.wait_for_sync_event(NULL, 'test_provider', :'sync_lsn', 60); -SELECT objname, label FROM pg_seclabels; -\c :provider_dsn - -SELECT spock.delta_apply('slabel1', 'x', true); --- Wait for the apply worker to process the removal before checking. -SELECT spock.sync_event() AS sync_lsn \gset \c :subscriber_dsn -CALL spock.wait_for_sync_event(NULL, 'test_provider', :'sync_lsn', 60); -SELECT objname, label FROM pg_seclabels; -\c :provider_dsn -- Reset the configuration to the default value +\c :provider_dsn ALTER SYSTEM SET spock.enable_ddl_replication = 'off'; ALTER SYSTEM SET spock.include_ddl_repset = 'off'; ALTER SYSTEM SET spock.allow_ddl_from_functions = 'off'; diff --git a/tests/regress/sql/basic.sql b/tests/regress/sql/basic.sql index cdf16fe4..6e1db841 100644 --- a/tests/regress/sql/basic.sql +++ b/tests/regress/sql/basic.sql @@ -119,18 +119,12 @@ DROP FUNCTION call_fn; -- A label creation checks CREATE TABLE slabel (x integer, y text PRIMARY KEY); - SELECT spock.delta_apply('slabel', 'x'); SELECT spock.delta_apply('slabel', 'y'); -- ERROR SELECT spock.delta_apply('slabel', 'z'); -- ERROR SELECT spock.delta_apply('slabel', 'x'); -- repeating call do nothing SELECT objname, label FROM pg_seclabels; --- Short round trip to check that subscriber has no security labels -\c :subscriber_dsn -SELECT objname, label FROM pg_seclabels; -\c :provider_dsn - -- Label drop checks SELECT spock.delta_apply('slabel', 'x', true); SELECT spock.delta_apply('slabel', 'y', true); From e9819e4dd13f97c2a39348e4bb5ac80b00ba4788 Mon Sep 17 00:00:00 2001 From: Mason Sharp Date: Fri, 17 Apr 2026 13:19:18 -0700 Subject: [PATCH 7/9] Revert "Introduce a delta_apply security label management." This reverts commit 31466f738aca5d52f0262929523b2e0693655c65. --- include/spock.h | 1 - sql/spock--6.0.0-devel.sql | 54 ------------------ src/spock.c | 30 ---------- src/spock_executor.c | 65 +--------------------- tests/regress/expected/basic.out | 82 ---------------------------- tests/regress/expected/infofuncs.out | 20 ------- tests/regress/sql/basic.sql | 32 ----------- tests/regress/sql/infofuncs.sql | 8 --- 8 files changed, 1 insertion(+), 291 deletions(-) diff --git a/include/spock.h b/include/spock.h index 457e79fc..c9a38557 100644 --- a/include/spock.h +++ b/include/spock.h @@ -28,7 +28,6 @@ #define SPOCK_VERSION_NUM 60000 #define EXTENSION_NAME "spock" -#define spock_SECLABEL_PROVIDER "spock" #define REPLICATION_ORIGIN_ALL "all" diff --git a/sql/spock--6.0.0-devel.sql b/sql/spock--6.0.0-devel.sql index b42d11fc..d96a1b9f 100644 --- a/sql/spock--6.0.0-devel.sql +++ b/sql/spock--6.0.0-devel.sql @@ -846,57 +846,3 @@ CREATE FUNCTION spock.reset_subscription_stats(subid oid DEFAULT NULL) RETURNS void AS 'MODULE_PATHNAME', 'spock_reset_subscription_stats' LANGUAGE C CALLED ON NULL INPUT VOLATILE; - --- Set delta_apply security label on specific column -CREATE FUNCTION spock.delta_apply( - rel regclass, - att_name name, - to_drop boolean DEFAULT false -) RETURNS boolean AS $$ -DECLARE - label text; - atttype name; - attdata record; - ctypname name; -BEGIN - IF (to_drop = true) THEN - DELETE FROM pg_seclabel WHERE objoid = rel AND - classoid = 'pg_class'::regclass AND - objsubid > 0 AND provider = 'spock'; - IF NOT FOUND THEN - RETURN false; - END IF; - - RETURN true; - END IF; - - -- - -- Find proper delta_apply function for the column type or ERROR - -- - - SELECT t.typname,t.typinput,t.typoutput - FROM pg_catalog.pg_attribute a, pg_type t - WHERE a.attrelid = rel AND a.attname = att_name AND (a.atttypid = t.oid) - INTO attdata; - IF NOT FOUND THEN - RAISE EXCEPTION 'column % does not exist in the table %', att_name, rel; - END IF; - - SELECT typname FROM pg_type WHERE - typname IN ('int2','int4','int8','float4','float8','numeric','money') AND - typinput = attdata.typinput AND typoutput = attdata.typoutput - INTO ctypname; - IF NOT FOUND THEN - RAISE EXCEPTION 'type "%" can not be used in delta_apply conflict resolution', - attdata.typname; - END IF; - - -- - -- Create security label on the column - -- - EXECUTE format('SECURITY LABEL FOR spock ON COLUMN %I.%I IS %L' , - rel, att_name, 'delta_apply'); - - RETURN true; -END; -$$ LANGUAGE plpgsql STRICT VOLATILE; diff --git a/src/spock.c b/src/spock.c index a5cb19c1..cdb980d8 100644 --- a/src/spock.c +++ b/src/spock.c @@ -25,7 +25,6 @@ #include "catalog/pg_type.h" #include "commands/extension.h" -#include "commands/seclabel.h" #include "executor/executor.h" @@ -915,32 +914,6 @@ log_message_filter(ErrorData *edata) } } -/* - * Spock security label hook. - * - * Just to be sure user adds a proper label: only table columns may be applied. - */ -static void -spock_object_relabel(const ObjectAddress *object, const char *seclabel) -{ - Oid extoid; - - extoid = get_extension_oid(EXTENSION_NAME, true); - if (!OidIsValid(extoid)) - elog(ERROR, "spock extension is not created yet"); - - /* - * Check: classId must be pg_class, objectId should an existing table and - * attnum must be more than 0. - */ - if (object->classId != RelationRelationId || object->objectSubId <= 0 || - !SearchSysCacheExists1(RELOID, ObjectIdGetDatum(object->objectId))) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("spock provider does not support labels on %s", - getObjectTypeDescription(object, false)))); -} - /* * Entry point for this module. */ @@ -1277,9 +1250,6 @@ _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); - #if PG_VERSION_NUM >= 180000 /* Spock replication conflict statistics */ spock_stat_register_conflict_stat(); diff --git a/src/spock_executor.c b/src/spock_executor.c index 8e1df23f..8aaa8361 100644 --- a/src/spock_executor.c +++ b/src/spock_executor.c @@ -20,18 +20,16 @@ #include "catalog/dependency.h" #include "catalog/index.h" -#include "catalog/indexing.h" #include "catalog/namespace.h" #include "catalog/objectaccess.h" #include "catalog/pg_authid_d.h" #include "catalog/pg_extension.h" #include "catalog/pg_inherits.h" -#include "catalog/pg_seclabel.h" #include "catalog/pg_type.h" #include "commands/defrem.h" #include "commands/extension.h" -#include "commands/seclabel.h" + #include "executor/executor.h" #include "nodes/nodeFuncs.h" @@ -191,40 +189,6 @@ spock_ProcessUtility(PlannedStmt *pstmt, const char *queryString, spock_autoddl_process(pstmt, queryString, context, toplevel_stmt); } -/* - * Derived from the core DeleteSecurityLabel routine - */ -static void -DeleteSecurityLabels(const char *provider) -{ - Relation pg_seclabel; - SysScanDesc scan; - HeapTuple htup; - - pg_seclabel = table_open(SecLabelRelationId, RowExclusiveLock); - - scan = systable_beginscan(pg_seclabel, InvalidOid, false, NULL, 0, NULL); - while (HeapTupleIsValid(htup = systable_getnext(scan))) - { - Datum datum; - bool isnull; - char *provider; - - datum = heap_getattr(htup, Anum_pg_seclabel_provider, - RelationGetDescr(pg_seclabel), &isnull); - Assert(!isnull); - provider = TextDatumGetCString(datum); - - if (strcmp(provider, spock_SECLABEL_PROVIDER) != 0) - continue; - - CatalogTupleDelete(pg_seclabel, &htup->t_self); - } - - systable_endscan(scan); - table_close(pg_seclabel, RowExclusiveLock); -} - /* * Handle object drop. * @@ -239,7 +203,6 @@ spock_object_access(ObjectAccessType access, { Oid save_userid = 0; int save_sec_context = 0; - ObjectAddress object; if (next_object_access_hook) (*next_object_access_hook) (access, classId, objectId, subId, arg); @@ -277,11 +240,7 @@ spock_object_access(ObjectAccessType access, * be handled by Postgres. */ if (dropping_spock_obj) - { - /* Need to drop any security labels created by the extension */ - DeleteSecurityLabels(spock_SECLABEL_PROVIDER); return; - } /* * Check that we have a local node. We need to elevate access because @@ -309,28 +268,6 @@ spock_object_access(ObjectAccessType access, /* Restore previous session privileges */ SetUserIdAndSecContext(save_userid, save_sec_context); } - /* SECURITY LABEL related section (see delta_apply for more details) */ - else if (access == OAT_POST_ALTER && subId > 0) - { - char *label; - - /* - * Something changes in the definition of the column. We have not enough - * data at the moment to check if the column will satisfy delta_apply - * type requirements. So, just warn and drop security label, if exists. - * TODO: the direction of further improvement is discovery of syscache - * or querying the table definition in attempt to identify the new type. - */ - ObjectAddressSubSet(object, classId, objectId, subId); - label = GetSecurityLabel(&object, "spock"); - if (label != NULL) - { - DeleteSecurityLabel(&object); - elog(WARNING, "the alter column statement removes spock security label '%s' on this column", - label); - pfree(label); - } - } } void diff --git a/tests/regress/expected/basic.out b/tests/regress/expected/basic.out index 7cbab6d8..642df00c 100644 --- a/tests/regress/expected/basic.out +++ b/tests/regress/expected/basic.out @@ -211,85 +211,3 @@ CREATE FUNCTION call_fn(creds text) RETURNS void AS $$ $$ LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE; SELECT call_fn(:fakecreds); ERROR: dsn "dbname=regression passwordXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -DROP FUNCTION call_fn; --- --- Basic SECURITY LABEL tests. Fix limits of acceptable behavior. --- Remember, these tests still check intra-node behaviour. --- --- A label creation checks -CREATE TABLE slabel (x integer, y text PRIMARY KEY); -SELECT spock.delta_apply('slabel', 'x'); - delta_apply -------------- - t -(1 row) - -SELECT spock.delta_apply('slabel', 'y'); -- ERROR -ERROR: type "text" can not be used in delta_apply conflict resolution -SELECT spock.delta_apply('slabel', 'z'); -- ERROR -ERROR: column z does not exist in the table slabel -SELECT spock.delta_apply('slabel', 'x'); -- repeating call do nothing - delta_apply -------------- - t -(1 row) - -SELECT objname, label FROM pg_seclabels; - objname | label -----------+------------- - slabel.x | delta_apply -(1 row) - --- Label drop checks -SELECT spock.delta_apply('slabel', 'x', true); - delta_apply -------------- - t -(1 row) - -SELECT spock.delta_apply('slabel', 'y', true); - delta_apply -------------- - f -(1 row) - -SELECT spock.delta_apply('slabel', 'z', true); - delta_apply -------------- - f -(1 row) - -SELECT objname, label FROM pg_seclabels; - objname | label ----------+------- -(0 rows) - --- Dependencies -SELECT spock.delta_apply('slabel', 'x', false); - delta_apply -------------- - t -(1 row) - -ALTER TABLE slabel ALTER COLUMN x TYPE text; -- just warn -WARNING: the alter column statement removes spock security label 'delta_apply' on this column -SELECT objname, label FROM pg_seclabels; - objname | label ----------+------- -(0 rows) - -ALTER TABLE slabel DROP COLUMN x; -ALTER TABLE slabel ADD COLUMN x numeric; -SELECT spock.delta_apply('slabel', 'x', false); - delta_apply -------------- - t -(1 row) - -ALTER TABLE slabel DROP COLUMN x; -SELECT objname, label FROM pg_seclabels; - objname | label ----------+------- -(0 rows) - -DROP TABLE slabel; diff --git a/tests/regress/expected/infofuncs.out b/tests/regress/expected/infofuncs.out index 1a7bf836..02b33bfc 100644 --- a/tests/regress/expected/infofuncs.out +++ b/tests/regress/expected/infofuncs.out @@ -20,24 +20,4 @@ WHERE extname = 'spock'; t (1 row) --- Check that security label is cleaned up on the extension drop -CREATE TABLE slabel (x money, y text PRIMARY KEY); -SELECT spock.delta_apply('slabel', 'x', false); - delta_apply -------------- - t -(1 row) - -SELECT objname, label FROM pg_seclabels; - objname | label -----------+------------- - slabel.x | delta_apply -(1 row) - DROP EXTENSION spock; -SELECT objname, label FROM pg_seclabels; - objname | label ----------+------- -(0 rows) - -DROP TABLE slabel; diff --git a/tests/regress/sql/basic.sql b/tests/regress/sql/basic.sql index 6e1db841..9bf4900d 100644 --- a/tests/regress/sql/basic.sql +++ b/tests/regress/sql/basic.sql @@ -110,35 +110,3 @@ CREATE FUNCTION call_fn(creds text) RETURNS void AS $$ $$ LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE; SELECT call_fn(:fakecreds); -DROP FUNCTION call_fn; - --- --- Basic SECURITY LABEL tests. Fix limits of acceptable behavior. --- Remember, these tests still check intra-node behaviour. --- - --- A label creation checks -CREATE TABLE slabel (x integer, y text PRIMARY KEY); -SELECT spock.delta_apply('slabel', 'x'); -SELECT spock.delta_apply('slabel', 'y'); -- ERROR -SELECT spock.delta_apply('slabel', 'z'); -- ERROR -SELECT spock.delta_apply('slabel', 'x'); -- repeating call do nothing -SELECT objname, label FROM pg_seclabels; - --- Label drop checks -SELECT spock.delta_apply('slabel', 'x', true); -SELECT spock.delta_apply('slabel', 'y', true); -SELECT spock.delta_apply('slabel', 'z', true); -SELECT objname, label FROM pg_seclabels; - --- Dependencies -SELECT spock.delta_apply('slabel', 'x', false); -ALTER TABLE slabel ALTER COLUMN x TYPE text; -- just warn -SELECT objname, label FROM pg_seclabels; -ALTER TABLE slabel DROP COLUMN x; -ALTER TABLE slabel ADD COLUMN x numeric; -SELECT spock.delta_apply('slabel', 'x', false); -ALTER TABLE slabel DROP COLUMN x; -SELECT objname, label FROM pg_seclabels; - -DROP TABLE slabel; diff --git a/tests/regress/sql/infofuncs.sql b/tests/regress/sql/infofuncs.sql index c0b19cdb..ee7536ac 100644 --- a/tests/regress/sql/infofuncs.sql +++ b/tests/regress/sql/infofuncs.sql @@ -9,12 +9,4 @@ SELECT spock.spock_version() = extversion FROM pg_extension WHERE extname = 'spock'; --- Check that security label is cleaned up on the extension drop -CREATE TABLE slabel (x money, y text PRIMARY KEY); -SELECT spock.delta_apply('slabel', 'x', false); -SELECT objname, label FROM pg_seclabels; - DROP EXTENSION spock; - -SELECT objname, label FROM pg_seclabels; -DROP TABLE slabel; From 991b1a0d3f1e1b3fc851bccabd095895fa3238ab Mon Sep 17 00:00:00 2001 From: Asif Rehman Date: Thu, 19 Feb 2026 14:35:57 +0500 Subject: [PATCH 8/9] Fix initdb assertion failure in attoptions patch for PG15/16/17 Skip HeapDetermineLogOldColumns() for catalog relations to avoid assertion failure during initdb when updating catalog tables. The fix adds IsCatalogRelationOid check before calling HeapDetermineLogOldColumns(), same as already done for PG18. (cherry picked from commit 782d0786e455af0ccfbf5b2e3e455f7e50f5841c) --- patches/15/pg15-015-attoptions.diff | 40 ++++++++++++++++------------- patches/16/pg16-015-attoptions.diff | 40 ++++++++++++++++------------- patches/17/pg17-015-attoptions.diff | 38 +++++++++++++++------------ 3 files changed, 65 insertions(+), 53 deletions(-) diff --git a/patches/15/pg15-015-attoptions.diff b/patches/15/pg15-015-attoptions.diff index 7b63311a..338cc845 100644 --- a/patches/15/pg15-015-attoptions.diff +++ b/patches/15/pg15-015-attoptions.diff @@ -1,5 +1,5 @@ diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c -index 5b696043c5..16ec985928 100644 +index 620602fba2d..8eebc4cde65 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -168,6 +168,15 @@ static relopt_bool boolRelOpts[] = @@ -38,7 +38,7 @@ index 5b696043c5..16ec985928 100644 /* list terminator */ {{NULL}} }; -@@ -2076,7 +2098,9 @@ attribute_reloptions(Datum reloptions, bool validate) +@@ -2085,7 +2107,9 @@ attribute_reloptions(Datum reloptions, bool validate) { static const relopt_parse_elt tab[] = { {"n_distinct", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct)}, @@ -50,10 +50,10 @@ index 5b696043c5..16ec985928 100644 return (bytea *) build_reloptions(reloptions, validate, diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c -index 88ab5f99c8..161333c5e0 100644 +index ef0e5eeca30..290cea25b12 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c -@@ -66,6 +66,7 @@ +@@ -67,6 +67,7 @@ #include "storage/smgr.h" #include "storage/spin.h" #include "storage/standby.h" @@ -61,7 +61,7 @@ index 88ab5f99c8..161333c5e0 100644 #include "utils/datum.h" #include "utils/inval.h" #include "utils/lsyscache.h" -@@ -86,6 +87,7 @@ static void check_lock_if_inplace_updateable_rel(Relation relation, +@@ -88,6 +89,7 @@ static void check_lock_if_inplace_updateable_rel(Relation relation, HeapTuple newtup); static void check_inplace_rel_lock(HeapTuple oldtup); #endif @@ -69,7 +69,7 @@ index 88ab5f99c8..161333c5e0 100644 static Bitmapset *HeapDetermineColumnsInfo(Relation relation, Bitmapset *interesting_cols, Bitmapset *external_cols, -@@ -117,6 +119,7 @@ static void index_delete_sort(TM_IndexDeleteOp *delstate); +@@ -122,6 +124,7 @@ static void index_delete_sort(TM_IndexDeleteOp *delstate); static int bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate); static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup); static HeapTuple ExtractReplicaIdentity(Relation rel, HeapTuple tup, bool key_required, @@ -77,7 +77,7 @@ index 88ab5f99c8..161333c5e0 100644 bool *copy); -@@ -2919,7 +2922,7 @@ l1: +@@ -2962,7 +2965,7 @@ l1: * Compute replica identity tuple before entering the critical section so * we don't PANIC upon a memory allocation failure. */ @@ -86,7 +86,7 @@ index 88ab5f99c8..161333c5e0 100644 /* * If this is the first possibly-multixact-able operation in the current -@@ -3150,6 +3153,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, +@@ -3193,6 +3196,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, Bitmapset *id_attrs; Bitmapset *interesting_attrs; Bitmapset *modified_attrs; @@ -94,15 +94,19 @@ index 88ab5f99c8..161333c5e0 100644 ItemId lp; HeapTupleData oldtup; HeapTuple heaptup; -@@ -3267,6 +3271,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, - modified_attrs = HeapDetermineColumnsInfo(relation, interesting_attrs, +@@ -3355,6 +3359,11 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, id_attrs, &oldtup, newtup, &id_has_external); -+ logged_old_attrs = HeapDetermineLogOldColumns(relation); ++ if (!IsCatalogRelationOid(relation->rd_id)) ++ logged_old_attrs = HeapDetermineLogOldColumns(relation); ++ else ++ logged_old_attrs = NULL; /* No need to log old values for catalog tables */ ++ /* * If we're not updating any "key" column, we can grab a weaker lock type. -@@ -3538,6 +3543,7 @@ l2: + * This allows for more concurrency when we are running simultaneously +@@ -3625,6 +3634,7 @@ l2: bms_free(key_attrs); bms_free(id_attrs); bms_free(modified_attrs); @@ -110,7 +114,7 @@ index 88ab5f99c8..161333c5e0 100644 bms_free(interesting_attrs); return result; } -@@ -3874,6 +3880,7 @@ l2: +@@ -3961,6 +3971,7 @@ l2: old_key_tuple = ExtractReplicaIdentity(relation, &oldtup, bms_overlap(modified_attrs, id_attrs) || id_has_external, @@ -118,7 +122,7 @@ index 88ab5f99c8..161333c5e0 100644 &old_key_copied); /* NO EREPORT(ERROR) from here till changes are logged */ -@@ -4023,6 +4030,7 @@ l2: +@@ -4110,6 +4121,7 @@ l2: bms_free(key_attrs); bms_free(id_attrs); bms_free(modified_attrs); @@ -126,7 +130,7 @@ index 88ab5f99c8..161333c5e0 100644 bms_free(interesting_attrs); return TM_Ok; -@@ -4195,6 +4203,26 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2, +@@ -4282,6 +4294,26 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2, } } @@ -153,7 +157,7 @@ index 88ab5f99c8..161333c5e0 100644 /* * Check which columns are being updated. * -@@ -8913,6 +8941,7 @@ log_heap_new_cid(Relation relation, HeapTuple tup) +@@ -9085,6 +9117,7 @@ log_heap_new_cid(Relation relation, HeapTuple tup) */ static HeapTuple ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, @@ -161,7 +165,7 @@ index 88ab5f99c8..161333c5e0 100644 bool *copy) { TupleDesc desc = RelationGetDescr(relation); -@@ -8945,13 +8974,16 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, +@@ -9117,13 +9150,16 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, } /* if the key isn't required and we're only logging the key, we're done */ @@ -180,7 +184,7 @@ index 88ab5f99c8..161333c5e0 100644 * If there's no defined replica identity columns, treat as !key_required. * (This case should not be reachable from heap_update, since that should diff --git a/src/include/utils/attoptcache.h b/src/include/utils/attoptcache.h -index ee37af9500..98b48a8fd8 100644 +index ee37af95001..98b48a8fd89 100644 --- a/src/include/utils/attoptcache.h +++ b/src/include/utils/attoptcache.h @@ -21,6 +21,8 @@ typedef struct AttributeOpts diff --git a/patches/16/pg16-015-attoptions.diff b/patches/16/pg16-015-attoptions.diff index ffea2382..9d542f87 100644 --- a/patches/16/pg16-015-attoptions.diff +++ b/patches/16/pg16-015-attoptions.diff @@ -1,5 +1,5 @@ diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c -index 469de9bb49..0119e5aa7c 100644 +index 9648a769580..087656983cd 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -168,6 +168,15 @@ static relopt_bool boolRelOpts[] = @@ -38,7 +38,7 @@ index 469de9bb49..0119e5aa7c 100644 /* list terminator */ {{NULL}} }; -@@ -2072,7 +2094,9 @@ attribute_reloptions(Datum reloptions, bool validate) +@@ -2081,7 +2103,9 @@ attribute_reloptions(Datum reloptions, bool validate) { static const relopt_parse_elt tab[] = { {"n_distinct", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct)}, @@ -50,10 +50,10 @@ index 469de9bb49..0119e5aa7c 100644 return (bytea *) build_reloptions(reloptions, validate, diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c -index 723e34e464..b2f845211d 100644 +index b251653540e..be87929c323 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c -@@ -67,6 +67,7 @@ +@@ -68,6 +68,7 @@ #include "storage/smgr.h" #include "storage/spin.h" #include "storage/standby.h" @@ -61,7 +61,7 @@ index 723e34e464..b2f845211d 100644 #include "utils/datum.h" #include "utils/inval.h" #include "utils/lsyscache.h" -@@ -87,6 +88,7 @@ static void check_lock_if_inplace_updateable_rel(Relation relation, +@@ -89,6 +90,7 @@ static void check_lock_if_inplace_updateable_rel(Relation relation, HeapTuple newtup); static void check_inplace_rel_lock(HeapTuple oldtup); #endif @@ -69,7 +69,7 @@ index 723e34e464..b2f845211d 100644 static Bitmapset *HeapDetermineColumnsInfo(Relation relation, Bitmapset *interesting_cols, Bitmapset *external_cols, -@@ -121,6 +123,7 @@ static void index_delete_sort(TM_IndexDeleteOp *delstate); +@@ -126,6 +128,7 @@ static void index_delete_sort(TM_IndexDeleteOp *delstate); static int bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate); static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup); static HeapTuple ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, @@ -77,7 +77,7 @@ index 723e34e464..b2f845211d 100644 bool *copy); -@@ -2780,7 +2783,7 @@ l1: +@@ -2823,7 +2826,7 @@ l1: * Compute replica identity tuple before entering the critical section so * we don't PANIC upon a memory allocation failure. */ @@ -86,7 +86,7 @@ index 723e34e464..b2f845211d 100644 /* * If this is the first possibly-multixact-able operation in the current -@@ -3013,6 +3016,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, +@@ -3056,6 +3059,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, Bitmapset *id_attrs; Bitmapset *interesting_attrs; Bitmapset *modified_attrs; @@ -94,15 +94,19 @@ index 723e34e464..b2f845211d 100644 ItemId lp; HeapTupleData oldtup; HeapTuple heaptup; -@@ -3135,6 +3139,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, - modified_attrs = HeapDetermineColumnsInfo(relation, interesting_attrs, +@@ -3225,6 +3229,11 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, id_attrs, &oldtup, newtup, &id_has_external); -+ logged_old_attrs = HeapDetermineLogOldColumns(relation); ++ if (!IsCatalogRelationOid(relation->rd_id)) ++ logged_old_attrs = HeapDetermineLogOldColumns(relation); ++ else ++ logged_old_attrs = NULL; /* No need to log old values for catalog tables */ ++ /* * If we're not updating any "key" column, we can grab a weaker lock type. -@@ -3409,6 +3414,7 @@ l2: + * This allows for more concurrency when we are running simultaneously +@@ -3498,6 +3507,7 @@ l2: bms_free(key_attrs); bms_free(id_attrs); bms_free(modified_attrs); @@ -110,7 +114,7 @@ index 723e34e464..b2f845211d 100644 bms_free(interesting_attrs); return result; } -@@ -3758,6 +3764,7 @@ l2: +@@ -3847,6 +3857,7 @@ l2: old_key_tuple = ExtractReplicaIdentity(relation, &oldtup, bms_overlap(modified_attrs, id_attrs) || id_has_external, @@ -118,7 +122,7 @@ index 723e34e464..b2f845211d 100644 &old_key_copied); /* NO EREPORT(ERROR) from here till changes are logged */ -@@ -3924,6 +3931,7 @@ l2: +@@ -4013,6 +4024,7 @@ l2: bms_free(key_attrs); bms_free(id_attrs); bms_free(modified_attrs); @@ -126,7 +130,7 @@ index 723e34e464..b2f845211d 100644 bms_free(interesting_attrs); return TM_Ok; -@@ -4096,6 +4104,26 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2, +@@ -4185,6 +4197,26 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2, } } @@ -153,7 +157,7 @@ index 723e34e464..b2f845211d 100644 /* * Check which columns are being updated. * -@@ -9051,6 +9079,7 @@ log_heap_new_cid(Relation relation, HeapTuple tup) +@@ -9225,6 +9257,7 @@ log_heap_new_cid(Relation relation, HeapTuple tup) */ static HeapTuple ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, @@ -161,7 +165,7 @@ index 723e34e464..b2f845211d 100644 bool *copy) { TupleDesc desc = RelationGetDescr(relation); -@@ -9083,13 +9112,16 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, +@@ -9257,13 +9290,16 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, } /* if the key isn't required and we're only logging the key, we're done */ @@ -180,7 +184,7 @@ index 723e34e464..b2f845211d 100644 * If there's no defined replica identity columns, treat as !key_required. * (This case should not be reachable from heap_update, since that should diff --git a/src/include/utils/attoptcache.h b/src/include/utils/attoptcache.h -index e4119b6aa2..6354a98157 100644 +index e4119b6aa28..6354a981570 100644 --- a/src/include/utils/attoptcache.h +++ b/src/include/utils/attoptcache.h @@ -21,6 +21,8 @@ typedef struct AttributeOpts diff --git a/patches/17/pg17-015-attoptions.diff b/patches/17/pg17-015-attoptions.diff index df156695..0fe3209c 100644 --- a/patches/17/pg17-015-attoptions.diff +++ b/patches/17/pg17-015-attoptions.diff @@ -1,5 +1,5 @@ diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c -index d6eb5d85599..4ef2e55cc44 100644 +index c6a2d13be8d..8e93a084ad6 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -166,6 +166,15 @@ static relopt_bool boolRelOpts[] = @@ -38,7 +38,7 @@ index d6eb5d85599..4ef2e55cc44 100644 /* list terminator */ {{NULL}} }; -@@ -2070,7 +2092,9 @@ attribute_reloptions(Datum reloptions, bool validate) +@@ -2079,7 +2101,9 @@ attribute_reloptions(Datum reloptions, bool validate) { static const relopt_parse_elt tab[] = { {"n_distinct", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct)}, @@ -50,10 +50,10 @@ index d6eb5d85599..4ef2e55cc44 100644 return (bytea *) build_reloptions(reloptions, validate, diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c -index 95e3be524a7..708d55b2618 100644 +index 1bacea4360e..20f60178c72 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c -@@ -64,6 +64,7 @@ +@@ -65,6 +65,7 @@ #include "storage/predicate.h" #include "storage/procarray.h" #include "storage/standby.h" @@ -61,7 +61,7 @@ index 95e3be524a7..708d55b2618 100644 #include "utils/datum.h" #include "utils/injection_point.h" #include "utils/inval.h" -@@ -85,6 +86,7 @@ static void check_lock_if_inplace_updateable_rel(Relation relation, +@@ -86,6 +87,7 @@ static void check_lock_if_inplace_updateable_rel(Relation relation, HeapTuple newtup); static void check_inplace_rel_lock(HeapTuple oldtup); #endif @@ -69,7 +69,7 @@ index 95e3be524a7..708d55b2618 100644 static Bitmapset *HeapDetermineColumnsInfo(Relation relation, Bitmapset *interesting_cols, Bitmapset *external_cols, -@@ -121,6 +123,7 @@ static void index_delete_sort(TM_IndexDeleteOp *delstate); +@@ -125,6 +127,7 @@ static void index_delete_sort(TM_IndexDeleteOp *delstate); static int bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate); static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup); static HeapTuple ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, @@ -77,7 +77,7 @@ index 95e3be524a7..708d55b2618 100644 bool *copy); -@@ -2938,7 +2941,7 @@ l1: +@@ -2980,7 +2983,7 @@ l1: * Compute replica identity tuple before entering the critical section so * we don't PANIC upon a memory allocation failure. */ @@ -86,7 +86,7 @@ index 95e3be524a7..708d55b2618 100644 /* * If this is the first possibly-multixact-able operation in the current -@@ -3171,6 +3174,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, +@@ -3213,6 +3216,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, Bitmapset *id_attrs; Bitmapset *interesting_attrs; Bitmapset *modified_attrs; @@ -94,15 +94,19 @@ index 95e3be524a7..708d55b2618 100644 ItemId lp; HeapTupleData oldtup; HeapTuple heaptup; -@@ -3338,6 +3342,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, - modified_attrs = HeapDetermineColumnsInfo(relation, interesting_attrs, +@@ -3383,6 +3387,11 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, id_attrs, &oldtup, newtup, &id_has_external); -+ logged_old_attrs = HeapDetermineLogOldColumns(relation); ++ if (!IsCatalogRelationOid(relation->rd_id)) ++ logged_old_attrs = HeapDetermineLogOldColumns(relation); ++ else ++ logged_old_attrs = NULL; /* No need to log old values for catalog tables */ ++ /* * If we're not updating any "key" column, we can grab a weaker lock type. -@@ -3612,6 +3617,7 @@ l2: + * This allows for more concurrency when we are running simultaneously +@@ -3656,6 +3665,7 @@ l2: bms_free(key_attrs); bms_free(id_attrs); bms_free(modified_attrs); @@ -110,7 +114,7 @@ index 95e3be524a7..708d55b2618 100644 bms_free(interesting_attrs); return result; } -@@ -3961,6 +3967,7 @@ l2: +@@ -4005,6 +4015,7 @@ l2: old_key_tuple = ExtractReplicaIdentity(relation, &oldtup, bms_overlap(modified_attrs, id_attrs) || id_has_external, @@ -118,7 +122,7 @@ index 95e3be524a7..708d55b2618 100644 &old_key_copied); /* NO EREPORT(ERROR) from here till changes are logged */ -@@ -4127,6 +4134,7 @@ l2: +@@ -4171,6 +4182,7 @@ l2: bms_free(key_attrs); bms_free(id_attrs); bms_free(modified_attrs); @@ -126,7 +130,7 @@ index 95e3be524a7..708d55b2618 100644 bms_free(interesting_attrs); return TM_Ok; -@@ -4299,6 +4307,26 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2, +@@ -4343,6 +4355,26 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2, } } @@ -153,7 +157,7 @@ index 95e3be524a7..708d55b2618 100644 /* * Check which columns are being updated. * -@@ -9076,6 +9104,7 @@ log_heap_new_cid(Relation relation, HeapTuple tup) +@@ -9207,6 +9239,7 @@ log_heap_new_cid(Relation relation, HeapTuple tup) */ static HeapTuple ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, @@ -161,7 +165,7 @@ index 95e3be524a7..708d55b2618 100644 bool *copy) { TupleDesc desc = RelationGetDescr(relation); -@@ -9108,13 +9137,16 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, +@@ -9239,13 +9272,16 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, } /* if the key isn't required and we're only logging the key, we're done */ From 519ab810d1a648838516d550c33eefdf85affe03 Mon Sep 17 00:00:00 2001 From: Mason Sharp Date: Sat, 2 May 2026 14:04:36 -0700 Subject: [PATCH 9/9] Fix git patch reversions --- src/spock_executor.c | 1 + src/spock_relcache.c | 1 + 2 files changed, 2 insertions(+) diff --git a/src/spock_executor.c b/src/spock_executor.c index 8aaa8361..ad469abb 100644 --- a/src/spock_executor.c +++ b/src/spock_executor.c @@ -210,6 +210,7 @@ spock_object_access(ObjectAccessType access, if (access == OAT_DROP) { ObjectAccessDrop *drop_arg = (ObjectAccessDrop *) arg; + ObjectAddress object; DropBehavior behavior; /* No need to check for internal deletions. */ diff --git a/src/spock_relcache.c b/src/spock_relcache.c index da51e3b3..1a7a5988 100644 --- a/src/spock_relcache.c +++ b/src/spock_relcache.c @@ -87,6 +87,7 @@ spock_relation_open(uint32 remoteid, LOCKMODE lockmode) { RangeVar *rv = makeNode(RangeVar); int i; + TupleDesc desc; ResultRelInfo *relinfo; rv->schemaname = (char *) entry->nspname;