Horton

Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)

torikoshia <torikoshia@oss.nttdata.com>

View thread
From: torikoshia <torikoshia@oss.nttdata.com>
To: jian he <jian.universality@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Alena Rybakina <lena.ribackina@yandex.ru>, Damir Belyalov <dam.bel07@gmail.com>, zhihuifan1213@163.com, Tom Lane <tgl@sss.pgh.pa.us>, Daniel Gustafsson <daniel@yesql.se>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>, anisimow.d@gmail.com, HukuToc@gmail.com, pgsql-hackers@lists.postgresql.org, Andrei Lepikhov <a.lepikhov@postgrespro.ru>
Date: 2024-01-09T14:36:37Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. doc: Fix COPY ON_ERROR option syntax synopsis.

  2. Disallow specifying ON_ERROR option without value.

  3. Rename COPY option from SAVE_ERROR_TO to ON_ERROR

  4. Fix spelling in notice

  5. Add new COPY option SAVE_ERROR_TO

Attachments

On Tue, Dec 19, 2023 at 10:14 AM Masahiko Sawada <sawada.mshk@gmail.com> 
wrote:
> If we want only such a feature we need to implement it together (the
> patch could be split, though). But if some parts of the feature are
> useful for users as well, I'd recommend implementing it incrementally.
> That way, the patches can get small and it would be easy for reviewers
> and committers to review/commit them.

Jian, how do you think this comment?

Looking back at the discussion so far, it seems that not everyone thinks 
saving table information is the best idea[1] and some people think just 
skipping error data is useful.[2]

Since there are issues to be considered from the design such as 
physical/logical replication treatment, putting error information to 
table is likely to take time for consensus building and development.

Wouldn't it be better to follow the following advice and develop the 
functionality incrementally?

On Fri, Dec 15, 2023 at 4:49 AM Masahiko Sawada 
<sawada(dot)mshk(at)gmail(dot)com> wrote:
> So I'm thinking we may be able to implement this
> feature incrementally. The first step would be something like an
> option to ignore all errors or an option to specify the maximum number
> of errors to tolerate before raising an ERROR. The second step would
> be to support logging destinations such as server logs and tables.


Attached a patch for this "first step" with reference to v7 patch, which 
logged errors and simpler than latest one.
- This patch adds new option SAVE_ERROR_TO, but currently only supports 
'none', which means just skips error data. It is expected to support 
'log' and 'table'.
- This patch Skips just soft errors and don't handle other errors such 
as missing column data.


BTW I have question and comment about v15 patch:

> +           {
> +               /*
> +               *
> +               * InputFunctionCall is more faster than 
> InputFunctionCallSafe.
> +               *
> +               */

Have you measured this?
When I tested it in an older patch, there were no big difference[3].

  > -   SAVEPOINT SCALAR SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY 
SELECT
  > +   SAVEPOINT SAVE_ERROR SCALAR SCHEMA SCHEMAS SCROLL SEARCH SECOND_P 
SECURITY SELECT

There was a comment that we shouldn't add new keyword for this[4].


I left as it was in v7 patch regarding these points.


[1] 
https://www.postgresql.org/message-id/20231109002600.fuihn34bjqqgmbjm%40awork3.anarazel.de
[2] 
https://www.postgresql.org/message-id/CAD21AoCeEOBN49fu43e6tBTynnswugA3oZ5AZvLeyDCpxpCXPg%40mail.gmail.com
[3] 
https://www.postgresql.org/message-id/19551e8c2717c24689913083f841ddb5%40oss.nttdata.com
[4] 
https://www.postgresql.org/message-id/20230322175000.qbdctk7bnmifh5an%40awork3.anarazel.de


-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation