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