Re: Mention column name in error messages

On Mon, Oct 17, 2016 at 12:48 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:

> *** /Users/mpaquier/git/postgres/src/test/regress/expected/xml_2.out
> Mon Oct 17 11:32:26 2016
> --- /Users/mpaquier/git/postgres/src/test/regress/results/xml.out
> Mon Oct 17 15:58:42 2016
> ***************
> *** 9,14 ****
> --- 9,15 ----
> LINE 1: INSERT INTO xmltest VALUES (3, '<wrong');
> ^
> DETAIL: line 1: Couldn't find end of Start Tag wrong line 1
> + CONTEXT: coercion failed for column "data" of type xml
> SELECT * FROM xmltest;
> id | data
> ----+--------------------
> make check is still broken here. You did not update the expected
> output used when build is done with the --with-libxml switch. It would
> be good to check other cases as well.
>
> On top of that, I have found that a couple of other regression tests
> are broken in contrib/, citext being one.
I've also tested with the patch. As Michael already pointed out, you
should update the expected output for citext and xml regression tests.

>
> + /* Set up callback to identify error line number. */
> + errcallback.callback = TransformExprCallback;
> Er, no. You want to know at least column name here, not a line number
>
Please change the comment accordingly.

> The context message is specifying only the column type and name. I
> think that it would be useful to provide as well the relation name.
> Imagine for example the case of a CTE using an INSERT query...
> Providing the query type would be also useful I think. You can look at
> state->p_is_insert for that. In short the context message could just
> have this shape:
> CONTEXT { INSERT | UPDATE } relname, column "col", type coltype.
>
+1 for providing relation name in the context message.

--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com