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

I'm sorry I was unable to respond right away.

On 09.05.2023 17:23, torikoshia wrote:
> You may already understand it, but these variable names are given in
> imitation of FREEZE and BINARY cases:
>
>   --- a/src/include/commands/copy.h
>   +++ b/src/include/commands/copy.h
>   @@ -42,6 +42,7 @@ typedef struct CopyFormatOptions
>                                    * -1 if not specified */
>       bool        binary;         /* binary format? */
>       bool        freeze;         /* freeze rows on loading? */
>   +   bool        ignore_datatype_errors;  /* ignore rows with
> datatype errors */
>
>   --- a/src/backend/commands/copy.c
>   +++ b/src/backend/commands/copy.c
>   @@ -419,6 +419,7 @@ ProcessCopyOptions(ParseState *pstate,
>       bool        format_specified = false;
>       bool        freeze_specified = false;
>       bool        header_specified = false;
>   +   bool        ignore_datatype_errors_specified = false;
>
> I think it would be sane to align the names with the FREEZE and BINARY
> options.
>
> I agree with the name is too long and we once used the name
> 'ignore_errors'.
> However, current implementation does not ignore all errors but just
> data type error, so I renamed it.
> There may be a better name, but I haven't come up with one.

Yes, you are right, I saw it.

>
> As far as I take a quick look at on PostgreSQL source code, there're
> few variable name with "_counter". It seems to be used for function names.
> Something like "ignored_errors_count" might be better.
I noticed that many variables are named with the "_counter" postfix, and
most of them are used as a counter. For example, PgStat_StatTabEntry or
JitInstrumentation structures consisted of many such variables. Despite
this, I agree with your suggested name, because I found many similar
variables that are used in the program as a counter, but it seems to me
that the most of them are still used by local variables in the function.