Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0574825

Browse files
alvherretenderwg
andcommitted
Fix ALTER TABLE DETACH for inconsistent indexes
When a partitioned table has an index that doesn't support a constraint,but a partition has an equivalent index that does, then a DETACHoperation would misbehave: a crash in assertion-enabled systems (becausewe fail to find the constraint in the parent that we expect to), or abroken coninhcount value (-1) in production systems (because we blindlybelieve that we've successfully detached the parent).While we should reject an ATTACH of a partition with such an index, wehave failed to do so in existing releases, so adding an error in stablereleases might break the (unlikely) existing applications that rely onthis behavior. At this point I don't even want to reject them inmaster, because it'd break pg_upgrade if such databases exist, and therewould be no easy way to fix existing databases without expensive indexrebuilds.(Later on we could add ALTER TABLE ... ADD CONSTRAINT USING INDEX topartitioned tables, which would allow the user to fix such patterns. Atthat point we could add more restrictions to prevent the problem fromits root.)Also, add a test case that leaves one table in this condition, so thatwe can verify that pg_upgrade continues to work if we later decide tochange the policy on the master branch.Backpatch to all supported branches.Co-authored-by: Tender Wang <tndrwang@gmail.com>Reported-by: Alexander Lakhin <exclusion@gmail.com>Reviewed-by: Tender Wang <tndrwang@gmail.com>Reviewed-by: Michael Paquier <michael@paquier.xyz>Discussion:https://postgr.es/m/18500-62948b6fe5522f56@postgresql.org
1 parentcf2c69e commit0574825

File tree

4 files changed

+97
-4
lines changed

4 files changed

+97
-4
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17462,22 +17462,31 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
1746217462
foreach(cell, indexes)
1746317463
{
1746417464
Oididxid = lfirst_oid(cell);
17465+
Oidparentidx;
1746517466
Relationidx;
1746617467
OidconstrOid;
17468+
OidparentConstrOid;
1746717469

1746817470
if (!has_superclass(idxid))
1746917471
continue;
1747017472

17471-
Assert((IndexGetRelation(get_partition_parent(idxid), false) ==
17472-
RelationGetRelid(rel)));
17473+
parentidx =get_partition_parent(idxid);
17474+
Assert(IndexGetRelation(parentidx, false) ==RelationGetRelid(rel));
1747317475

1747417476
idx = index_open(idxid, AccessExclusiveLock);
1747517477
IndexSetParentIndex(idx, InvalidOid);
1747617478

17477-
/* If there's a constraint associated with the index, detach it too */
17479+
/*
17480+
* If there's a constraint associated with the index, detach it too.
17481+
* Careful: it is possible for a constraint index in a partition to be
17482+
* the child of a non-constraint index, so verify whether the parent
17483+
* index does actually have a constraint.
17484+
*/
1747817485
constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel),
1747917486
idxid);
17480-
if (OidIsValid(constrOid))
17487+
parentConstrOid = get_relation_idx_constraint_oid(RelationGetRelid(rel),
17488+
parentidx);
17489+
if (OidIsValid(parentConstrOid) && OidIsValid(constrOid))
1748117490
ConstraintSetParentConstraint(constrOid, InvalidOid, InvalidOid);
1748217491

1748317492
index_close(idx, NoLock);

‎src/test/regress/expected/sanity_check.out

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ quad_poly_tbl|t
172172
radix_text_tbl|t
173173
ramp|f
174174
real_city|f
175+
regress_constr_partition1|t
176+
regress_constr_partitioned|t
175177
road|t
176178
shighway|t
177179
slow_emp4000|f

