at1-check-unique.patch

text/plain

Filename: at1-check-unique.patch
Type: text/plain
Part: 0
Message: Re: ALTER TYPE 1: recheck index-based constraints

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: context
File+
src/backend/catalog/index.c 12 0
src/backend/commands/cluster.c 3 0
src/backend/commands/indexcmds.c 2 0
src/backend/commands/tablecmds.c 4 0
src/include/catalog/index.h 2 0
src/include/commands/cluster.h 1 0
src/test/regress/expected/alter_table.out 4 0
src/test/regress/sql/alter_table.sql 0 0
*** a/src/backend/catalog/index.c
--- b/src/backend/catalog/index.c
***************
*** 2543,2558 **** reindex_index(Oid indexId, bool skip_constraint_checks)
   * do CCI after having collected the index list.  (This way we can still use
   * catalog indexes while collecting the list.)
   *
!  * We also skip rechecking uniqueness/exclusion constraint properties if
!  * heap_rebuilt is true.  This avoids likely deadlock conditions when doing
!  * VACUUM FULL or CLUSTER on system catalogs.  REINDEX should be used to
!  * rebuild an index if constraint inconsistency is suspected.
   *
   * Returns true if any indexes were rebuilt.  Note that a
   * CommandCounterIncrement will occur after each index rebuild.
   */
  bool
! reindex_relation(Oid relid, bool toast_too, bool heap_rebuilt)
  {
  	Relation	rel;
  	Oid			toast_relid;
--- 2543,2559 ----
   * do CCI after having collected the index list.  (This way we can still use
   * catalog indexes while collecting the list.)
   *
!  * If we rebuilt a heap without changing tuple values, we also skip rechecking
!  * uniqueness/exclusion constraint properties.  This avoids likely deadlock
!  * conditions when doing VACUUM FULL or CLUSTER on system catalogs.  REINDEX
!  * should be used to rebuild an index if constraint inconsistency is suspected.
   *
   * Returns true if any indexes were rebuilt.  Note that a
   * CommandCounterIncrement will occur after each index rebuild.
   */
  bool
! reindex_relation(Oid relid, bool toast_too, bool heap_rebuilt,
! 				 bool tuples_changed)
  {
  	Relation	rel;
  	Oid			toast_relid;
***************
*** 2629,2635 **** reindex_relation(Oid relid, bool toast_too, bool heap_rebuilt)
  			if (is_pg_class)
  				RelationSetIndexList(rel, doneIndexes, InvalidOid);
  
! 			reindex_index(indexOid, heap_rebuilt);
  
  			CommandCounterIncrement();
  
--- 2630,2636 ----
  			if (is_pg_class)
  				RelationSetIndexList(rel, doneIndexes, InvalidOid);
  
! 			reindex_index(indexOid, heap_rebuilt && !tuples_changed);
  
  			CommandCounterIncrement();
  
***************
*** 2661,2670 **** reindex_relation(Oid relid, bool toast_too, bool heap_rebuilt)
  
  	/*
  	 * If the relation has a secondary toast rel, reindex that too while we
! 	 * still hold the lock on the master table.
  	 */
  	if (toast_too && OidIsValid(toast_relid))
! 		result |= reindex_relation(toast_relid, false, false);
  
  	return result;
  }
--- 2662,2674 ----
  
  	/*
  	 * If the relation has a secondary toast rel, reindex that too while we
! 	 * still hold the lock on the master table.  There's never a reason to
! 	 * reindex the toast table if heap_rebuilt; it will always be fresh.
  	 */
+ 	Assert(!(toast_too && heap_rebuilt));
  	if (toast_too && OidIsValid(toast_relid))
! 		result |= reindex_relation(toast_relid, false, heap_rebuilt,
! 								   tuples_changed);
  
  	return result;
  }
*** a/src/backend/commands/cluster.c
--- b/src/backend/commands/cluster.c
***************
*** 565,571 **** rebuild_relation(Relation OldHeap, Oid indexOid,
  	 * rebuild the target's indexes and throw away the transient table.
  	 */
  	finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog,
! 					 swap_toast_by_content, frozenXid);
  }
  
  
--- 565,571 ----
  	 * rebuild the target's indexes and throw away the transient table.
  	 */
  	finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog,
! 					 swap_toast_by_content, false, frozenXid);
  }
  
  
***************
*** 1362,1367 **** void
--- 1362,1368 ----
  finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
  				 bool is_system_catalog,
  				 bool swap_toast_by_content,
