From 70468a850c071d854bdd498569f3f513fdfc8ff6 Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Sat, 26 Sep 2026 06:06:58 +0530 Subject: [PATCH v76] Add test module demonstrating a residual conflict-log-table size gap Add boomtype, a demonstration C-language type that stores and sends/receives only 4 bytes but whose output function always renders 600MB regardless of input, plus a TAP test that replicates a table keyed on two such columns over a binary=true subscription and forces an update_missing conflict, overflowing the conflict log table's JSON rendering past the 1GB limit -- illustrating that CONFLICT_MAX_VALUE_SIZE in conflict.c bounds a value's stored size, not what its output function actually returns, a gap already noted by the "XXX" comment in build_index_key_json() and not something this documents as a bug to fix, since it requires a C-language output function and an explicit binary=true subscription, well outside what an ordinary subscription owner can trigger with SQL alone. --- .../modules/test_conflict_typeout/Makefile | 20 +++ .../t/001_oversized_typeout.pl | 88 ++++++++++ .../test_conflict_typeout--1.0.sql | 49 ++++++ .../test_conflict_typeout.c | 156 ++++++++++++++++++ .../test_conflict_typeout.control | 4 + 5 files changed, 317 insertions(+) create mode 100644 src/test/modules/test_conflict_typeout/Makefile create mode 100644 src/test/modules/test_conflict_typeout/t/001_oversized_typeout.pl create mode 100644 src/test/modules/test_conflict_typeout/test_conflict_typeout--1.0.sql create mode 100644 src/test/modules/test_conflict_typeout/test_conflict_typeout.c create mode 100644 src/test/modules/test_conflict_typeout/test_conflict_typeout.control diff --git a/src/test/modules/test_conflict_typeout/Makefile b/src/test/modules/test_conflict_typeout/Makefile new file mode 100644 index 00000000000..5ceeb374f03 --- /dev/null +++ b/src/test/modules/test_conflict_typeout/Makefile @@ -0,0 +1,20 @@ +# src/test/modules/test_conflict_typeout/Makefile + +MODULES = test_conflict_typeout + +EXTENSION = test_conflict_typeout +DATA = test_conflict_typeout--1.0.sql +PGFILEDESC = "test_conflict_typeout - demonstrates the conflict log table's output-function size gap" + +TAP_TESTS = 1 + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = src/test/modules/test_conflict_typeout +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/src/test/modules/test_conflict_typeout/t/001_oversized_typeout.pl b/src/test/modules/test_conflict_typeout/t/001_oversized_typeout.pl new file mode 100644 index 00000000000..b8b4196b8a5 --- /dev/null +++ b/src/test/modules/test_conflict_typeout/t/001_oversized_typeout.pl @@ -0,0 +1,88 @@ +# Copyright (c) 2025-2026, PostgreSQL Global Development Group + +# Demonstrates a documented residual gap in the conflict log table's +# per-column size cap (CONFLICT_MAX_VALUE_SIZE in conflict.c): the cap +# bounds a value's *stored* size before deciding whether to render it into +# the log, but never bounds what the type's own output function actually +# returns. A C-language type that stores a few bytes, sends/receives just +# as few, but whose output function ignores its input and always renders +# hundreds of megabytes, sails through the cap untouched. +# +# This is not something the patch is expected to catch (see the "XXX" note +# above the cap in build_index_key_json()) -- reaching it requires a +# C-language output function and a binary=true subscription, well outside +# what an ordinary subscription owner can trigger with SQL alone. This +# test exists to make that documented gap concrete, not to assert a fix. +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node_publisher = PostgreSQL::Test::Cluster->new('publisher'); +$node_publisher->init(allows_streaming => 'logical'); +$node_publisher->start; + +my $node_subscriber = PostgreSQL::Test::Cluster->new('subscriber'); +$node_subscriber->init(allows_streaming => 'logical'); +$node_subscriber->start; + +for my $node ($node_publisher, $node_subscriber) +{ + $node->safe_psql('postgres', 'CREATE EXTENSION test_conflict_typeout'); + $node->safe_psql('postgres', + "CREATE TABLE bt (a int, b boomtype, c boomtype, PRIMARY KEY (b, c))" + ); +} + +my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres'; +$node_publisher->safe_psql('postgres', + "CREATE PUBLICATION pub_bt FOR TABLE bt"); + +my $appname = 'sub_bt'; +$node_subscriber->safe_psql( + 'postgres', + "CREATE SUBSCRIPTION sub_bt + CONNECTION '$publisher_connstr application_name=$appname' + PUBLICATION pub_bt WITH (binary = true, conflict_log_destination = all)" +); + +$node_subscriber->wait_for_subscription_sync($node_publisher, $appname); + +# Only 4 bytes per column cross the wire either way (boomtype_send), so +# this succeeds regardless of the pathological output function. +$node_publisher->safe_psql('postgres', "INSERT INTO bt VALUES (1, '100', '200')"); +$node_publisher->wait_for_catchup($appname); + +is( $node_subscriber->safe_psql('postgres', 'SELECT a FROM bt'), + '1', + 'row replicated using the compact binary send/recv, not the output function' +); + +# Delete the row only on the subscriber, so the publisher's later UPDATE +# becomes an update_missing conflict -- a LOG-level conflict that must +# render the (b, c) replica identity key into the conflict log table. +$node_subscriber->safe_psql('postgres', 'DELETE FROM bt'); + +my $log_offset = -s $node_subscriber->logfile; + +$node_publisher->safe_psql('postgres', + "UPDATE bt SET a = 2 WHERE b = '100' AND c = '200'"); + +# Rendering both (b, c) values via boomtype_out produces 600MB each, +# 1.2GB combined -- past the ~1GB StringInfo limit -- even though the +# per-column size cap saw only the 8-byte stored form of each and let +# both through unomitted. +$node_subscriber->wait_for_log( + qr/string buffer exceeds maximum allowed length/, + $log_offset); + +pass( + 'oversized user-defined typeoutput overflows conflict log table rendering, ' + . 'despite each value passing the per-column size cap' +); + +$node_subscriber->stop; +$node_publisher->stop; + +done_testing(); diff --git a/src/test/modules/test_conflict_typeout/test_conflict_typeout--1.0.sql b/src/test/modules/test_conflict_typeout/test_conflict_typeout--1.0.sql new file mode 100644 index 00000000000..c359e137676 --- /dev/null +++ b/src/test/modules/test_conflict_typeout/test_conflict_typeout--1.0.sql @@ -0,0 +1,49 @@ +/* src/test/modules/test_conflict_typeout/test_conflict_typeout--1.0.sql */ + +\echo Use "CREATE EXTENSION test_conflict_typeout" to load this file. \quit + +CREATE TYPE boomtype; + +CREATE FUNCTION boomtype_in(cstring) RETURNS boomtype + AS 'MODULE_PATHNAME', 'boomtype_in' LANGUAGE C IMMUTABLE STRICT; +CREATE FUNCTION boomtype_out(boomtype) RETURNS cstring + AS 'MODULE_PATHNAME', 'boomtype_out' LANGUAGE C IMMUTABLE STRICT; +CREATE FUNCTION boomtype_send(boomtype) RETURNS bytea + AS 'MODULE_PATHNAME', 'boomtype_send' LANGUAGE C IMMUTABLE STRICT; +CREATE FUNCTION boomtype_recv(internal) RETURNS boomtype + AS 'MODULE_PATHNAME', 'boomtype_recv' LANGUAGE C IMMUTABLE STRICT; + +CREATE TYPE boomtype ( + INPUT = boomtype_in, + OUTPUT = boomtype_out, + SEND = boomtype_send, + RECEIVE = boomtype_recv, + INTERNALLENGTH = VARIABLE, + STORAGE = plain +); + +CREATE FUNCTION boomtype_cmp(boomtype, boomtype) RETURNS int4 + AS 'MODULE_PATHNAME', 'boomtype_cmp' LANGUAGE C IMMUTABLE STRICT; + +/* + * Written directly in C, not as SQL-language wrappers around boomtype_cmp: + * the apply worker runs with search_path = '', and an unqualified name + * inside a SQL function body would fail to resolve there. + */ +CREATE FUNCTION boomtype_lt(boomtype, boomtype) RETURNS bool + AS 'MODULE_PATHNAME', 'boomtype_lt' LANGUAGE C IMMUTABLE STRICT; +CREATE FUNCTION boomtype_eq(boomtype, boomtype) RETURNS bool + AS 'MODULE_PATHNAME', 'boomtype_eq' LANGUAGE C IMMUTABLE STRICT; +CREATE FUNCTION boomtype_gt(boomtype, boomtype) RETURNS bool + AS 'MODULE_PATHNAME', 'boomtype_gt' LANGUAGE C IMMUTABLE STRICT; + +CREATE OPERATOR < (LEFTARG = boomtype, RIGHTARG = boomtype, PROCEDURE = boomtype_lt); +CREATE OPERATOR = (LEFTARG = boomtype, RIGHTARG = boomtype, PROCEDURE = boomtype_eq, + COMMUTATOR = =); +CREATE OPERATOR > (LEFTARG = boomtype, RIGHTARG = boomtype, PROCEDURE = boomtype_gt); + +CREATE OPERATOR CLASS boomtype_ops DEFAULT FOR TYPE boomtype USING btree AS + OPERATOR 1 <, + OPERATOR 3 =, + OPERATOR 5 >, + FUNCTION 1 boomtype_cmp(boomtype, boomtype); diff --git a/src/test/modules/test_conflict_typeout/test_conflict_typeout.c b/src/test/modules/test_conflict_typeout/test_conflict_typeout.c new file mode 100644 index 00000000000..687e715a397 --- /dev/null +++ b/src/test/modules/test_conflict_typeout/test_conflict_typeout.c @@ -0,0 +1,156 @@ +/*-------------------------------------------------------------------------- + * test_conflict_typeout.c + * + * Demonstration type for the conflict-log-table residual gap: a + * C-language type whose output function is disproportionate to its + * storage/send footprint. The conflict log table's per-column size cap + * (CONFLICT_MAX_VALUE_SIZE in conflict.c) only measures a value's stored + * size before deciding whether to render it, never what the output + * function actually produces, so a type built this way sails through the + * cap and can still overflow the log table's JSON rendering. + * + * boomtype stores a single 4-byte integer, wrapped in a varlena header. + * Its send/receive functions are equally small, so a subscription with + * binary = true only ever moves 4 bytes of this type across the wire. + * Its output function ignores the stored value entirely and always + * renders BOOM_OUTPUT_SIZE bytes. + * + * Copyright (c) 2025-2026, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/test/modules/test_conflict_typeout/test_conflict_typeout.c + * + *-------------------------------------------------------------------------- + */ +#include "postgres.h" + +#include "fmgr.h" +#include "lib/stringinfo.h" +#include "libpq/pqformat.h" +#include "utils/builtins.h" +#include "varatt.h" + +PG_MODULE_MAGIC; + +/* + * Two columns of this size land at 1.2GB combined, comfortably over the + * ~1GB StringInfo limit, while each individual value is well under it. + */ +#define BOOM_OUTPUT_SIZE 600000000 + +PG_FUNCTION_INFO_V1(boomtype_in); +PG_FUNCTION_INFO_V1(boomtype_out); +PG_FUNCTION_INFO_V1(boomtype_send); +PG_FUNCTION_INFO_V1(boomtype_recv); +PG_FUNCTION_INFO_V1(boomtype_cmp); +PG_FUNCTION_INFO_V1(boomtype_lt); +PG_FUNCTION_INFO_V1(boomtype_eq); +PG_FUNCTION_INFO_V1(boomtype_gt); + +static int32 +boomtype_cmp_internal(bytea *a, bytea *b) +{ + int32 av, + bv; + + memcpy(&av, VARDATA_ANY(a), sizeof(int32)); + memcpy(&bv, VARDATA_ANY(b), sizeof(int32)); + return av < bv ? -1 : (av > bv ? 1 : 0); +} + +Datum +boomtype_in(PG_FUNCTION_ARGS) +{ + char *str = PG_GETARG_CSTRING(0); + int32 val = pg_strtoint32(str); + bytea *result = (bytea *) palloc(VARHDRSZ + sizeof(int32)); + + SET_VARSIZE(result, VARHDRSZ + sizeof(int32)); + memcpy(VARDATA(result), &val, sizeof(int32)); + PG_RETURN_BYTEA_P(result); +} + +/* + * Ignores the (8-byte-stored) input entirely. A well-behaved output + * function's result size tracks its input; this one doesn't, which is + * exactly what the conflict log's size cap cannot detect, since the cap + * only measures the raw stored size of the value, not what this function + * is about to return. + */ +Datum +boomtype_out(PG_FUNCTION_ARGS) +{ + char *huge = palloc(BOOM_OUTPUT_SIZE + 1); + + memset(huge, 'x', BOOM_OUTPUT_SIZE); + huge[BOOM_OUTPUT_SIZE] = '\0'; + PG_RETURN_CSTRING(huge); +} + +/* Compact binary form -- this is what actually crosses the wire. */ +Datum +boomtype_send(PG_FUNCTION_ARGS) +{ + bytea *val = PG_GETARG_BYTEA_PP(0); + int32 intval; + StringInfoData buf; + + memcpy(&intval, VARDATA_ANY(val), sizeof(int32)); + pq_begintypsend(&buf); + pq_sendint32(&buf, intval); + PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); +} + +Datum +boomtype_recv(PG_FUNCTION_ARGS) +{ + StringInfo buf = (StringInfo) PG_GETARG_POINTER(0); + int32 val = pq_getmsgint(buf, 4); + bytea *result = (bytea *) palloc(VARHDRSZ + sizeof(int32)); + + SET_VARSIZE(result, VARHDRSZ + sizeof(int32)); + memcpy(VARDATA(result), &val, sizeof(int32)); + PG_RETURN_BYTEA_P(result); +} + +/* + * Just enough comparison support for a btree opclass, so boomtype can be a + * PK. Written directly in C, rather than as SQL-language wrappers around + * boomtype_cmp, because the apply worker runs with search_path = '' and an + * unqualified name inside a SQL function body would fail to resolve there. + */ +Datum +boomtype_cmp(PG_FUNCTION_ARGS) +{ + bytea *a = PG_GETARG_BYTEA_PP(0); + bytea *b = PG_GETARG_BYTEA_PP(1); + + PG_RETURN_INT32(boomtype_cmp_internal(a, b)); +} + +Datum +boomtype_lt(PG_FUNCTION_ARGS) +{ + bytea *a = PG_GETARG_BYTEA_PP(0); + bytea *b = PG_GETARG_BYTEA_PP(1); + + PG_RETURN_BOOL(boomtype_cmp_internal(a, b) < 0); +} + +Datum +boomtype_eq(PG_FUNCTION_ARGS) +{ + bytea *a = PG_GETARG_BYTEA_PP(0); + bytea *b = PG_GETARG_BYTEA_PP(1); + + PG_RETURN_BOOL(boomtype_cmp_internal(a, b) == 0); +} + +Datum +boomtype_gt(PG_FUNCTION_ARGS) +{ + bytea *a = PG_GETARG_BYTEA_PP(0); + bytea *b = PG_GETARG_BYTEA_PP(1); + + PG_RETURN_BOOL(boomtype_cmp_internal(a, b) > 0); +} diff --git a/src/test/modules/test_conflict_typeout/test_conflict_typeout.control b/src/test/modules/test_conflict_typeout/test_conflict_typeout.control new file mode 100644 index 00000000000..bd7369d8309 --- /dev/null +++ b/src/test/modules/test_conflict_typeout/test_conflict_typeout.control @@ -0,0 +1,4 @@ +comment = 'Demonstrates the conflict log table residual output-function size gap' +default_version = '1.0' +module_pathname = '$libdir/test_conflict_typeout' +relocatable = false -- 2.54.0