Thread

Re: Avoid index rebuilds for no-rewrite ALTER TABLE ALTER TYPE

Noah Misch <noah@leadboat.com> — 2011-06-30T15:55:19Z

On Wed, Jun 29, 2011 at 09:42:06AM -0400, Robert Haas wrote:
> On Tue, Jun 28, 2011 at 3:40 PM, Noah Misch <noah@leadboat.com> wrote:

> > Here's the call stack in question:
> >
> > ? ? ? ?RelationBuildLocalRelation
> > ? ? ? ?heap_create
> > ? ? ? ?index_create
> > ? ? ? ?DefineIndex
> > ? ? ? ?ATExecAddIndex
> >
> > Looking at it again, it wouldn't bring the end of the world to add a relfilenode
> > argument to each. None of those have more than four callers.
> 
> Yeah.  Those functions take an awful lot of arguments, which suggests
> that some refactoring might be in order, but I still think it's
> cleaner to add another argument than to change the state around
> after-the-fact.
> 
> > ATExecAddIndex()
> > would then call RelationPreserveStorage() before calling DefineIndex(), which
> > would in turn put things in a correct state from the start. ?Does that seem
> > appropriate? ?Offhand, I do like it better than what I had.
> 
> I wish we could avoid the whole death-and-resurrection thing
> altogether, but off-hand I'm not seeing a real clean way to do that.
> At the very least we had better comment it to death.

I couldn't think of a massive amount to say about that, but see what you think
of this level of commentary.

Looking at this again turned up a live bug in the previous version: if the old
index file were created in the current transaction, we would wrongly remove its
delete-at-abort entry as well as its delete-at-commit entry.  This leaked the
disk file.  Fixed by adding an argument to RelationPreserveStorage() indicating
which kind to remove.  Test case:

	BEGIN;
	CREATE TABLE t AS SELECT * FROM generate_series(1,100000) t(n);
	CREATE INDEX ti ON t(n);
	SELECT pg_relation_filepath('ti');
	ALTER TABLE t ALTER n TYPE int;
	ROLLBACK;
	CHECKPOINT;
	-- file named above should be gone

I also updated the ATPostAlterTypeCleanup() variable names per discussion and
moved IndexStmt.oldNode to a more-natural position in the structure.

Thanks,
nm