+ 				 bool tuples_changed,
  				 TransactionId frozenXid)
  {
  	ObjectAddress object;
***************
*** 1395,1401 **** finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
  	 * so no chance to reclaim disk space before commit.  We do not need a
  	 * final CommandCounterIncrement() because reindex_relation does it.
  	 */
! 	reindex_relation(OIDOldHeap, false, true);
  
  	/* Destroy new heap with old filenode */
  	object.classId = RelationRelationId;
--- 1396,1402 ----
  	 * so no chance to reclaim disk space before commit.  We do not need a
  	 * final CommandCounterIncrement() because reindex_relation does it.
  	 */
! 	reindex_relation(OIDOldHeap, false, true, tuples_changed);
  
  	/* Destroy new heap with old filenode */
  	object.classId = RelationRelationId;
*** a/src/backend/commands/indexcmds.c
--- b/src/backend/commands/indexcmds.c
***************
*** 1629,1635 **** ReindexTable(RangeVar *relation)
  
  	ReleaseSysCache(tuple);
  
! 	if (!reindex_relation(heapOid, true, false))
  		ereport(NOTICE,
  				(errmsg("table \"%s\" has no indexes",
  						relation->relname)));
--- 1629,1635 ----
  
  	ReleaseSysCache(tuple);
  
! 	if (!reindex_relation(heapOid, true, false, false))
  		ereport(NOTICE,
  				(errmsg("table \"%s\" has no indexes",
  						relation->relname)));
***************
*** 1742,1748 **** ReindexDatabase(const char *databaseName, bool do_system, bool do_user)
  		StartTransactionCommand();
  		/* functions in indexes may want a snapshot set */
  		PushActiveSnapshot(GetTransactionSnapshot());
! 		if (reindex_relation(relid, true, false))
  			ereport(NOTICE,
  					(errmsg("table \"%s.%s\" was reindexed",
  							get_namespace_name(get_rel_namespace(relid)),
--- 1742,1748 ----
  		StartTransactionCommand();
  		/* functions in indexes may want a snapshot set */
  		PushActiveSnapshot(GetTransactionSnapshot());
! 		if (reindex_relation(relid, true, false, false))
  			ereport(NOTICE,
  					(errmsg("table \"%s.%s\" was reindexed",
  							get_namespace_name(get_rel_namespace(relid)),
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 1076,1082 **** ExecuteTruncate(TruncateStmt *stmt)
  			/*
  			 * Reconstruct the indexes to match, and we're done.
  			 */
! 			reindex_relation(heap_relid, true, false);
  		}
  	}
  
--- 1076,1082 ----
  			/*
  			 * Reconstruct the indexes to match, and we're done.
  			 */
! 			reindex_relation(heap_relid, true, false, false);
  		}
  	}
  
***************
*** 3236,3248 **** ATRewriteTables(List **wqueue, LOCKMODE lockmode)
  
  			/*
  			 * Swap the physical files of the old and new heaps, then rebuild
! 			 * indexes and discard the new heap.  We can use RecentXmin for
  			 * the table's new relfrozenxid because we rewrote all the tuples
  			 * in ATRewriteTable, so no older Xid remains in the table.  Also,
  			 * we never try to swap toast tables by content, since we have no
  			 * interest in letting this code work on system catalogs.
  			 */
! 			finish_heap_swap(tab->relid, OIDNewHeap, false, false, RecentXmin);
  		}
  		else
  		{
--- 3236,3249 ----
  
  			/*
  			 * Swap the physical files of the old and new heaps, then rebuild
! 			 * indexes and discard the old heap.  We can use RecentXmin for
  			 * the table's new relfrozenxid because we rewrote all the tuples
  			 * in ATRewriteTable, so no older Xid remains in the table.  Also,
  			 * we never try to swap toast tables by content, since we have no
  			 * interest in letting this code work on system catalogs.
  			 */
! 			finish_heap_swap(tab->relid, OIDNewHeap,
! 							 false, false, true, RecentXmin);
  		}
  		else
  		{
*** a/src/include/catalog/index.h
--- b/src/include/catalog/index.h
***************
*** 71,77 **** extern double IndexBuildHeapScan(Relation heapRelation,
  extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot);
  
  extern void reindex_index(Oid indexId, bool skip_constraint_checks);
! extern bool reindex_relation(Oid relid, bool toast_too, bool heap_rebuilt);
  
  extern bool ReindexIsProcessingHeap(Oid heapOid);
  extern bool ReindexIsProcessingIndex(Oid indexOid);
--- 71,78 ----
  extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot);
  
  extern void reindex_index(Oid indexId, bool skip_constraint_checks);
! extern bool reindex_relation(Oid relid, bool toast_too, bool heap_rebuilt,
! 				 bool tuples_changed);
  
  extern bool ReindexIsProcessingHeap(Oid heapOid);
  extern bool ReindexIsProcessingIndex(Oid indexOid);
*** a/src/include/commands/cluster.h
--- b/src/include/commands/cluster.h
***************
*** 29,34 **** extern Oid	make_new_heap(Oid OIDOldHeap, Oid NewTableSpace);
--- 29,35 ----
  extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
  				 bool is_system_catalog,
  				 bool swap_toast_by_content,
+ 				 bool tuples_changed,
  				 TransactionId frozenXid);
  
  #endif   /* CLUSTER_H */
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
***************
*** 1774,1779 **** DEBUG:  Rebuilding index "t_strarr_idx"
--- 1774,1781 ----
  DEBUG:  Rebuilding index "t_square_idx"
  DEBUG:  Rebuilding index "t_expr_idx"
  DEBUG:  Rebuilding index "t_touchy_f_idx"
+ ERROR:  could not create unique index "t_touchy_f_idx"
+ DETAIL:  Key (touchy_f(constraint1))=(100) is duplicated.
  ALTER TABLE t ALTER constraint2 TYPE trickint;						-- noop-e
  DEBUG:  Rewriting table "t"
  DEBUG:  Rebuilding index "t_constraint4_key"
***************
*** 1792,1816 **** DEBUG:  Rebuilding index "t_strarr_idx"
  DEBUG:  Rebuilding index "t_square_idx"
  DEBUG:  Rebuilding index "t_touchy_f_idx"
  DEBUG:  Rebuilding index "t_expr_idx"
! -- Temporary fixup until behavior of the previous two improves.
! ALTER TABLE t ALTER constraint1 TYPE int, ALTER constraint2 TYPE int;
! DEBUG:  Rewriting table "t"
! DEBUG:  Rebuilding index "t_constraint4_key"
! DEBUG:  Rebuilding index "t_integral_key"
! DEBUG:  Rebuilding index "t_rational_key"
! DEBUG:  Rebuilding index "t_daytimetz_key"
! DEBUG:  Rebuilding index "t_daytime_key"
! DEBUG:  Rebuilding index "t_stamptz_key"
! DEBUG:  Rebuilding index "t_stamp_key"
! DEBUG:  Rebuilding index "t_timegap_key"
! DEBUG:  Rebuilding index "t_bits_key"
! DEBUG:  Rebuilding index "t_network_key"
! DEBUG:  Rebuilding index "t_string_idx"
! DEBUG:  Rebuilding index "t_string_idx1"
! DEBUG:  Rebuilding index "t_strarr_idx"
! DEBUG:  Rebuilding index "t_square_idx"
! DEBUG:  Rebuilding index "t_touchy_f_idx"
! DEBUG:  Rebuilding index "t_expr_idx"
  -- Change a column with an outgoing foreign key constraint.
  ALTER TABLE t ALTER constraint3 TYPE numeric(8,1); -- rewrite, FK error
  DEBUG:  Rewriting table "t"
--- 1794,1801 ----
  DEBUG:  Rebuilding index "t_square_idx"
  DEBUG:  Rebuilding index "t_touchy_f_idx"
  DEBUG:  Rebuilding index "t_expr_idx"
! ERROR:  could not create unique index "t_expr_idx"
! DETAIL:  Key ((1))=(1) is duplicated.
  -- Change a column with an outgoing foreign key constraint.
  ALTER TABLE t ALTER constraint3 TYPE numeric(8,1); -- rewrite, FK error
  DEBUG:  Rewriting table "t"
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
***************
*** 1246,1253 **** FROM pg_trigger WHERE tgrelid = 't'::regclass ORDER BY tgname;
  ALTER TABLE t ALTER constraint0 TYPE trickint;						-- verify-e
  ALTER TABLE t ALTER constraint1 TYPE trickint;						-- noop-e
  ALTER TABLE t ALTER constraint2 TYPE trickint;						-- noop-e
- -- Temporary fixup until behavior of the previous two improves.
- ALTER TABLE t ALTER constraint1 TYPE int, ALTER constraint2 TYPE int;
  
  -- Change a column with an outgoing foreign key constraint.
  ALTER TABLE t ALTER constraint3 TYPE numeric(8,1); -- rewrite, FK error
--- 1246,1251 ----