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

Поиск
Список
Период
Сортировка
От Royce Ausburn
Тема [Review] Include detailed information about a row failing a CHECK constraint into the error message
Дата
Msg-id A48A1365-4788-45F9-9854-EE8CD476445E@inomial.com
обсуждение исходный текст
Ответ на Re: Re: [patch] Include detailed information about a row failing a CHECK constraint into the error message  (Jan Kundrát <jkt@flaska.net>)
Ответы Re: [Review] Include detailed information about a row failing a CHECK constraint into the error message  (Jan Kundrát <jkt@flaska.net>)
Re: [Review] Include detailed information about a row failing a CHECK constraint into the error message  (Tom Lane <tgl@sss.pgh.pa.us>)
Список 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>

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: declarations of range-vs-element <@ and @>
Следующее
От: Simon Riggs
Дата:
Сообщение: Re: Minor optimisation of XLogInsert()