8000 Make ALTER TABLE revalidate uniqueness and exclusion constraints. · postgrespro/postgres_cluster@8ceb245 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8ceb245

Browse files
robertmhaasrobertmhaas
committed
Make ALTER TABLE revalidate uniqueness and exclusion constraints.
Failure to do so can lead to constraint violations. This was broken by commit 1ddc270 on 2010-02-07, so back-patch to 9.0. Noah Misch. Regression test by me.
1 parent 14b9f69 commit 8ceb245

File tree

8 files changed

+48
-27
lines changed

8 files changed

+48
-27
lines changed

‎src/backend/catalog/index.c‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2529,26 +2529,29 @@ reindex_index(Oid indexId, bool skip_constraint_checks)
25292529
* reindex_relation - This routine is used to recreate all indexes
25302530
* of a relation (and optionally its toast relation too, if any).
25312531
*
2532-
* If heap_rebuilt is true, then the relation was just completely rebuilt by
2533-
* an operation such as VACUUM FULL or CLUSTER, and therefore its indexes are
2534-
* inconsistent with it. This makes things tricky if the relation is a system
2535-
* catalog that we might consult during the reindexing. To deal with that
2536-
* case, we mark all of the indexes as pending rebuild so that they won't be
2537-
* trusted until rebuilt. The caller is required to call us *without* having
2538-
* made the rebuilt versions visible by doing CommandCounterIncrement; we'll
2539-
* do CCI after having collected the index list. (This way we can still use
2540-
* catalog indexes while collecting the list.)
2532+
* "flags" can include REINDEX_SUPPRESS_INDEX_USE and REINDEX_CHECK_CONSTRAINTS.
25412533
*
2542-
* We also skip rechecking uniqueness/exclusion constraint properties if
2543-
* heap_rebuilt is true. This avoids likely deadlock conditions when doing
2544-
* VACUUM FULL or CLUSTER on system catalogs. REINDEX should be used to
2545-
* rebuild an index if constraint inconsistency is suspected.
2534+
* If flags has REINDEX_SUPPRESS_INDEX_USE, the relation was just completely
2535+
* rebuilt by an operation such as VACUUM FULL or CLUSTER, and therefore its
2536+
* indexes are inconsistent with it. This makes things tricky if the relation
2537+
* is a system catalog that we might consult during the reindexing. To deal
2538+
* with that case, we mark all of the indexes as pending rebuild so that they
2539+
* won't be trusted until rebuilt. The caller is required to call us *without*
2540+
* having made the rebuilt versions visible by doing CommandCounterIncrement;
< 8000 /code>2541+
* we'll do CCI after having collected the index list. (This way we can still
2542+
* use catalog indexes while collecting the list.)
2543+
*
2544+
* To avoid deadlocks, VACUUM FULL or CLUSTER on a system catalog must omit the
2545+
* REINDEX_CHECK_CONSTRAINTS flag. REINDEX should be used to rebuild an index
2546+
* if constraint inconsistency is suspected. For optimal performance, other
2547+
* callers should include the flag only after transforming the data in a manner
2548+
* that risks a change in constraint validity.
25462549
*
25472550
* Returns true if any indexes were rebuilt. Note that a
25482551
* CommandCounterIncrement will occur after each index rebuild.
25492552
*/
25502553
bool
2551-
reindex_relation(Oid relid, bool toast_too, bool heap_rebuilt)
2554+
reindex_relation(Oid relid, bool toast_too, int flags)
25522555
{
25532556
Relation rel;
25542557
Oid toast_relid;
@@ -2604,7 +2607,7 @@ reindex_relation(Oid relid, bool toast_too, bool heap_rebuilt)
26042607
List *doneIndexes;
26052608
ListCell *indexId;
26062609

2607-
if (heap_rebuilt)
2610+
if (flags & REINDEX_SUPPRESS_INDEX_USE)
26082611
{
26092612
/* Suppress use of all the indexes until they are rebuilt */
26102613
SetReindexPending(indexIds);
@@ -2625,11 +2628,11 @@ reindex_relation(Oid relid, bool toast_too, bool heap_rebuilt)
26252628
if (is_pg_class)
26262629
RelationSetIndexList(rel, doneIndexes, InvalidOid);
26272630

2628-
reindex_index(indexOid, heap_rebuilt);
2631+
reindex_index(indexOid, !(flags & REINDEX_CHECK_CONSTRAINTS));
26292632

26302633
CommandCounterIncrement();
26312634

2632-
if (heap_rebuilt)
2635+
if (flags & REINDEX_SUPPRESS_INDEX_USE)
26332636
RemoveReindexPending(indexOid);
26342637

26352638
if (is_pg_class)
@@ -2657,10 +2660,12 @@ reindex_relation(Oid relid, bool toast_too, bool heap_rebuilt)
26572660

26582661
/*
26592662
* If the relation has a secondary toast rel, reindex that too while we
2660-
* still hold the lock on the master table.
2663+
* still hold the lock on the master table. There's never a reason to
2664+
* reindex the toast table right after rebuilding the heap.
26612665
*/
2666+
Assert(!(toast_too && (flags & REINDEX_SUPPRESS_INDEX_USE)));
26622667
if (toast_too && OidIsValid(toast_relid))
2663-
result |= reindex_relation(toast_relid, false, false);
2668+
result |= reindex_relation(toast_relid, false, flags);
26642669

26652670
return result;
26662671
}

‎src/backend/commands/cluster.c‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ rebuild_relation(Relation Ol A93C dHeap, Oid indexOid,
565565
* rebuild the target's indexes and throw away the transient table.
566566
*/
567567
finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog,
568-
swap_toast_by_content, frozenXid);
568+
swap_toast_by_content, false, frozenXid);
569569
}
570570

