Re: ALTER TYPE 2: skip already-provable no-work rewrites

On Fri, Feb 11, 2011 at 02:49:27PM -0500, Robert Haas wrote:
> You might want to consider a second boolean in lieu of a three way
> enum.  I'm not sure if that's cleaner but if it lets you write:
>
> if (blah)
>     at->verify = true;
>
> instead of:
>
> if (blah)
>      at->worklevel = Min(at->worklevel, WORK_VERIFY);
>
> ...then I think that might be cleaner.

Good point; the Max() calls did not make much sense all by themselves.  The
point was to make sure nothing decreased the worklevel.  Wrapping them in a
macro, say, ATRequireWork, probably would have helped.

That said, I've tried both constructions, and I marginally prefer the end result
with AlteredTableInfo.verify.  I've inlined ATColumnChangeRequiresRewrite into
ATPrepAlterColumnType; it would need to either pass back two bools or take an
AlteredTableInfo arg to mutate, so this seemed cleaner.  I've omitted the
assertion that my previous version added to ATRewriteTable; it was helpful for
other scan-only type changes, but it's excessive for domains alone.  Otherwise,
the differences are cosmetic.

The large block in ATRewriteTable is now superfluous.  For easier review, I
haven't removed it.

I missed a typo in the last patch: "T if we a rewrite is forced".  Not changed
in this patch as I assume you'll want to commit it separately.

nm