On Mon, Jan 19, 2015 at 11:05:22AM -0500, Stephen Frost wrote:
> One remaining question is about single-column key violations. Should we
> special-case those and allow them to be shown or no? I can't see a
> reason not to currently but I wonder if we might have cause to act
> differently in the future (not that I can think of a reason we'd ever
> need to).
Keep them hidden. Attempting to delete a PK row should not reveal
otherwise-inaccessible values involved in any constraint violation. It's
tempting to make an exception for single-column UNIQUE constraints, but some
of the ensuing data disclosures are questionable. What if the violation arose
from a column default expression or from index corruption?
On Mon, Jan 19, 2015 at 11:46:35AM -0500, Stephen Frost wrote:
> Right, ExecBuildSlotValueDescription() was made to be consistent with
> BuildIndexValueDescription. The reason for BuildIndexValueDescription
> returning an empty string is different from why you hypothosize though-
> it's exported and I was a bit worried that existing external callers
> might not be prepared for it to return a NULL instead of a string of
> some kind. If this was a green field instead of a back-patch fix, I'd
> certainly return NULL instead.
>
> If others feel that's not a good reason to avoid returning NULL, I can
> certainly change it around.
I won't lose sleep if it does return "" for that reason, but I'm relatively
unconcerned about extension API compatibility here. That function is called
from datatype-independent index uniqueness checks. This mailing list has
discussed the difficulties of implementing an index access method in an
extension, and no such extension has come forward.
Your latest patch has whitespace errors; visit "git diff --check".