Re: ALTER TYPE 0: Introduction; test cases - Mailing list pgsql-hackers
From | Noah Misch |
---|---|
Subject | Re: ALTER TYPE 0: Introduction; test cases |
Date | |
Msg-id | 20110115152548.GB28228@tornado.leadboat.com Whole thread Raw |
In response to | Re: ALTER TYPE 0: Introduction; test cases (Robert Haas <robertmhaas@gmail.com>) |
Responses |
Re: ALTER TYPE 0: Introduction; test cases
(Robert Haas <robertmhaas@gmail.com>)
|
List | pgsql-hackers |
On Sat, Jan 15, 2011 at 08:57:30AM -0500, Robert Haas wrote: > On Sat, Jan 15, 2011 at 1:30 AM, Noah Misch <noah@leadboat.com> wrote: > > 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. > > + * Table, NOT NULL and DEFAULT constraints and the "oid" system column do > + * not (currently) follow the row type, so they require no attention here. > + * The non-propagation of DEFAULT and NOT NULL make ADD COLUMN safe, too. > > This comment seems somewhat unrelated to the rest of the patch, and I > really hope that the first word ("Table") actually means "CHECK", > because we certainly shouldn't be treating table check constraints and > column check constraints differently, at least AIUI. "Table" should be "CHECK"; thanks. I added the comment in this patch because it's clarifying existing behavior that was not obvious to me, rather than documenting a new fact arising due to the patch series. A few of the new test cases address this behavior. > > 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. > > Well, I pretty much think that split is going to be not what anyone > wants for any purpose OTHER than the regression tests. So if there's > no other way around it I'd be much more inclined to remove the > regression tests than to keep that split. Do you value test coverage so little?
pgsql-hackers by date: