Re: ALTER TYPE 0: Introduction; test cases

On Tue, Jan 11, 2011 at 09:41:35PM -0500, Robert Haas wrote:
> On Tue, Jan 11, 2011 at 7:25 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > On Tue, Jan 11, 2011 at 06:37:33AM -0500, Robert Haas wrote:
> >> On Sun, Jan 9, 2011 at 4:59 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> >> > This begins the patch series for the design I recently proposed[1] for avoiding
> >> > some table rewrites in ALTER TABLE ... ALTER COLUMN ... TYPE. ?I'm posting these
> >> > patches today:
> >> >
> >> > 0 - new test cases
> >>
> >> This doesn't look right. ?You might be building it, but you sure
> >> aren't rebuilding it.
> >>
> >> +CREATE TABLE parent (keycol numeric PRIMARY KEY);
> >> +NOTICE: ?CREATE TABLE / PRIMARY KEY will create implicit index
> >> "parent_pkey" for table "parent"
> >> +DEBUG: ?Rebuilding index "parent_pkey"
> >
> > Yes. ?I considered saying "Building" unconditionally. ?Differentiating the debug
> > message by passing down the fact that the index recently existed seemed like
> > overkill. ?What do you think?
>
> I'm wondering if we should consider moving this call to index_build()
> so that it appears everywhere that we build an index rather than just
> in ALTER TABLE, and saying something like:
>
> building index "%s" on table "%s"

The patch does have it in index_build. That new wording seems better.

> > The theoretical basis is that users can do little to directly change the need to
> > rebuild a TOAST index. ?We'll rebuild the TOAST index if and only if we rewrote
> > the table. ?The practical basis is that the TOAST relation names contain parent
> > relation OIDs, so we don't want them mentioned in regression test output.
>
> Perhaps in this case we could write:
>
> building TOAST index for table "%s"

Good idea; thanks.

I'll incorporate those changes into the next version.