On Sun, Feb 13, 2011 at 12:04:20AM -0500, Robert Haas wrote:
> On Sat, Feb 12, 2011 at 10:45 AM, Noah Misch <noah@leadboat.com> wrote:
> > 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 think I like the idea of passing it the AlteredTableInfo.
Okay.
> > 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.
>
> So in the case of a constrained domain, it looks like we're going to
> evaluate the changed columns, but if no error is thrown, we're going
> to assume they match the original ones and throw out the data?
Correct. We can see that a RelabelType changes no values by inspecting
ExecEvalRelabelType. Likewise, by inspecting ExecEvalCoerceToDomain, we can
know that a CoerceToDomain node may introduce errors but never modified values.
> Yikes.
> I didn't like that Assert much, but maybe we need it, because this is
> scary.
Can you elaborate on the fear-inducing aspect? I don't mind re-adding the
Assert, but it seems that some positive understanding of the assumption's
validity is in order.
nm