inconsistent comparison of CHECK constraints

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:
   {       Datum        val;       bool        isnull;
       val = fastgetattr(tup,                         Anum_pg_constraint_conbin,
conDesc->rd_att,&isnull);       if (isnull)           elog(ERROR, "null conbin for rel %s",
RelationGetRelationName(rel));      if (equal(expr, stringToNode(TextDatumGetCString(val))))           found = true;
}

So plain string comparison of the node's string representation.

MergeConstraintsIntoExisting is instead doing this:
   if (acon->condeferrable != bcon->condeferrable ||       acon->condeferred != bcon->condeferred ||
strcmp(decompile_conbin(a,tupleDesc),              decompile_conbin(b, tupleDesc)) != 0) 

where decompile_conbin is defined roughly as
   expr = DirectFunctionCall2(pg_get_expr, attr,                              ObjectIdGetDatum(con->conrelid));
returnTextDatumGetCString(expr); 

So it is first decompiling the node into its source representation, then
comparing that.


Do we care about this?  If so, which version is preferrable?

I also noticed that MergeConstraintsIntoExisting is doing a scan on
conrelid and then manually filtering for conname, which seems worse than
the other code that's just using conname/connamespace as scankey.  This
is probably better on tables with tons of constraints.

--
Álvaro Herrera <alvherre@alvh.no-ip.org>