From 8e0eec9819b84de52c57e777edb70815df98d158 Mon Sep 17 00:00:00 2001 From: Chris Munns Date: Thu, 16 Jul 2026 11:14:41 -0400 Subject: [PATCH] Filter internal partition-FK clones from FK constraint listing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pgcopydb lists FK constraints straight from pg_constraint and creates them directly. On a partitioned schema that also returns Postgres's internal per-partition FK clones (pg_constraint.conparentid <> 0): once the top-level constraint is created it cascades to every partition, then pgcopydb tries to create each clone again and hits "constraint already exists" — one ERROR per clone, so a migration that actually succeeds looks like it failed. Filter the clones out with c.conparentid = 0; recreating the top-level constraint re-cascades to all partitions, so nothing is lost (mirrors pg_dump). It cannot be a static SQL constant, though: - conparentid is PG 11+ only; referencing it on a 9.x/10 source aborts the clone with "column c.conparentid does not exist". Those sources are still supported (follow-9.6) and cannot have partition FK clones anyway. - --defer-validate-fks relies on the clones: PG < 18 rejects NOT VALID on a partitioned parent, so pgcopydb skips the parent and creates the per-leaf clones NOT VALID instead, which is the only enforcement that lands. So the predicate is injected at runtime in schema_list_fk_constraints only when the source is PG >= 11 and --defer-validate-fks is off. Adds a fk-partition-clone test (HASH-partitioned table with FKs to regular tables) asserting the clone finishes with no "constraint already exists" errors and correct, validated FKs on the target. Verified locally: full suite green on PG16/PG18; FK and partition suites green on PG16/17/18. --- .github/workflows/run-tests-tiered.yml | 1 + src/bin/pgcopydb/copydb_schema.c | 3 +- src/bin/pgcopydb/schema.c | 99 ++++++++++++++- src/bin/pgcopydb/schema.h | 3 +- tests/Makefile | 7 +- tests/fk-partition-clone/Dockerfile | 9 ++ tests/fk-partition-clone/Makefile | 15 +++ tests/fk-partition-clone/compose.yaml | 42 +++++++ tests/fk-partition-clone/copydb.sh | 159 +++++++++++++++++++++++++ 9 files changed, 332 insertions(+), 6 deletions(-) create mode 100644 tests/fk-partition-clone/Dockerfile create mode 100644 tests/fk-partition-clone/Makefile create mode 100644 tests/fk-partition-clone/compose.yaml create mode 100755 tests/fk-partition-clone/copydb.sh diff --git a/.github/workflows/run-tests-tiered.yml b/.github/workflows/run-tests-tiered.yml index 932253975..5f826f6a2 100644 --- a/.github/workflows/run-tests-tiered.yml +++ b/.github/workflows/run-tests-tiered.yml @@ -158,6 +158,7 @@ jobs: - follow-defer-indexes - fk-not-valid - defer-validate-fks + - fk-partition-clone - follow-defer-validate-fks - follow-sequence-reset steps: diff --git a/src/bin/pgcopydb/copydb_schema.c b/src/bin/pgcopydb/copydb_schema.c index 4a87c33f9..ffb3363e3 100644 --- a/src/bin/pgcopydb/copydb_schema.c +++ b/src/bin/pgcopydb/copydb_schema.c @@ -994,7 +994,8 @@ copydb_prepare_index_specs(CopyDataSpec *specs, PGSQL *pgsql) * schema_list_all_indexes. pgcopydb handles FK constraint creation * directly instead of delegating to pg_restore. */ - if (!schema_list_fk_constraints(pgsql, &(specs->filters), sourceDB)) + if (!schema_list_fk_constraints(pgsql, &(specs->filters), sourceDB, + specs->deferValidateFKs)) { /* errors have already been logged */ return false; diff --git a/src/bin/pgcopydb/schema.c b/src/bin/pgcopydb/schema.c index 545c44dde..a189fb992 100644 --- a/src/bin/pgcopydb/schema.c +++ b/src/bin/pgcopydb/schema.c @@ -3204,6 +3204,10 @@ static void getFKConstraintArray(void *ctx, PGresult *result); * Base SELECT columns and FROM/WHERE clauses shared across all FK constraint * filter variants. The filter-specific JOIN clauses are inserted between the * FROM block and the WHERE block via the array below. + * + * The conparentid partition-clone filter is added at runtime by + * schema_list_fk_constraints (it matches the literal " WHERE c.contype = 'f' " + * below, so keep that text verbatim). */ #define FK_SELECT_COLS \ "SELECT c.oid, " \ @@ -3345,7 +3349,8 @@ struct FilteringQueries listSourceFKConstraintsSQL[] = { bool schema_list_fk_constraints(PGSQL *pgsql, SourceFilters *filters, - DatabaseCatalog *catalog) + DatabaseCatalog *catalog, + bool deferValidateFKs) { SourceFKConstraintContext context = { catalog, false }; @@ -3372,9 +3377,99 @@ schema_list_fk_constraints(PGSQL *pgsql, return false; } - bool ok = pgsql_execute_with_params(pgsql, sql, 0, NULL, NULL, + /* + * Drop Postgres's internal per-partition FK clones (conparentid <> 0): the + * top-level constraint re-cascades on the target, so the clones are + * redundant and collide. Gated because conparentid is PG 11+ only, and + * --defer-validate-fks needs the clones (PG < 18 can't create a NOT VALID + * FK on a partitioned parent, so the per-leaf clones are the enforcement). + */ + char *gatedSql = NULL; + + if (pgsql->pgversion_num == 0) + { + if (!pgsql_server_version(pgsql)) + { + /* errors have already been logged */ + if (filters->ctePreamble != NULL) + { + free(sql); + } + return false; + } + } + + if (pgsql->pgversion_num >= 110000 && !deferValidateFKs) + { + const char *anchor = " WHERE c.contype = 'f' "; + const char *clause = " AND c.conparentid = 0 "; + char *pos = strstr(sql, anchor); + + if (pos == NULL) + { + log_error("BUG: FK constraints query is missing its \"%s\" clause; " + "cannot filter out partition FK clones", anchor); + if (filters->ctePreamble != NULL) + { + free(sql); + } + return false; + } + + PQExpBuffer buf = createPQExpBuffer(); + + if (buf == NULL) + { + log_error("Failed to allocate FK constraints query buffer"); + if (filters->ctePreamble != NULL) + { + free(sql); + } + return false; + } + + /* splice after the anchor; append (not printf) as the query has %I */ + size_t prefixLen = (size_t) (pos - sql) + strlen(anchor); + + appendBinaryPQExpBuffer(buf, sql, prefixLen); + appendPQExpBufferStr(buf, clause); + appendPQExpBufferStr(buf, sql + prefixLen); + + if (PQExpBufferBroken(buf)) + { + log_error("Failed to build FK constraints query: out of memory"); + (void) destroyPQExpBuffer(buf); + if (filters->ctePreamble != NULL) + { + free(sql); + } + return false; + } + + gatedSql = strdup(buf->data); + (void) destroyPQExpBuffer(buf); + + if (gatedSql == NULL) + { + log_error("Failed to build FK constraints query: out of memory"); + if (filters->ctePreamble != NULL) + { + free(sql); + } + return false; + } + } + + bool ok = pgsql_execute_with_params(pgsql, + gatedSql != NULL ? gatedSql : sql, + 0, NULL, NULL, &context, &getFKConstraintArray); + if (gatedSql != NULL) + { + free(gatedSql); + } + if (filters->ctePreamble != NULL && sql != NULL) { free(sql); diff --git a/src/bin/pgcopydb/schema.h b/src/bin/pgcopydb/schema.h index c171d4bc7..23169e269 100644 --- a/src/bin/pgcopydb/schema.h +++ b/src/bin/pgcopydb/schema.h @@ -533,7 +533,8 @@ bool schema_list_pg_depend(PGSQL *pgsql, bool schema_list_fk_constraints(PGSQL *pgsql, SourceFilters *filters, - DatabaseCatalog *catalog); + DatabaseCatalog *catalog, + bool deferValidateFKs); bool schema_send_table_checksum(PGSQL *pgsql, SourceTable *table); bool schema_fetch_table_checksum(PGSQL *pgsql, TableChecksum *sum, bool *done); diff --git a/tests/Makefile b/tests/Makefile index 06983c7d4..62b476f8b 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -15,7 +15,7 @@ all: pagila pagila-multi-steps blobs unit filtering filtering-standby extensions follow-wal2json follow-standby follow-9.6 follow-data-only follow-target-reconnect \ endpos-in-multi-wal-txn exclude-extension \ blob-snapshot-release follow-defer-indexes fk-not-valid defer-validate-fks \ - follow-defer-validate-fks follow-sequence-reset; + fk-partition-clone follow-defer-validate-fks follow-sequence-reset; pagila: build $(MAKE) -C $@ @@ -86,6 +86,9 @@ fk-not-valid: build defer-validate-fks: build $(MAKE) -C $@ +fk-partition-clone: build + $(MAKE) -C $@ + follow-defer-validate-fks: build $(MAKE) -C $@ @@ -105,4 +108,4 @@ build: .PHONY: follow-wal2json follow-standby follow-9.6 follow-target-reconnect .PHONY: endpos-in-multi-wal-txn exclude-extension .PHONY: blob-snapshot-release follow-defer-indexes fk-not-valid defer-validate-fks -.PHONY: follow-defer-validate-fks follow-sequence-reset +.PHONY: fk-partition-clone follow-defer-validate-fks follow-sequence-reset diff --git a/tests/fk-partition-clone/Dockerfile b/tests/fk-partition-clone/Dockerfile new file mode 100644 index 000000000..1a4cb78cf --- /dev/null +++ b/tests/fk-partition-clone/Dockerfile @@ -0,0 +1,9 @@ +FROM pagila + +COPY --from=pgcopydb /usr/local/bin/pgcopydb /usr/local/bin + +WORKDIR /usr/src/pgcopydb +COPY copydb.sh copydb.sh + +USER docker +CMD ["/usr/src/pgcopydb/copydb.sh"] diff --git a/tests/fk-partition-clone/Makefile b/tests/fk-partition-clone/Makefile new file mode 100644 index 000000000..d405ef0e4 --- /dev/null +++ b/tests/fk-partition-clone/Makefile @@ -0,0 +1,15 @@ +# Copyright (c) 2021 The PostgreSQL Global Development Group. +# Licensed under the PostgreSQL License. + +test: down run down ; + +run: build + $(DOCKER) compose run test + +down: + $(DOCKER) compose down + +build: + $(DOCKER) compose build + +.PHONY: down build test diff --git a/tests/fk-partition-clone/compose.yaml b/tests/fk-partition-clone/compose.yaml new file mode 100644 index 000000000..cfebc6da2 --- /dev/null +++ b/tests/fk-partition-clone/compose.yaml @@ -0,0 +1,42 @@ +services: + source: + image: postgres:${PGVERSION:-16} + expose: + - 5432 + environment: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: h4ckm3 + POSTGRES_HOST_AUTH_METHOD: trust + command: > + -c ssl=on + -c ssl_cert_file=/etc/ssl/certs/ssl-cert-snakeoil.pem + -c ssl_key_file=/etc/ssl/private/ssl-cert-snakeoil.key + target: + image: postgres:${PGVERSION:-16} + expose: + - 5432 + environment: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: h4ckm3 + POSTGRES_HOST_AUTH_METHOD: trust + command: > + -c ssl=on + -c ssl_cert_file=/etc/ssl/certs/ssl-cert-snakeoil.pem + -c ssl_key_file=/etc/ssl/private/ssl-cert-snakeoil.key + test: + build: + context: . + dockerfile: Dockerfile + cap_add: + - SYS_ADMIN + - SYS_PTRACE + environment: + PGSSLMODE: "require" + PGCOPYDB_SOURCE_PGURI: postgres://postgres:h4ckm3@source/postgres + PGCOPYDB_TARGET_PGURI: postgres://postgres:h4ckm3@target/postgres + PGCOPYDB_TABLE_JOBS: 4 + PGCOPYDB_INDEX_JOBS: 4 + PGCOPYDB_FAIL_FAST: "true" + depends_on: + - source + - target diff --git a/tests/fk-partition-clone/copydb.sh b/tests/fk-partition-clone/copydb.sh new file mode 100755 index 000000000..beb55e562 --- /dev/null +++ b/tests/fk-partition-clone/copydb.sh @@ -0,0 +1,159 @@ +#! /bin/bash + +set -x +set -e + +# Disable pager for psql to avoid hanging in non-interactive environments +export PAGER=cat + +# This script expects the following environment variables to be set: +# +# - PGCOPYDB_SOURCE_PGURI +# - PGCOPYDB_TARGET_PGURI +# - PGCOPYDB_TABLE_JOBS +# - PGCOPYDB_INDEX_JOBS + +# make sure source and target databases are ready +pgcopydb ping + +# +# A HASH-partitioned table whose FKs reference regular tables. Postgres clones +# each FK per partition; before the fix pgcopydb listed those clones and tried +# to re-create them after the cascade already had, flooding the log with +# "constraint already exists". This asserts a clean, correct clone. +# +psql -d ${PGCOPYDB_SOURCE_PGURI} <<'SQL' + +CREATE TABLE ref_company (id uuid PRIMARY KEY); +CREATE TABLE ref_message (id uuid PRIMARY KEY); +CREATE TABLE ref_snapshot (id uuid PRIMARY KEY); + +CREATE TABLE hash_recipients ( + company_id uuid NOT NULL, + message_id uuid NOT NULL, + recipient_key varchar NOT NULL, + snapshot_id uuid NOT NULL, + tag_ids uuid[] DEFAULT '{}'::uuid[] NOT NULL, + created_at timestamp NOT NULL, + updated_at timestamp NOT NULL, + CONSTRAINT hash_recipients_pkey + PRIMARY KEY (company_id, message_id, recipient_key), + CONSTRAINT hash_recipients_snapshot_fkey + FOREIGN KEY (snapshot_id) + REFERENCES ref_snapshot(id) ON DELETE SET NULL, + CONSTRAINT hash_recipients_company_fkey + FOREIGN KEY (company_id) REFERENCES ref_company(id), + CONSTRAINT hash_recipients_message_fkey + FOREIGN KEY (message_id) REFERENCES ref_message(id) +) +PARTITION BY HASH (company_id); + +CREATE TABLE hash_recipients_p0 PARTITION OF hash_recipients + FOR VALUES WITH (MODULUS 3, REMAINDER 0); +CREATE TABLE hash_recipients_p1 PARTITION OF hash_recipients + FOR VALUES WITH (MODULUS 3, REMAINDER 1); +CREATE TABLE hash_recipients_p2 PARTITION OF hash_recipients + FOR VALUES WITH (MODULUS 3, REMAINDER 2); + +-- Partitioned-index parents (ON ONLY), which also exercise the index path. +CREATE INDEX index_hash_recipients_company ON ONLY hash_recipients (company_id); +CREATE INDEX index_hash_recipients_message ON ONLY hash_recipients (message_id); + +INSERT INTO ref_company SELECT gen_random_uuid() FROM generate_series(1, 5); +INSERT INTO ref_message SELECT gen_random_uuid() FROM generate_series(1, 5); +INSERT INTO ref_snapshot SELECT gen_random_uuid() FROM generate_series(1, 5); + +INSERT INTO hash_recipients + (company_id, message_id, recipient_key, snapshot_id, created_at, updated_at) +SELECT c.id, m.id, 'k' || row_number() OVER () || '@example.com', s.id, + now(), now() + FROM ref_company c, ref_message m, ref_snapshot s + LIMIT 20; + +SQL + +src_rows=$(psql -AtX -d ${PGCOPYDB_SOURCE_PGURI} \ + -c "SELECT count(*) FROM hash_recipients") +echo "=== Source hash_recipients rows: ${src_rows} (expect 20) ===" + +if [ "${src_rows}" != "20" ]; then + echo "ERROR: source data setup is wrong" + exit 1 +fi + +# Plain clone, capturing output to check for spurious FK errors. +clonelog=$(mktemp) + +set +e +pgcopydb clone --notice 2>&1 | tee "${clonelog}" +clone_rc=${PIPESTATUS[0]} +set -e + +echo "" +echo "=== Clone finished (exit ${clone_rc}), verifying results ===" +echo "" + +if [ "${clone_rc}" != "0" ]; then + echo "ERROR: pgcopydb clone failed (exit ${clone_rc})" + exit 1 +fi + +# Regression guard: no FK clone should be re-created. Before the fix this +# printed one "constraint ... already exists" per clone (9 here); after, none. +already=$(grep -cE 'constraint .* already exists' "${clonelog}" || true) +echo "FK 'already exists' error lines: ${already} (expect 0)" + +if [ "${already}" != "0" ]; then + echo "ERROR: partition FK clones were listed and collided on recreate;" + echo " the conparentid filter is not being applied" + exit 1 +fi + +# The target must carry the 3 top-level FKs plus the cascaded per-partition +# clones, all validated. +tgt_parent_fks=$(psql -AtX -d ${PGCOPYDB_TARGET_PGURI} -c \ + "SELECT count(*) FROM pg_constraint + WHERE contype = 'f' AND conparentid = 0 + AND conrelid = 'hash_recipients'::regclass") + +tgt_clone_fks=$(psql -AtX -d ${PGCOPYDB_TARGET_PGURI} -c \ + "SELECT count(*) FROM pg_constraint + WHERE contype = 'f' AND conparentid <> 0") + +tgt_unvalidated=$(psql -AtX -d ${PGCOPYDB_TARGET_PGURI} -c \ + "SELECT count(*) FROM pg_constraint + WHERE contype = 'f' AND NOT convalidated") + +echo "Target top-level FKs: ${tgt_parent_fks} (expect 3)" +echo "Target per-partition FK clones: ${tgt_clone_fks} (expect 9)" +echo "Target unvalidated FKs: ${tgt_unvalidated} (expect 0)" + +if [ "${tgt_parent_fks}" != "3" ]; then + echo "ERROR: expected 3 top-level FK constraints on the partitioned table" + exit 1 +fi + +if [ "${tgt_clone_fks}" != "9" ]; then + echo "ERROR: expected 9 cascaded per-partition FK clones (3 FKs x 3 parts)" + exit 1 +fi + +if [ "${tgt_unvalidated}" != "0" ]; then + echo "ERROR: all FK constraints should be validated after a plain clone" + exit 1 +fi + +# +# Data must be copied in full and remain writable through the FKs. +# +tgt_rows=$(psql -AtX -d ${PGCOPYDB_TARGET_PGURI} \ + -c "SELECT count(*) FROM hash_recipients") +echo "Target hash_recipients rows: ${tgt_rows} (expect 20)" + +if [ "${tgt_rows}" != "20" ]; then + echo "ERROR: partitioned table data was not fully copied" + exit 1 +fi + +echo "" +echo "=== fk-partition-clone test passed ==="