Re: Add new error_action COPY ON_ERROR "log"

On Wed, Mar 13, 2024 at 11:02 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Wed, Mar 13, 2024 at 11:09 AM Michael Paquier <michael@paquier.xyz> wrote:
> >
> > Hmm.  This NOTICE is really bugging me.  It is true that the clients
> > would get more information, but the information is duplicated on the
> > server side because the error context provides the same information as
> > the NOTICE itself:
> > NOTICE:  data type incompatibility at line 1 for column "a"
> > CONTEXT:  COPY aa, line 1, column a: "a"
> > STATEMENT:  copy aa from stdin with (on_error ignore, log_verbosity verbose);
>
> Yes, if wanted, clients can also get the CONTEXT - for instance, using
> '\set SHOW_CONTEXT always' in psql.
>
> I think we can enhance the NOTICE message to include the column value
> (just like CONTEXT message is showing) and leverage relname_only to
> emit only the relation name in the CONTEXT message.
>
>         /*
>          * We suppress error context information other than the relation name,
>          * if one of the operations below fails.
>          */
>         Assert(!cstate->relname_only);
>         cstate->relname_only = true;
>
> I'm attaching the v8 patch set implementing the above idea. With this,
> [1] is sent to the client, [2] is sent to the server log. This
> approach not only reduces the duplicate info in the NOTICE and CONTEXT
> messages, but also makes it easy for users to get all the necessary
> info in the NOTICE message without having to set extra parameters to
> get CONTEXT message.
>
> Another idea is to move even the table name to NOTICE message and hide
> the context with errhidecontext when we emit the new NOTICE messages.
>
> Thoughts?
>

The current approach, eliminating the duplicated information in
CONTEXT, seems good to me.

One question about the latest (v8) patch:

+                   else
+                       ereport(NOTICE,
+                               errmsg("data type incompatibility at
line %llu for column %s: null input",
+                                      (unsigned long long) cstate->cur_lineno,
+                                      cstate->cur_attname));
+

How can we reach this path? It seems we don't cover this path by the tests.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com