public inbox for [email protected]
help / color / mirror / Atom feedFrom: Matheus Alcantara <[email protected]>
To: jian he <[email protected]>
Cc: torikoshia <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Jim Jones <[email protected]>
Cc: Kirill Reshke <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: Yugo NAGATA <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Change COPY ... ON_ERROR ignore to ON_ERROR ignore_row
Date: Mon, 9 Feb 2026 12:36:55 -0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CACJufxGUP09S7a2akQ4GD62-Rf2bcV52cqnw4qqBOXcn9bmxCA@mail.gmail.com>
References: <CAKFQuwawy1e6YR4S=j+y7pXqg_Dw1WBVrgvf=BP3d1_aSfe_+Q@mail.gmail.com>
<CACJufxF6_YwAboiCaVYLRtNpO4kGbXqkXzH_7W=pUvrNXK8WuQ@mail.gmail.com>
<CAD21AoBWwEzpp3Z6tp-O-AQGftK-kuj6vRva2L5daWoWBtbnRg@mail.gmail.com>
<CACJufxGEHmijmP-QqvrmqU6cxmhgpdjY7ewQBQ=E9NmdyEcqmw@mail.gmail.com>
<[email protected]>
<CACJufxG=em0PHZvy1EAZ+vxPZ8UA68MfQ-Hji+h+WgnWNpqmVQ@mail.gmail.com>
<CACJufxF0c3k5O8up9NOY-m02nyJ0f6N1tKxZwjCewTqvvFmbLw@mail.gmail.com>
<CACJufxHr-LBV2pB9m64mA=1pgVMM4LvUn+-MzpeViWV=Ks_cyg@mail.gmail.com>
<[email protected]>
<CACJufxEs_VBV39gYHnpLFOMcUaUD-ADAst_ePTCgmoDR8O=ekg@mail.gmail.com>
<[email protected]>
<CACJufxGYPXQ_Jz1avF5eSh_XJRsxhPSUZ+=RzG3Hz4_XNAc32g@mail.gmail.com>
<[email protected]>
<CACJufxHFwQMw1As+QFk+fA7S8ZxRG2wOvHcvmsWuj2XJ+W6d_A@mail.gmail.com>
<[email protected]>
<CACJufxGUP09S7a2akQ4GD62-Rf2bcV52cqnw4qqBOXcn9bmxCA@mail.gmail.com>
On 09/02/26 00:59, jian he wrote:
>> Thanks, overall the patch looks good to me. I'm attaching a diff with
>> just some small tweaks on documentation and error messages. Please see
>> and check if it's make sense.
>>
> In the function CopyFrom, we have:
> if (cstate->opts.on_error == COPY_ON_ERROR_IGNORE &&
> cstate->escontext->error_occurred)
> {
> cstate->escontext->error_occurred = false;
> pgstat_progress_update_param(PROGRESS_COPY_TUPLES_SKIPPED,
> cstate->num_errors);
>
> That means PROGRESS_COPY_TUPLES_SKIPPED applied for COPY_ON_ERROR_IGNORE only.
> So
> <para>
> Number of tuples skipped because they contain malformed data.
> This counter only advances when
> <literal>ignore</literal> is specified to the <literal>ON_ERROR</literal>
> option.
> </para></entry>
> should be ok.
>
Ok, agree.
>> I'm wondering if we should have an else if block on
>> CopyFromTextLikeOneRow() when cstate->cur_attval is NULL to handle
>> COPY_ON_ERROR_SET_NULL when log_verbosity is set to
>> COPY_LOG_VERBOSITY_VERBOSE
>>
>> if (cstate->opts.on_error == COPY_ON_ERROR_IGNORE)
>> ereport(NOTICE,
>> errmsg("skipping row due to data type incompatibility at line %" PRIu64 " for column \"%s\": null input",
>> cstate->cur_lineno,
>> cstate->cur_attname));
>> + else if (cstate->opts.on_error == COPY_ON_ERROR_SET_NULL)
>> + ereport(NOTICE,
>> + errmsg("setting to null due to data type incompatibility at line %" PRIu64 " for column \"%s\": null input",
>> + cstate->cur_lineno,
>> + cstate->cur_attname));
>>
>
> CopyFromTextLikeOneRow, we have:
> cstate->cur_attname = NameStr(att->attname);
> cstate->cur_attval = string;
>
> even if "string" is NULL (two InputFunctionCallSafe function call with
> "str" value as NULL), it will fail at
> ```
> else if (string == NULL)
> ereport(ERROR,
> errcode(ERRCODE_NOT_NULL_VIOLATION),
> errmsg("null value in column \"%s\"
> violates not-null constraint of domain %s",
> cstate->cur_attname,
> format_type_be(typioparams[m])),
> errdatatype(typioparams[m]));
> ```
> so i think condition like:
> if (cstate->opts.log_verbosity == COPY_LOG_VERBOSITY_VERBOSE &&
> cstate->cur_attval == NULL &&
> cstate->opts.on_error == COPY_ON_ERROR_SET_NULL)
> is not reachable.
> therefore I didn't add the ELSE IF block.
>
Ok, make sense. I've tested and it seems correct.
> inspired by your change, I further simplified the error handling code.
>
Thanks for the new version. It looks good to me. I don't have any
other comments.
--
Matheus Alcantara
EDB: https://www.enterprisedb.com
view thread (15+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Change COPY ... ON_ERROR ignore to ON_ERROR ignore_row
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox