Re: inconsistent comparison of CHECK constraints

Excerpts from Tom Lane's message of lun ene 16 12:44:57 -0300 2012:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > While reviewing Nikhil Sontakke's fix for the inherited constraints open
> > item we have, I noticed that MergeWithExistingConstraint and
> > MergeConstraintsIntoExisting are using rather different mechanism to
> > compare equality of the constraint expressions; the former does this:
>
> >         if (equal(expr, stringToNode(TextDatumGetCString(val))))
>
> > So plain string comparison of the node's string representation.
>
> No, that's *not* a "plain string comparison", and if it were it would be
> wrong.  This is doing equal() on the node trees, which is in fact the
> correct implementation.

Aha, that makes sense.

> > MergeConstraintsIntoExisting is instead doing this:
>
> >     if (acon->condeferrable != bcon->condeferrable ||
> >         acon->condeferred != bcon->condeferred ||
> >         strcmp(decompile_conbin(a, tupleDesc),
> >                decompile_conbin(b, tupleDesc)) != 0)
>
> That's kind of a crock, but it's necessary because it's trying to detect
> equivalence of constraint expressions belonging to different tables,
> which could have different physical column numbers as noted by the
> comment.  So I don't see a way to reduce it to a simple equal().
> But for constraints applicable to just one table, equal() should be
> preferred as it's simpler and more reliable.

It makes plenty of sense too.

I've left the two separate implementations alone.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support