Re: ALTER TYPE 0: Introduction; test cases

From: Noah Misch <noah(at)leadboat(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: ALTER TYPE 0: Introduction; test cases
Date: 2011-01-15 06:30:16
Message-ID: 20110115063016.GA9755@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Here's v2 based on your feedback.

I pruned test coverage down to just the highlights. By the end of patch series,
the net change becomes +67 to alter_table.sql and +111 to alter_table.out. The
alter_table.out delta is larger in this patch (+150), because the optimizations
don't yet apply and more objects are reported as rebuilt.

If this looks sane, I'll rebase the rest of the patches accordingly.

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:
> 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"

Done.

I also fixed the leading capital letter on the other new messages.

> > 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"
>
> There's limited need for users to know the actual name of the TOAST
> table, but I think it's better to log a line for all the actions we're
> performing or none of them, rather than some but not others.

That was a good idea, but the implementation is awkward. index_build has the
TOAST table and index relations, and there's no good way to find the parent
table from either. No index covers pg_class.reltoastrelid, so scanning by that
would be a bad idea. Autovacuum solves this by building its own hash table with
the mapping; that wouldn't fit well here. We could parse the parent OID out of
the TOAST name (heh, heh). I suppose we could pass the parent relation from
create_toast_table down through index_create to index_build. Currently,
index_create knows nothing of the fact that it's making a TOAST index, and
changing that to improve this messages seems a bit excessive. Thoughts?

For this version, I've kept the DEBUG1/DEBUG2 split by TOAST.

Attachment Content-Type Size
at0v2-debug-tests.patch text/plain 13.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Smith 2011-01-15 06:51:50 Re: auto-sizing wal_buffers
Previous Message Alex Hunsaker 2011-01-15 05:31:03 Re: Optimize PL/Perl function argument passing [PATCH]