I'm confused. Why does ExecBuildSlotValueDescription() return an empty string instead of NULL? There doesn't seem to be any value left in that idea, and returning NULL would make the callsites slightly simpler as well. (Also, I think the comment on top of it should be updated to reflect the permissions-related issues.)
It seems that the reason for this is to be consistent with BuildIndexValueDescription, which seems to do the same thing to simplify the stuff going on at check_exclusion_constraint() -- by returning an empty string, that code doesn't need to check which of the returned values is empty, only whether both are. That seems an odd choice to me; it seems better to me to be specific, i.e. include each of the %s depending on whether the returned string is null or not. You would have three possible different errdetails, but that seems a good thing anyway. (Also, ISTM the "if (!strlen(foo))" idiom should be avoided because it is confusing; better test explicitely for zero or nonzero. Anyway if you change the functions to return NULL, you can test for NULL-ness easily and there's no possible confusion anymore.) -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
