On Tue, Jan 25, 2011 at 10:22 PM, Noah Misch <noah@leadboat.com> wrote:
> I'm fine with this patch.
OK, committed.
> The next patch removed new_changeoids, so we would instead be keeping it with
> this as the only place referencing it.
[...]
> The at2v2 patch would then morph to do something like:
>
> if (worklevel != WORK_NONE)
> tab->new_changetypes = true;
Well, I'm not too keen on either of those things. The second one,
especially, looks like the sense of the Boolean is clearly being
abused, so either the Boolean needs to be renamed or some other change
is required.
I'd also suggest that this big if-block you changed to a case
statement could just as well stay as an if-block. There are only
three cases, and we want to avoid rearranging things more than
necessary. It complicates both review and back-patching to no good
end.
I think you should collect up what's left of ALTER TABLE 0 and the
stuff on this thread, rebase it, and submit it as a single patch on
this thread that applies directly against the master branch. We may
decide to split it back up again in some other way, but I think the
current division isn't actually buying us much.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
