[Review] Include detailed information about a row failing a CHECK constraint into the error message

From: Royce Ausburn <royce(dot)ml(at)inomial(dot)com>
To: Jan Kundrát <jkt(at)flaska(dot)net>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: [Review] Include detailed information about a row failing a CHECK constraint into the error message
Date: 2011-11-16 22:13:33
Message-ID: A48A1365-4788-45F9-9854-EE8CD476445E@inomial.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


The patch applies cleanly to the current git master and is in context diff format.

The patch fails the regression tests because it is outputting new DETAIL line which four of tests aren't expecting. The tests will need to be updated.

Functionality:
The patch works as advertised. An insert or update that fails a check condition indeed logs the row that has failed:

test=# create table test (
test(# a integer,
test(# b integer CHECK (b > 2),
test(# c text,
test(# d integer
test(#
test(#
test(# );
CREATE TABLE
test=#
test=# insert into test select 1, 2, 'Test', 4;
ERROR: new row for relation "test" violates check constraint "test_b_check"
DETAIL: Failing row: (1, 2, Test, 4).

One comment I have on the output is that strings are not in quotes. It's a little jarring, but might not be that big a deal. A contrived case that is pretty confusing:

test=# insert into test select 1, 2, '3, 4', 4;
ERROR: new row for relation "test" violates check constraint "test_b_check"
DETAIL: Failing row: (1, 2, 3, 4, 4).

A select inserting 4 columns seemingly results in a 5 column row ;)

Another super minor thing, postgres doesn't seem to put periods at the end of log messages, yet this new detail line does.

Code

I'm no C or postgres expert, but the code looks okay to me.

Attached is a small script I used to test/demo the functionality.

--Royce

On 11/11/2011, at 2:56 AM, Jan Kundrát wrote:

> On 11/10/11 16:05, Tom Lane wrote:
>> I agree with Jan that this is probably useful; I'm pretty sure there
>> have been requests for it before. We just have to make sure that the
>> length of the message stays in bounds.
>>
>> One tip for keeping the length down: there is no value in repeating
>> information from the primary error message, such as the name of the
>> constraint.
>
> Thanks to your comments and suggestions, I appreciate the time of the
> reviewers.
>
> Attached is a second version of this patch which keeps the size of the
> output at 64 characters per column (which is an arbitrary value defined
> as a const int, which I hope matches your style). Longer values have
> their last three characters replaced by "...", so there's no way to
> distinguish them from a legitimate string that ends with just that.
> There's also no escaping of special-string values, similar to how the
> BuildIndexValueDescription operates.
>
> Cheers,
> Jan
>
> --
> Trojita, a fast e-mail client -- http://trojita.flaska.net/
> <context_in_check_constraints-v2.patch>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2011-11-16 22:15:29 Re: Minor optimisation of XLogInsert()
Previous Message Tom Lane 2011-11-16 21:41:11 Re: declarations of range-vs-element <@ and @>