On 11/16/11 23:13, Royce Ausburn wrote:
> 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.

Hi Royce, thanks for your time which you've put into this review.

What is the suggested way to go form here? Shall I update the unit tests?

> 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 ;)

Yes, I agree that the unescaped format of strings leads to ambiguous
results here. The code was copy-pasted from the checks which handle the
UNIQUE constraints, so if there's an obvious improvement, it should
probably be applied in there as well.

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

Again, I'm not familiar with the correct procedure. Shall I send a
revised patch for this one?

With kind regards,
Jan

-- 
Trojita, a fast e-mail client -- http://trojita.flaska.net/

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to