Re: ALTER TYPE 1: recheck index-based constraints

On Thu, Jan 20, 2011 at 09:26:29AM -0500, Robert Haas wrote:
> My main beef with the Boolean flags is that this kind of thing is not too clear:
>
> reindex_relation(myrel, false, false, true, true, false, true,
> false, false, true);
>
> Unless you have an excellent memory, you can't tell what the heck
> that's doing without flipping back and forth between the function
> definition and the call site. With a bit-field, it's a lot easier to
> glance at the call site and have a clue what's going on. We're of
> course not quite to the point of that exaggerated example yet.

Agreed.

> > However, suppose we inverted both flags, say REINDEX_SKIP_CONSTRAINT_CHECKS and
> > REINDEX_ALLOW_OLD_INDEX_USE. ?Then, flags = 0 can hurt performance but not
> > correctness. ?That's looking like a win.
>
> I prefer the positive sense for those flags because I think it's more
> clear. There aren't so many call sites or so many people using this
> that we have to worry about what people are going to do in new calling
> locations; getting it right in any new code shouldn't be a
> consideration.

Okay. I've attached a new patch version based on that strategy.