‎src/test/regress/input/constraints.source

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,46 @@ ALTER TABLE parted_fk_naming ATTACH PARTITION parted_fk_naming_1 FOR VALUES IN (
429429
SELECT conname FROM pg_constraint WHERE conrelid = 'parted_fk_naming_1'::regclass AND contype = 'f';
430430
DROP TABLE parted_fk_naming;
431431

432+
--
433+
-- Test various ways to create primary keys on partitions, linked to unique
434+
-- indexes (without constraints) on the partitioned table. Ideally these should
435+
-- fail, but we don't dare change released behavior, so instead cope with it at
436+
-- DETACH time.
437+
CREATE TEMP TABLE t (a integer, b integer) PARTITION BY HASH (a, b);
438+
CREATE TEMP TABLE tp (a integer, b integer, PRIMARY KEY (a, b), UNIQUE (b, a));
439+
ALTER TABLE t ATTACH PARTITION tp FOR VALUES WITH (MODULUS 1, REMAINDER 0);
440+
CREATE UNIQUE INDEX t_a_idx ON t (a, b);
441+
CREATE UNIQUE INDEX t_b_idx ON t (b, a);
442+
ALTER INDEX t_a_idx ATTACH PARTITION tp_pkey;
443+
ALTER INDEX t_b_idx ATTACH PARTITION tp_b_a_key;
444+
ALTER TABLE t DETACH PARTITION tp;
445+
SELECT conname, conparentid, conislocal, coninhcount
446+
FROM pg_constraint WHERE conname IN ('tp_pkey', 'tp_b_a_key');
447+
DROP TABLE t, tp;
448+
449+
CREATE TEMP TABLE t (a integer) PARTITION BY LIST (a);
450+
CREATE TEMP TABLE tp (a integer PRIMARY KEY);
451+
CREATE UNIQUE INDEX t_a_idx ON t (a);
452+
ALTER TABLE t ATTACH PARTITION tp FOR VALUES IN (1);
453+
ALTER TABLE t DETACH PARTITION tp;
454+
DROP TABLE t, tp;
455+
456+
CREATE TEMP TABLE t (a integer) PARTITION BY LIST (a);
457+
CREATE TEMP TABLE tp (a integer PRIMARY KEY);
458+
CREATE UNIQUE INDEX t_a_idx ON ONLY t (a);
459+
ALTER TABLE t ATTACH PARTITION tp FOR VALUES IN (1);
460+
ALTER TABLE t DETACH PARTITION tp;
461+
DROP TABLE t, tp;
462+
463+
CREATE TABLE regress_constr_partitioned (a integer) PARTITION BY LIST (a);
464+
CREATE TABLE regress_constr_partition1 PARTITION OF regress_constr_partitioned FOR VALUES IN (1);
465+
ALTER TABLE regress_constr_partition1 ADD PRIMARY KEY (a);
466+
CREATE UNIQUE INDEX ON regress_constr_partitioned (a);
467+
BEGIN;
468+
ALTER TABLE regress_constr_partitioned DETACH PARTITION regress_constr_partition1;
469+
ROLLBACK;
470+
-- Leave this one in funny state for pg_upgrade testing
471+
432472
-- test a HOT update that invalidates the conflicting tuple.
433473
-- the trigger should still fire and catch the violation
434474

‎src/test/regress/output/constraints.source

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,48 @@ SELECT conname FROM pg_constraint WHERE conrelid = 'parted_fk_naming_1'::regclas
598598
(1 row)
599599

600600
DROP TABLE parted_fk_naming;
601+
--
602+
-- Test various ways to create primary keys on partitions, linked to unique
603+
-- indexes (without constraints) on the partitioned table. Ideally these should
604+
-- fail, but we don't dare change released behavior, so instead cope with it at
605+
-- DETACH time.
606+
CREATE TEMP TABLE t (a integer, b integer) PARTITION BY HASH (a, b);
607+
CREATE TEMP TABLE tp (a integer, b integer, PRIMARY KEY (a, b), UNIQUE (b, a));
608+
ALTER TABLE t ATTACH PARTITION tp FOR VALUES WITH (MODULUS 1, REMAINDER 0);
609+
CREATE UNIQUE INDEX t_a_idx ON t (a, b);
610+
CREATE UNIQUE INDEX t_b_idx ON t (b, a);
611+
ALTER INDEX t_a_idx ATTACH PARTITION tp_pkey;
612+
ALTER INDEX t_b_idx ATTACH PARTITION tp_b_a_key;
613+
ALTER TABLE t DETACH PARTITION tp;
614+
SELECT conname, conparentid, conislocal, coninhcount
615+
FROM pg_constraint WHERE conname IN ('tp_pkey', 'tp_b_a_key');
616+
conname | conparentid | conislocal | coninhcount
617+
------------+-------------+------------+-------------
618+
tp_pkey | 0 | t | 0
619+
tp_b_a_key | 0 | t | 0
620+
(2 rows)
621+
622+
DROP TABLE t, tp;
623+
CREATE TEMP TABLE t (a integer) PARTITION BY LIST (a);
624+
CREATE TEMP TABLE tp (a integer PRIMARY KEY);
625+
CREATE UNIQUE INDEX t_a_idx ON t (a);
626+
ALTER TABLE t ATTACH PARTITION tp FOR VALUES IN (1);
627+
ALTER TABLE t DETACH PARTITION tp;
628+
DROP TABLE t, tp;
629+
CREATE TEMP TABLE t (a integer) PARTITION BY LIST (a);
630+
CREATE TEMP TABLE tp (a integer PRIMARY KEY);
631+
CREATE UNIQUE INDEX t_a_idx ON ONLY t (a);
632+
ALTER TABLE t ATTACH PARTITION tp FOR VALUES IN (1);
633+
ALTER TABLE t DETACH PARTITION tp;
634+
DROP TABLE t, tp;
635+
CREATE TABLE regress_constr_partitioned (a integer) PARTITION BY LIST (a);
636+
CREATE TABLE regress_constr_partition1 PARTITION OF regress_constr_partitioned FOR VALUES IN (1);
637+
ALTER TABLE regress_constr_partition1 ADD PRIMARY KEY (a);
638+
CREATE UNIQUE INDEX ON regress_constr_partitioned (a);
639+
BEGIN;
640+
ALTER TABLE regress_constr_partitioned DETACH PARTITION regress_constr_partition1;
641+
ROLLBACK;
642+
-- Leave this one in funny state for pg_upgrade testing
601643
-- test a HOT update that invalidates the conflicting tuple.
602644
-- the trigger should still fire and catch the violation
603645
BEGIN;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp