Thread

ALTER TYPE 7: avoid index rebuilds/FK validations

Noah Misch <noah@leadboat.com> — 2011-01-13T23:01:24Z

When the earlier patches in this series allow ALTER TABLE ... ALTER COLUMN
... TYPE to skip the table rewrite, it still rebuilds indexes and rechecks
constraints depending on that column.  With this patch, we'll skip an index
rebuild when the change does not cross operator families and the index has no
expression column or predicate.  Also, we'll skip FOREIGN KEY validation
unconditionally (create-time foreign key compatibility checks still apply).
These policies have some formal hazards detailed in the code comments, but I
believe those hazards do not extend to any built-in types or credible
user-defined types.  We may wish to remove the formal gap by extending the
contract of B-tree operator families: something along the lines that casts
between types in the family must preserve equality within the family.

The standing code handled index/constraint dependencies of changing columns by
extracting the SQL definition using pg_get_indexdef or pg_get_constraintdef,
dropping the object, and recreating it afresh.  To implement this optimization
for indexes and index-backed constraints, we need to update the index definition
without perturbing its storage.  I saw two major ways to do this and have
attached implementations of each, both having the same user-facing semantics.
I'd appreciate feedback on which is preferable.  The first skips the drop and
updates pg_index.indclass, pg_attribute, and pg_constraint.conexclop.  The
second keeps the drop and create, then resurrects the old relfilenode and
assigns it to the new object.  The second strategy is significantly simpler and
smaller, but it seems less-like anything else we do.  As a further variation on
the second approach, I considered drilling holes through the performDeletion()
and DefineIndex() stacks to avoid the drop-and-later-preserve dynamic, but that
ultimately appeared uglier.

ATPostAlterTypeCleanup has this comment:

	/*
	 * Re-parse the index and constraint definitions, and attach them to the
	 * appropriate work queue entries.	We do this before dropping because in
	 * the case of a FOREIGN KEY constraint, we might not yet have exclusive
	 * lock on the table the constraint is attached to, and we need to get
	 * that before dropping.  It's safe because the parser won't actually look
	 * at the catalogs to detect the existing entry.
	 */

Is the second sentence true?  I don't think so, so I deleted it for now.  Here
is the sequence of lock requests against the table possessing the FOREIGN KEY
constraint when we alter the parent/upstream column:

transformAlterTableStmt - ShareRowExclusiveLock
ATPostAlterTypeParse - lockmode of original ALTER TABLE
RemoveTriggerById() for update trigger - ShareRowExclusiveLock
RemoveTriggerById() for insert trigger - ShareRowExclusiveLock
RemoveConstraintById() - AccessExclusiveLock
CreateTrigger() for insert trigger - ShareRowExclusiveLock
CreateTrigger() for update trigger - ShareRowExclusiveLock
RI_Initial_Check() - AccessShareLock (3x)

Thanks,
nm