Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features) - Mailing list pgsql-hackers

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