PostgreSQL 15.10 update corrective action for ATTACH PARTITION/DETACH PARTITION

On 11/26/24 01:59, Paul Foerster wrote:

Hi,

I have a question regarding the recent security update for PostgreSQL 15.

We have a gitlab database. It used to run on the PostgreSQL 15.8 software. I updated from 15.8 to 15.10 and executed the corrective actions as outlined in:

https://www.postgresql.org/about/news/postgresql-171-165-159-1414-1317-and-1221-released-2955/

I executed "SELECT conrelid::pg_catalog.regclass AS "constrained table", conname AS constraint, confrelid::pg_catalog.regclass AS "references", pg_catalog.format('ALTER TABLE %s DROP CONSTRAINT %I;', conrelid::pg_catalog.regclass, conname) AS "drop", pg_catalog.format('ALTER TABLE %s ADD CONSTRAINT %I %s;', conrelid::pg_catalog.regclass, conname, pg_catalog.pg_get_constraintdef(oid)) AS "add" FROM pg_catalog.pg_constraint c WHERE contype = 'f' AND conparentid = 0 AND (SELECT count(*) FROM pg_catalog.pg_constraint c2 WHERE c2.conparentid = c.oid) <> (SELECT count(*) FROM pg_catalog.pg_inherits i WHERE (i.inhparent = c.conrelid OR i.inhparent = c.confrelid) AND EXISTS (SELECT 1 FROM pg_catalog.pg_partitioned_table WHERE partrelid = i.inhparent));" which gave the result below:

-[ RECORD 1 ]-----+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
constrained table | p_ci_pipelines
constraint | fk_262d4c2d19_p
references | p_ci_pipelines
drop | alter table p_ci_pipelines drop constraint fk_262d4c2d19_p;
add | alter table p_ci_pipelines add constraint fk_262d4c2d19_p FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL;

I then executed the two alter table statements without any problem. No error was reported and all seems ok.

Now, if I execute the query to find the constraints again, I would expect the result to be empty. But it is not.

Did you commit the statements?

Are you using concurrent sessions to do this?

When you run the query again do you get the same two statements?

Why is that and what am I supposed to do? Is the problem fixed now or is it still pending? Any ideas would be greatly appreciated.

Cheers
Paul

--
Adrian Klaver
adrian.klaver@aklaver.com

Hi Adrian,

On 26 Nov 2024, at 17:56, Adrian Klaver <adrian.klaver@aklaver.com> wrote:

Did you commit the statements?

Yes. I have autocommit on, the psql default.

Are you using concurrent sessions to do this?

No. I do this in one session. 1. select, 2. drop, 3. add, 4. select.

When you run the query again do you get the same two statements?

Yes. I can repeat the above 4 steps as much as I want. The result remains the same. I would have expected to have an empty result doing the final repeated select, but it shows exactly the same output.

Cheers,
Paul

Paul Foerster <paul.foerster@gmail.com> writes:

On 26 Nov 2024, at 17:56, Adrian Klaver <adrian.klaver@aklaver.com> wrote:
When you run the query again do you get the same two statements?

Yes. I can repeat the above 4 steps as much as I want. The result remains the same. I would have expected to have an empty result doing the final repeated select, but it shows exactly the same output.

I would have expected an empty result too. Can you confirm that
p_ci_pipelines used to be a partition of something? Can you show us
the full DDL (or psql \d+ output) for the partitioned table it
used to be part of, and for that matter also for p_ci_pipelines?
Did the FK used to reference the whole partitioned table, or just
this partition?

I'm suspicious that our repair recipe might not have accounted
for self-reference FKs fully, but that's just a gut feeling at
this point.

regards, tom lane

Hi Tom,

On 26 Nov 2024, at 22:25, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I would have expected an empty result too. Can you confirm that
p_ci_pipelines used to be a partition of something? Can you show us
the full DDL (or psql \d+ output) for the partitioned table it
used to be part of, and for that matter also for p_ci_pipelines?
Did the FK used to reference the whole partitioned table, or just
this partition?

I'm suspicious that our repair recipe might not have accounted
for self-reference FKs fully, but that's just a gut feeling at
this point.

Of course, it contains no secret data. Please find the full log below. According to the add constraint statement, it is a self reference.

Thanks for looking into it.

Cheers,
Paul

gitxp1t=# \set
AUTOCOMMIT = 'on'
...
VERSION = 'PostgreSQL 15.10 on x86_64-pc-linux-gnu, compiled by gcc (SUSE Linux) 7.5.0, 64-bit'
...