571571

@@ -1362,10 +1362,12 @@ void
13621362
finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
13631363
bool is_system_catalog,
13641364
bool swap_toast_by_content,
1365+
bool check_constraints,
13651366
TransactionId frozenXid)
13661367
{
13671368
ObjectAddress object;
13681369
Oid mapped_tables[4];
1370+
int reindex_flags;
13691371
int i;
13701372

13711373
/* Zero out possible results from swapped_relation_files */
@@ -1395,7 +1397,10 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
13951397
* so no chance to reclaim disk space before commit. We do not need a
13961398
* final CommandCounterIncrement() because reindex_relation does it.
13971399
*/
1398-
reindex_relation(OIDOldHeap, false, true);
1400+
reindex_flags = REINDEX_SUPPRESS_INDEX_USE;
1401+
if (check_constraints)
1402+
reindex_flags |= REINDEX_CHECK_CONSTRAINTS;
1403+
reindex_relation(OIDOldHeap, false, reindex_flags);
13991404

14001405
/* Destroy new heap with old filenode */
14011406
object.classId = RelationRelationId;

‎src/backend/commands/indexcmds.c‎

Lines changed: 2 additions & 2 deletions
A93C
Original file line numberDiff line numberDiff line change
@@ -1629,7 +1629,7 @@ ReindexTable(RangeVar *relation)
16291629

16301630
ReleaseSysCache(tuple);
16311631

1632-
if (!reindex_relation(heapOid, true, false))
1632+
if (!reindex_relation(heapOid, true, 0))
16331633
ereport(NOTICE,
16341634
(errmsg("table \"%s\" has no indexes",
16351635
relation->relname)));
@@ -1742,7 +1742,7 @@ ReindexDatabase(const char *databaseName, bool do_system, bool do_user)
17421742
StartTransactionCommand();
17431743
/* functions in indexes may want a snapshot set */
17441744
PushActiveSnapshot(GetTransactionSnapshot());
1745-
if (reindex_relation(relid, true, false))
1745+
if (reindex_relation(relid, true, 0))
17461746
ereport(NOTICE,
17471747
(errmsg("table \"%s.%s\" was reindexed",
17481748
get_namespace_name(get_rel_namespace(relid)),

‎src/backend/commands/tablecmds.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,7 @@ ExecuteTruncate(TruncateStmt *stmt)
10761076
/*
10771077
* Reconstruct the indexes to match, and we're done.
10781078
*/
1079-
reindex_relation(heap_relid, true, false);
1079+
reindex_relation(heap_relid, true, 0);
10801080
}
10811081
}
10821082

@@ -3236,13 +3236,14 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
32363236

32373237
/*
32383238
* Swap the physical files of the old and new heaps, then rebuild
3239-
* indexes and discard the new heap. We can use RecentXmin for
3239+
* indexes and discard the old heap. We can use RecentXmin for
32403240
* the table's new relfrozenxid because we rewrote all the tuples
32413241
* in ATRewriteTable, so no older Xid remains in the table. Also,
32423242
* we never try to swap toast tables by content, since we have no
32433243
* interest in letting this code work on system catalogs.
32443244
*/
3245-
finish_heap_swap(tab->relid, OIDNewHeap, false, false, RecentXmin);
3245+
finish_heap_swap(tab->relid, OIDNewHeap,
3246+
false, false, true, RecentXmin);
32463247
}
32473248
else
32483249
{

‎src/include/catalog/index.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ extern double IndexBuildHeapScan(Relation heapRelation,
7171
extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot);
7272

7373
extern void reindex_index(Oid indexId, bool skip_constraint_checks);
74-
extern bool reindex_relation(Oid relid, bool toast_too, bool heap_rebuilt);
74+
75+
#define REINDEX_CHECK_CONSTRAINTS 0x1
76+
#define REINDEX_SUPPRESS_INDEX_USE 0x2
77+
extern bool reindex_relation(Oid relid, bool toast_too, int flags);
7578

7679
extern bool ReindexIsProcessingHeap(Oid heapOid);
7780
extern bool ReindexIsProcessingIndex(Oid indexOid);

‎src/include/commands/cluster.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ extern Oid make_new_heap(Oid OIDOldHeap, Oid NewTableSpace);
2929
extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
3030
bool is_system_catalog,
3131
bool swap_toast_by_content,
32+
bool check_constraints,
3233
TransactionId frozenXid);
3334

3435
#endif /* CLUSTER_H */

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,10 @@ insert into atacc1 (test) values (4);
413413
-- try adding a unique oid constraint
414414
alter table atacc1 add constraint atacc_oid1 unique(oid);
415415
NOTICE: ALTER TABLE / ADD UNIQUE will create implicit index "atacc_oid1" for table "atacc1"
416+
-- try to create duplicates via alter table using - should fail
417+
alter table atacc1 alter column test type integer using 0;
418+
ERROR: could not create unique index "atacc_test1"
419+
DETAIL: Key (test)=(0) is duplicated.
416420
drop table atacc1;
417421
-- let's do one where the unique constraint fails when added
418422
create table atacc1 ( test int );

‎src/test/regress/sql/alter_table.sql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,8 @@ insert into atacc1 (test) values (2);
419419
insert 5F11 into atacc1 (test) values (4);
420420
-- try adding a unique oid constraint
421421
alter table atacc1 add constraint atacc_oid1 unique(oid);
422+
-- try to create duplicates via alter table using - should fail
423+
alter table atacc1 alter column test type integer using 0;
422424
drop table atacc1;
423425

424426
-- let's do one where the unique constraint fails when added

0 commit comments

Comments
 (0)
0