gitxp1t=# SELECT conrelid::pg_catalog.regclass AS "constrained table",
gitxp1t-# conname AS constraint,
gitxp1t-# confrelid::pg_catalog.regclass AS "references",
gitxp1t-# pg_catalog.format('ALTER TABLE %s DROP CONSTRAINT %I;',
gitxp1t(# conrelid::pg_catalog.regclass, conname) AS "drop",
gitxp1t-# pg_catalog.format('ALTER TABLE %s ADD CONSTRAINT %I %s;',
gitxp1t(# conrelid::pg_catalog.regclass, conname,
gitxp1t(# pg_catalog.pg_get_constraintdef(oid)) AS "add"
gitxp1t-# FROM pg_catalog.pg_constraint c
gitxp1t-# WHERE contype = 'f' AND conparentid = 0 AND
gitxp1t-# (SELECT count(*) FROM pg_catalog.pg_constraint c2
gitxp1t(# WHERE c2.conparentid = c.oid) <>
gitxp1t-# (SELECT count(*) FROM pg_catalog.pg_inherits i
gitxp1t(# WHERE (i.inhparent = c.conrelid OR i.inhparent = c.confrelid) AND
gitxp1t(# EXISTS (SELECT 1 FROM pg_catalog.pg_partitioned_table
gitxp1t(# WHERE partrelid = i.inhparent));
constrained table | constraint | references | drop | add -------------------+-----------------+----------------+-------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
p_ci_pipelines | fk_262d4c2d19_p | p_ci_pipelines | ALTER TABLE p_ci_pipelines DROP CONSTRAINT fk_262d4c2d19_p; | ALTER TABLE p_ci_pipelines ADD CONSTRAINT fk_262d4c2d19_p FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL;
(1 row)
gitxp1t=# ALTER TABLE p_ci_pipelines DROP CONSTRAINT fk_262d4c2d19_p;
ALTER TABLE
gitxp1t=# ALTER TABLE p_ci_pipelines ADD CONSTRAINT fk_262d4c2d19_p FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL;
ALTER TABLE
gitxp1t=# SELECT conrelid::pg_catalog.regclass AS "constrained table",
gitxp1t-# conname AS constraint,
gitxp1t-# confrelid::pg_catalog.regclass AS "references",
gitxp1t-# pg_catalog.format('ALTER TABLE %s DROP CONSTRAINT %I;',
gitxp1t(# conrelid::pg_catalog.regclass, conname) AS "drop",
gitxp1t-# pg_catalog.format('ALTER TABLE %s ADD CONSTRAINT %I %s;',
gitxp1t(# conrelid::pg_catalog.regclass, conname,
gitxp1t(# pg_catalog.pg_get_constraintdef(oid)) AS "add"
gitxp1t-# FROM pg_catalog.pg_constraint c
gitxp1t-# WHERE contype = 'f' AND conparentid = 0 AND
gitxp1t-# (SELECT count(*) FROM pg_catalog.pg_constraint c2
gitxp1t(# WHERE c2.conparentid = c.oid) <>
gitxp1t-# (SELECT count(*) FROM pg_catalog.pg_inherits i
gitxp1t(# WHERE (i.inhparent = c.conrelid OR i.inhparent = c.confrelid) AND
gitxp1t(# EXISTS (SELECT 1 FROM pg_catalog.pg_partitioned_table
gitxp1t(# WHERE partrelid = i.inhparent));
constrained table | constraint | references | drop | add -------------------+-----------------+----------------+-------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
p_ci_pipelines | fk_262d4c2d19_p | p_ci_pipelines | ALTER TABLE p_ci_pipelines DROP CONSTRAINT fk_262d4c2d19_p; | ALTER TABLE p_ci_pipelines ADD CONSTRAINT fk_262d4c2d19_p FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL;
(1 row)

gitxp1t=# \d+ p_ci_pipelines
Partitioned table "public.p_ci_pipelines"
Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description
-------------------------------+-----------------------------+-----------+----------+------------------------------------------+----------+-------------+--------------+-------------
ref | character varying | | | | extended | | |
sha | character varying | | | | extended | | |
before_sha | character varying | | | | extended | | |
created_at | timestamp without time zone | | | | plain | | |
updated_at | timestamp without time zone | | | | plain | | |
tag | boolean | | | false | plain | | |
yaml_errors | text | | | | extended | | |
committed_at | timestamp without time zone | | | | plain | | |
project_id | integer | | | | plain | | |
status | character varying | | | | extended | | |
started_at | timestamp without time zone | | | | plain | | |
finished_at | timestamp without time zone | | | | plain | | |
duration | integer | | | | plain | | |
user_id | integer | | | | plain | | |
lock_version | integer | | | 0 | plain | | |
pipeline_schedule_id | integer | | | | plain | | |
source | integer | | | | plain | | |
config_source | integer | | | | plain | | |
protected | boolean | | | | plain | | |
failure_reason | integer | | | | plain | | |
iid | integer | | | | plain | | |
merge_request_id | integer | | | | plain | | |
source_sha | bytea | | | | extended | | |
target_sha | bytea | | | | extended | | |
external_pull_request_id | bigint | | | | plain | | |
ci_ref_id | bigint | | | | plain | | |
locked | smallint | | not null | 1 | plain | | |
partition_id | bigint | | not null | | plain | | |
id | bigint | | not null | nextval('ci_pipelines_id_seq'::regclass) | plain | | |
auto_canceled_by_id | bigint | | | | plain | | |
auto_canceled_by_partition_id | bigint | | | | plain | | |
Partition key: LIST (partition_id)
Indexes:
"p_ci_pipelines_pkey" PRIMARY KEY, btree (id, partition_id)
"p_ci_pipelines_auto_canceled_by_id_idx" btree (auto_canceled_by_id)
"p_ci_pipelines_ci_ref_id_id_idx" btree (ci_ref_id, id) WHERE locked = 1
"p_ci_pipelines_ci_ref_id_id_source_status_idx" btree (ci_ref_id, id DESC, source, status) WHERE ci_ref_id IS NOT NULL
"p_ci_pipelines_external_pull_request_id_idx" btree (external_pull_request_id) WHERE external_pull_request_id IS NOT NULL
"p_ci_pipelines_id_idx" btree (id) WHERE source = 13
"p_ci_pipelines_merge_request_id_idx" btree (merge_request_id) WHERE merge_request_id IS NOT NULL
"p_ci_pipelines_pipeline_schedule_id_id_idx" btree (pipeline_schedule_id, id)
"p_ci_pipelines_project_id_id_idx" btree (project_id, id DESC)
"p_ci_pipelines_project_id_iid_partition_id_idx" UNIQUE, btree (project_id, iid, partition_id) WHERE iid IS NOT NULL
"p_ci_pipelines_project_id_ref_id_idx" btree (project_id, ref, id DESC)
"p_ci_pipelines_project_id_ref_status_id_idx" btree (project_id, ref, status, id)
"p_ci_pipelines_project_id_sha_idx" btree (project_id, sha)
"p_ci_pipelines_project_id_source_idx" btree (project_id, source)
"p_ci_pipelines_project_id_status_config_source_idx" btree (project_id, status, config_source)
"p_ci_pipelines_project_id_status_created_at_idx" btree (project_id, status, created_at)
"p_ci_pipelines_project_id_status_updated_at_idx" btree (project_id, status, updated_at)
"p_ci_pipelines_project_id_user_id_status_ref_idx" btree (project_id, user_id, status, ref) WHERE source <> 12
"p_ci_pipelines_status_id_idx" btree (status, id)
"p_ci_pipelines_user_id_created_at_config_source_idx" btree (user_id, created_at, config_source)
"p_ci_pipelines_user_id_created_at_source_idx" btree (user_id, created_at, source)
"p_ci_pipelines_user_id_id_idx" btree (user_id, id) WHERE status::text = ANY (ARRAY['running'::character varying::text, 'waiting_for_resource'::character varying::text, 'preparing'::character varying::text, 'pending'::character varying::text, 'created'::character varying::text, 'scheduled'::character varying::text])
"p_ci_pipelines_user_id_id_idx1" btree (user_id, id DESC) WHERE failure_reason = 3
Check constraints:
"check_2ba2a044b9" CHECK (project_id IS NOT NULL)
Foreign-key constraints:
"fk_190998ef09" FOREIGN KEY (external_pull_request_id) REFERENCES external_pull_requests(id) ON DELETE SET NULL
"fk_262d4c2d19_p" FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL
"fk_3d34ab2e06" FOREIGN KEY (pipeline_schedule_id) REFERENCES ci_pipeline_schedules(id) ON DELETE SET NULL
"fk_d80e161c54" FOREIGN KEY (ci_ref_id) REFERENCES ci_refs(id) ON DELETE SET NULL
Referenced by:
TABLE "p_ci_pipelines" CONSTRAINT "fk_262d4c2d19_p" FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL
TABLE "ci_pipeline_chat_data" CONSTRAINT "fk_64ebfab6b3_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "p_ci_builds" CONSTRAINT "fk_87f4cefcda_p" FOREIGN KEY (upstream_pipeline_partition_id, upstream_pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "p_ci_builds" CONSTRAINT "fk_a2141b1522_p" FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL
TABLE "p_ci_builds" CONSTRAINT "fk_d3130c9a7f_p" FOREIGN KEY (partition_id, commit_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "ci_sources_pipelines" CONSTRAINT "fk_d4e29af7d7_p" FOREIGN KEY (source_partition_id, source_pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "ci_sources_pipelines" CONSTRAINT "fk_e1bad85861_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "p_ci_pipeline_variables" CONSTRAINT "fk_f29c5f4380_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "p_ci_stages" CONSTRAINT "fk_fb57e6cc56_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "ci_sources_projects" CONSTRAINT "fk_rails_10a1eb379a_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "gitlab_partitions_dynamic.ci_builds_101" CONSTRAINT "fk_rails_4540ead625_p" FOREIGN KEY (upstream_pipeline_partition_id, upstream_pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "gitlab_partitions_dynamic.ci_builds_102" CONSTRAINT "fk_rails_4540ead625_p" FOREIGN KEY (upstream_pipeline_partition_id, upstream_pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "ci_builds" CONSTRAINT "fk_rails_4540ead625_p" FOREIGN KEY (upstream_pipeline_partition_id, upstream_pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "gitlab_partitions_dynamic.ci_builds_101" CONSTRAINT "fk_rails_494e57ee78_p" FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL NOT VALID
TABLE "ci_builds" CONSTRAINT "fk_rails_494e57ee78_p" FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL NOT VALID
TABLE "gitlab_partitions_dynamic.ci_builds_102" CONSTRAINT "fk_rails_494e57ee78_p" FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL NOT VALID
TABLE "ci_pipeline_variables" CONSTRAINT "fk_rails_507416c33a_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "gitlab_partitions_dynamic.ci_pipeline_variables_102" CONSTRAINT "fk_rails_507416c33a_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "ci_pipeline_metadata" CONSTRAINT "fk_rails_50c1e9ea10_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "gitlab_partitions_dynamic.ci_stages_102" CONSTRAINT "fk_rails_5d4d96d44b_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "ci_stages" CONSTRAINT "fk_rails_5d4d96d44b_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "ci_pipeline_messages" CONSTRAINT "fk_rails_8d3b04e3e1_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "p_ci_pipelines_config" CONSTRAINT "fk_rails_906c9a2533_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "ci_pipeline_artifacts" CONSTRAINT "fk_rails_a9e811a466_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "p_ci_builds_execution_configs" CONSTRAINT "fk_rails_c26408d02c_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
TABLE "ci_builds" CONSTRAINT "fk_rails_d739f46384_p" FOREIGN KEY (partition_id, commit_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "gitlab_partitions_dynamic.ci_builds_101" CONSTRAINT "fk_rails_d739f46384_p" FOREIGN KEY (partition_id, commit_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "gitlab_partitions_dynamic.ci_builds_102" CONSTRAINT "fk_rails_d739f46384_p" FOREIGN KEY (partition_id, commit_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "gitlab_partitions_dynamic.ci_builds_execution_configs_102" CONSTRAINT "fk_rails_e214655a86_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "gitlab_partitions_dynamic.ci_builds_execution_configs_100" CONSTRAINT "fk_rails_e214655a86_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "gitlab_partitions_dynamic.ci_builds_execution_configs_101" CONSTRAINT "fk_rails_e214655a86_p" FOREIGN KEY (partition_id, pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID
TABLE "ci_daily_build_group_report_results" CONSTRAINT "fk_rails_ee072d13b3_p" FOREIGN KEY (partition_id, last_pipeline_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE
Triggers:
p_ci_pipelines_loose_fk_trigger AFTER DELETE ON p_ci_pipelines REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records()
Partitions: ci_pipelines FOR VALUES IN ('100', '101', '102')

Paul Foerster <paul.foerster@gmail.com> writes:

On 26 Nov 2024, at 22:25, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm suspicious that our repair recipe might not have accounted
for self-reference FKs fully, but that's just a gut feeling at
this point.

Of course, it contains no secret data. Please find the full log below. According to the add constraint statement, it is a self reference.
Thanks for looking into it.

Okay, so I was able to reproduce this from scratch on HEAD:

regression=# create table p_ci_pipelines(partition_id int, id int, primary key(partition_id,id), auto_canceled_by_partition_id int, auto_canceled_by_id int) partition by LIST (partition_id);
CREATE TABLE
regression=# create table ci_pipelines partition of p_ci_pipelines FOR VALUES IN ('100', '101', '102');
CREATE TABLE
regression=# ALTER TABLE p_ci_pipelines ADD CONSTRAINT fk_262d4c2d19_p FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL;
ALTER TABLE
regression=# SELECT conrelid::pg_catalog.regclass AS "constrained table",
conname AS constraint,
confrelid::pg_catalog.regclass AS "references",
pg_catalog.format('ALTER TABLE %s DROP CONSTRAINT %I;',
conrelid::pg_catalog.regclass, conname) AS "drop",
pg_catalog.format('ALTER TABLE %s ADD CONSTRAINT %I %s;',
conrelid::pg_catalog.regclass, conname,
pg_catalog.pg_get_constraintdef(oid)) AS "add"
FROM pg_catalog.pg_constraint c
WHERE contype = 'f' AND conparentid = 0 AND
(SELECT count(*) FROM pg_catalog.pg_constraint c2
WHERE c2.conparentid = c.oid) <>
(SELECT count(*) FROM pg_catalog.pg_inherits i
WHERE (i.inhparent = c.conrelid OR i.inhparent = c.confrelid) AND
EXISTS (SELECT 1 FROM pg_catalog.pg_partitioned_table
WHERE partrelid = i.inhparent));
constrained table | constraint | references | drop | add
-------------------+-----------------+----------------+-------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
p_ci_pipelines | fk_262d4c2d19_p | p_ci_pipelines | ALTER TABLE p_ci_pipelines DROP CONSTRAINT fk_262d4c2d19_p; | ALTER TABLE p_ci_pipelines ADD CONSTRAINT fk_262d4c2d19_p FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL;
(1 row)

I doubt that there's anything actually wrong with the catalog state at
this point (perhaps Alvaro would confirm that). That leads to the
conclusion that what's wrong is the release notes' query for fingering
broken constraints, and it needs some additional test to avoid
complaining about (I suspect) self-reference cases.

regards, tom lane

On 2024-Nov-27, Tom Lane wrote:

I doubt that there's anything actually wrong with the catalog state at
this point (perhaps Alvaro would confirm that). That leads to the
conclusion that what's wrong is the release notes' query for fingering
broken constraints, and it needs some additional test to avoid
complaining about (I suspect) self-reference cases.

Ugh, I hadn't noticed this report, thanks for CCing me. I'll have a
look at this tomorrow. You're right that I didn't think of checking
self-referencing FKs with the query.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
<Schwern> It does it in a really, really complicated way
<crab> why does it need to be complicated?
<Schwern> Because it's MakeMaker.

Hi Tom, hi Alvaro,

On 27 Nov 2024, at 19:52, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Okay, so I was able to reproduce this from scratch on HEAD:

great, thanks.

I doubt that there's anything actually wrong with the catalog state at
this point (perhaps Alvaro would confirm that). That leads to the
conclusion that what's wrong is the release notes' query for fingering
broken constraints, and it needs some additional test to avoid
complaining about (I suspect) self-reference cases.

In the meantime, I updated the whole company. The one test database actually was the only database that this was returned. I found no other occurrences.

As I understand it, the worst thing that could happen is that one or more rows end up in a detached partition table which should actually be in another partition, right? Since there were no rows, no harm could have been done. Also, since this is a self reference, the wrong table is also the right one.

Again, thanks very much for clarifying this.

Cheers
Paul

On 2024-Nov-27, Tom Lane wrote:

I doubt that there's anything actually wrong with the catalog state at
this point (perhaps Alvaro would confirm that). That leads to the
conclusion that what's wrong is the release notes' query for fingering
broken constraints, and it needs some additional test to avoid
complaining about (I suspect) self-reference cases.

Yes, I think the catalog state is correct and the release notes query is
wrong. I propose a repaired version below. But first, I think there's
still a problem specific to partition creation when a self-referencing
FKs exists. If you do create table / create partition / add FK, then
the query from the release notes does report the FK. But if you do
create table / add FK / create partition, nothing is reported. Clearly,
both those things cannot be simultaneously correct.

-- Case 1: create the partition when the FK already exists
drop table if exists p_ci_pipelines;
create table p_ci_pipelines(partition_id int, id int, primary key(partition_id,id), auto_canceled_by_partition_id int, auto_canceled_by_id int) partition by LIST (partition_id);
ALTER TABLE p_ci_pipelines ADD CONSTRAINT fk_262d4c2d19_p FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines (partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL;
create table ci_pipelines partition of p_ci_pipelines FOR VALUES IN ('100', '101', '102');

-- Case 2: create both tables, then add the FK
drop table if exists p_ci_pipelines;
create table p_ci_pipelines(partition_id int, id int, primary key(partition_id,id), auto_canceled_by_partition_id int, auto_canceled_by_id int) partition by LIST (partition_id);
create table ci_pipelines partition of p_ci_pipelines FOR VALUES IN ('100', '101', '102');
ALTER TABLE p_ci_pipelines ADD CONSTRAINT fk_262d4c2d19_p FOREIGN KEY (auto_canceled_by_partition_id, auto_canceled_by_id) REFERENCES p_ci_pipelines (partition_id, id) ON UPDATE CASCADE ON DELETE SET NULL;

Naturally, if in any of those situations you drop and recreate the FK,
it degenerates to case 2, so if you do what the release notes say, it'll
continue to report the FK.

We can use the following query (which lists the constraint and its derivate
pg_constraint rows) to see what goes wrong:

WITH RECURSIVE arrh AS (
SELECT oid, conrelid, conname, confrelid, NULL::name AS conparent
FROM pg_constraint
WHERE connamespace = 'public'::regnamespace AND
contype = 'f' AND conparentid = 0
UNION ALL
SELECT c.oid, c.conrelid, c.conname, c.confrelid,
(pg_identify_object('pg_constraint'::regclass, arrh.oid, 0)).identity
FROM pg_constraint c
JOIN arrh ON c.conparentid = arrh.oid
) SELECT conrelid::regclass, conname, confrelid::regclass, conparent
FROM arrh
ORDER BY conrelid::regclass::text, conname;

For case 2, this is the result:

conrelid │ conname │ confrelid │ conparent
────────────────┼─────────────────────────────────────────────────────────────────┼────────────────┼──────────────────────────────────────────
ci_pipelines │ fk_262d4c2d19_p │ p_ci_pipelines │ fk_262d4c2d19_p on public.p_ci_pipelines
p_ci_pipelines │ fk_262d4c2d19_p │ p_ci_pipelines │
p_ci_pipelines │ p_ci_pipelines_auto_canceled_by_partition_id_auto_canceled_fkey │ ci_pipelines │ fk_262d4c2d19_p on public.p_ci_pipelines

For case 1, where the release notes query reports nothing, we get the
following list of constraints instead:

conrelid │ conname │ confrelid │ conparent
────────────────┼─────────────────┼────────────────┼──────────────────────────────────────────
ci_pipelines │ fk_262d4c2d19_p │ p_ci_pipelines │ fk_262d4c2d19_p on public.p_ci_pipelines
p_ci_pipelines │ fk_262d4c2d19_p │ p_ci_pipelines │
(2 filas)

Let's look at the triggers. For case 1 we have the following triggers:

WITH RECURSIVE arrh AS (
SELECT t.oid, t.tgrelid::regclass as tablename, tgname,
t.tgfoid::regproc as trigfn,
(pg_identify_object('pg_constraint'::regclass, c.oid, 0)).identity as constr,
NULL::bool as samefunc,
NULL::name AS parent
FROM pg_trigger t
LEFT JOIN pg_constraint c ON c.oid = t.tgconstraint
WHERE (SELECT relnamespace FROM pg_class WHERE oid = t.tgrelid) = 'public'::regnamespace
AND c.contype = 'f' AND t.tgparentid = 0
UNION ALL
SELECT t2.oid, t2.tgrelid::regclass as tablename, t2.tgname,
t2.tgfoid::regproc as trigfn,
(pg_identify_object('pg_constraint'::regclass, c2.oid, 0)).identity,
arrh.trigfn = t2.tgfoid as samefunc,
replace((pg_identify_object('pg_trigger'::regclass, t2.tgparentid, 0)).identity,
t2.tgparentid::text, 'TGOID')
FROM pg_trigger t2
LEFT JOIN pg_constraint c2 ON c2.oid = t2.tgconstraint
JOIN arrh ON t2.tgparentid = arrh.oid
) SELECT tgname, tablename, constr, samefunc, parent
FROM arrh
ORDER BY tablename::text, constr;

tgname │ tablename │ constr │ samefunc │ parent
──────────────────────────────┼────────────────┼──────────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────
RI_ConstraintTrigger_c_16659 │ ci_pipelines │ fk_262d4c2d19_p on public.ci_pipelines │ t │ "RI_ConstraintTrigger_c_TGOID" on public.p_ci_pipelines
RI_ConstraintTrigger_c_16658 │ ci_pipelines │ fk_262d4c2d19_p on public.ci_pipelines │ t │ "RI_ConstraintTrigger_c_TGOID" on public.p_ci_pipelines
RI_ConstraintTrigger_a_16648 │ p_ci_pipelines │ fk_262d4c2d19_p on public.p_ci_pipelines │ │
RI_ConstraintTrigger_a_16649 │ p_ci_pipelines │ fk_262d4c2d19_p on public.p_ci_pipelines │ │
RI_ConstraintTrigger_c_16650 │ p_ci_pipelines │ fk_262d4c2d19_p on public.p_ci_pipelines │ │
RI_ConstraintTrigger_c_16651 │ p_ci_pipelines │ fk_262d4c2d19_p on public.p_ci_pipelines │ │
(6 filas)

For case 2 we have this:

tgname │ tablename │ constr │ samefunc │ parent
──────────────────────────────┼────────────────┼──────────────────────────────────────────────────────────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────
RI_ConstraintTrigger_c_16680 │ ci_pipelines │ fk_262d4c2d19_p on public.ci_pipelines │ t │ "RI_ConstraintTrigger_c_TGOID" on public.p_ci_pipelines
RI_ConstraintTrigger_c_16679 │ ci_pipelines │ fk_262d4c2d19_p on public.ci_pipelines │ t │ "RI_ConstraintTrigger_c_TGOID" on public.p_ci_pipelines
RI_ConstraintTrigger_a_16675 │ ci_pipelines │ p_ci_pipelines_auto_canceled_by_partition_id_auto_canceled_fkey on public.p_ci_pipelines │ t │ "RI_ConstraintTrigger_a_TGOID" on public.p_ci_pipelines
RI_ConstraintTrigger_a_16674 │ ci_pipelines │ p_ci_pipelines_auto_canceled_by_partition_id_auto_canceled_fkey on public.p_ci_pipelines │ t │ "RI_ConstraintTrigger_a_TGOID" on public.p_ci_pipelines
RI_ConstraintTrigger_a_16671 │ p_ci_pipelines │ fk_262d4c2d19_p on public.p_ci_pipelines │ │
RI_ConstraintTrigger_a_16672 │ p_ci_pipelines │ fk_262d4c2d19_p on public.p_ci_pipelines │ │
RI_ConstraintTrigger_c_16676 │ p_ci_pipelines │ fk_262d4c2d19_p on public.p_ci_pipelines │ │
RI_ConstraintTrigger_c_16677 │ p_ci_pipelines │ fk_262d4c2d19_p on public.p_ci_pipelines │ │
(8 filas)

Here, the difference is the two action triggers on the partition, which
hang under the secondary constraint for the partition. And those are
critical, because without them, the ON DELETE clause is not executed:

insert into ci_pipelines values (100, 1, null, null); -- create referenced row
insert into ci_pipelines values (101, 2, 100, 1); -- create the reference
delete from ci_pipelines where id = 1; -- should SET NULL but doesn't
select * from p_ci_pipelines ;
partition_id │ id │ auto_canceled_by_partition_id │ auto_canceled_by_id
──────────────┼────┼───────────────────────────────┼─────────────────────
101 │ 2 │ 100 │ 1
(1 fila)

(Obviously if we drop the constraint at this point and try to recreate,
it'll complain that the referenced row doesn't exist).

This doesn't happen with the tables defined as case 2; the FK columns
are set to NULL, as intended.

partition_id │ id │ auto_canceled_by_partition_id │ auto_canceled_by_id
──────────────┼────┼───────────────────────────────┼─────────────────────
101 │ 2 │ │
(1 fila)

This all was to say that the query in the release notes is undoubtedly
wrong. After thinking some more about it, I think the fix is to add 1
to the number of constraints:

SELECT conrelid::pg_catalog.regclass AS "constrained table",
conname AS constraint,
confrelid::pg_catalog.regclass AS "references",
pg_catalog.format('ALTER TABLE %s DROP CONSTRAINT %I;',
conrelid::pg_catalog.regclass, conname) AS "drop",
pg_catalog.format('ALTER TABLE %s ADD CONSTRAINT %I %s;',
conrelid::pg_catalog.regclass, conname,
pg_catalog.pg_get_constraintdef(oid)) AS "add"
FROM pg_catalog.pg_constraint c
WHERE contype = 'f' AND conparentid = 0 AND
(SELECT count(*) FROM pg_catalog.pg_constraint c2
WHERE c2.conparentid = c.oid) <>
((SELECT count(*) FROM pg_catalog.pg_inherits i
WHERE (i.inhparent = c.conrelid OR i.inhparent = c.confrelid) AND
EXISTS (SELECT 1 FROM pg_catalog.pg_partitioned_table
WHERE partrelid = i.inhparent)) +
CASE when pg_partition_root(conrelid) = confrelid THEN 1 ELSE 0 END);

This reports case 2 as OK and case 1 as bogus, as should be. I tried
adding more partitions and this seems to hold correctly. I was afraid
though that this would fail if we create an FK in an intermediate level
of the partition hierarchy ... but experimentation doesn't seem to give
that result. I've run out of time today to continue to look though.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"La vida es para el que se aventura"

Hi Alvaro,

On 29 Nov 2024, at 18:15, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

This all was to say that the query in the release notes is undoubtedly
wrong. After thinking some more about it, I think the fix is to add 1
to the number of constraints:

SELECT conrelid::pg_catalog.regclass AS "constrained table",
conname AS constraint,
confrelid::pg_catalog.regclass AS "references",
pg_catalog.format('ALTER TABLE %s DROP CONSTRAINT %I;',
conrelid::pg_catalog.regclass, conname) AS "drop",
pg_catalog.format('ALTER TABLE %s ADD CONSTRAINT %I %s;',
conrelid::pg_catalog.regclass, conname,
pg_catalog.pg_get_constraintdef(oid)) AS "add"
FROM pg_catalog.pg_constraint c
WHERE contype = 'f' AND conparentid = 0 AND
(SELECT count(*) FROM pg_catalog.pg_constraint c2
WHERE c2.conparentid = c.oid) <>
((SELECT count(*) FROM pg_catalog.pg_inherits i
WHERE (i.inhparent = c.conrelid OR i.inhparent = c.confrelid) AND
EXISTS (SELECT 1 FROM pg_catalog.pg_partitioned_table
WHERE partrelid = i.inhparent)) +
CASE when pg_partition_root(conrelid) = confrelid THEN 1 ELSE 0 END);

This reports case 2 as OK and case 1 as bogus, as should be. I tried
adding more partitions and this seems to hold correctly. I was afraid
though that this would fail if we create an FK in an intermediate level
of the partition hierarchy ... but experimentation doesn't seem to give
that result. I've run out of time today to continue to look though.

Thanks very much for this really detailed analysis and sharing your insights. I'll give the new query a try on Monday when I'm back at work. Do I also need to recheck all other databases with this new query which didn't report anything with the original query?

Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"La vida es para el que se aventura"

You're located in the middle of the forest east of Freiburg im Breisgau in Germany? 🤣

Cheers,
Paul

Hello,

On 2024-Nov-29, Paul Foerster wrote:

On 29 Nov 2024, at 18:15, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

This reports case 2 as OK and case 1 as bogus, as should be. I tried
adding more partitions and this seems to hold correctly. I was afraid
though that this would fail if we create an FK in an intermediate level
of the partition hierarchy ... but experimentation doesn't seem to give
that result. I've run out of time today to continue to look though.

Thanks very much for this really detailed analysis and sharing your
insights. I'll give the new query a try on Monday when I'm back at
work. Do I also need to recheck all other databases with this new
query which didn't report anything with the original query?

Only if you have self-referencing FKs in partitioned tables. It
would be an interesting data point to verify whether this reports
anything else. Also, I'd be really curious if your databases include
the case I'm suspicious about: a multi-level hierarchy containing an FK
that points to an intermediate level of itself.

Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"La vida es para el que se aventura"

You're located in the middle of the forest east of Freiburg im
Breisgau in Germany? 🤣

I'm within fives minutes of longitude and latitude of that location, yes
:-) I didn't want to give unnecessary precision there, but is somebody
wants to chat sometime or whatever is welcome to ping me.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Estoy de acuerdo contigo en que la verdad absoluta no existe...
El problema es que la mentira sí existe y tu estás mintiendo" (G. Lama)

Hi Alvaro,

On 30 Nov 2024, at 08:41, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

Only if you have self-referencing FKs in partitioned tables. It
would be an interesting data point to verify whether this reports
anything else. Also, I'd be really curious if your databases include
the case I'm suspicious about: a multi-level hierarchy containing an FK
that points to an intermediate level of itself.

The instance I reported was the only one in our several hundred databases. So I guess this is really a corner case. As mentioned I'll try on Monday.

I'm within fives minutes of longitude and latitude of that location, yes
:-) I didn't want to give unnecessary precision there, but is somebody
wants to chat sometime or whatever is welcome to ping me.

Then you're not really far way. I'm located in the Solothurn, Switzerland area which is only less than 90 km away. Drop me a line if you ever make it to Solothurn. 🤣

Cheers,
Paul

Hello,

Belatedly, I came back to this issue in the release notes. Here's a
query for correctly reporting the problem and not reporting the cases
where there isn't a problem:

SELECT conrelid::pg_catalog.regclass AS "constrained table",
conname AS constraint,
confrelid::pg_catalog.regclass AS "references",
pg_catalog.format('ALTER TABLE %s DROP CONSTRAINT %I;',
conrelid::pg_catalog.regclass, conname) AS "drop",
pg_catalog.format('ALTER TABLE %s ADD CONSTRAINT %I %s;',
conrelid::pg_catalog.regclass, conname,
pg_catalog.pg_get_constraintdef(oid)) AS "add"
FROM pg_catalog.pg_constraint c
WHERE contype = 'f' AND conparentid = 0 AND
(SELECT count(*) FROM pg_catalog.pg_constraint c2
WHERE c2.conparentid = c.oid) <>
((SELECT count(*) FROM pg_catalog.pg_inherits i
WHERE (i.inhparent = c.conrelid OR i.inhparent = c.confrelid) AND
EXISTS (SELECT 1 FROM pg_catalog.pg_partitioned_table
WHERE partrelid = i.inhparent)) +
CASE WHEN pg_partition_root(conrelid) = confrelid THEN
(SELECT count(*) FROM pg_catalog.pg_partition_tree(confrelid) WHERE level = 1)
ELSE 0 END);

The difference from the query that's currently in the release notes is
that here we count the number of direct partitions of the referenced
table and expect that there be exactly that number of additional
constraint entries in a self-referential FK, compared to the situation
where the FK references a different table. (The query I suggested
previously in this thread had a "+1" instead of adding the number of
partitions, which obviously works correctly only in one particular
case.)

I tested this using Paul's scenario, and a few more, and as far as I can
tell, it is correct.

I'm going to fix the query in the release notes for all past branches
now, to avoid confusing people upgrading in the future ... hopefully not
many, but I don't think it's going to be zero people.

Regards

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Para tener más hay que desear menos"

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

I'm going to fix the query in the release notes for all past branches
now, to avoid confusing people upgrading in the future ... hopefully not
many, but I don't think it's going to be zero people.

OK, thanks.

regards, tom lane