Horton

Thread

Commits

  1. Add progress reporting of skipped tuples during COPY FROM.

  1. Add tuples_skipped to pg_stat_progress_copy

    torikoshia <torikoshia@oss.nttdata.com> — 2024-01-17T05:22:03Z

    Hi,

132de9968840c introduced SAVE_ERROR_TO option to COPY and enabled to 
skip malformed data, but there is no way to watch the number of skipped 
rows during COPY.

Attached patch adds tuples_skipped to pg_stat_progress_copy, which 
counts the number of skipped tuples because source data is malformed.
If SAVE_ERROR_TO is not specified, this column remains zero.

The advantage would be that users can quickly notice and stop COPYing 
when there is a larger amount of skipped data than expected, for 
example.

As described in commit log, it is expected to add more choices for 
SAVE_ERROR_TO like 'log' and using such options may enable us to know 
the number of skipped tuples during COPY, but exposed in 
pg_stat_progress_copy would be easier to monitor.


What do you think?

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation

  2. Re: Add tuples_skipped to pg_stat_progress_copy

    Masahiko Sawada <sawada.mshk@gmail.com> — 2024-01-17T05:47:49Z

    On Wed, Jan 17, 2024 at 2:22 PM torikoshia <torikoshia@oss.nttdata.com> wrote:
>
> Hi,
>
> 132de9968840c introduced SAVE_ERROR_TO option to COPY and enabled to
> skip malformed data, but there is no way to watch the number of skipped
> rows during COPY.
>
> Attached patch adds tuples_skipped to pg_stat_progress_copy, which
> counts the number of skipped tuples because source data is malformed.
> If SAVE_ERROR_TO is not specified, this column remains zero.
>
> The advantage would be that users can quickly notice and stop COPYing
> when there is a larger amount of skipped data than expected, for
> example.
>
> As described in commit log, it is expected to add more choices for
> SAVE_ERROR_TO like 'log' and using such options may enable us to know
> the number of skipped tuples during COPY, but exposed in
> pg_stat_progress_copy would be easier to monitor.
>
>
> What do you think?

+1

The patch is pretty simple. Here is a comment:

+       (if <literal>SAVE_ERROR_TO</literal> is specified, otherwise zero).
+      </para></entry>
+     </row>

To be precise, this counter only advances when a value other than
'ERROR' is specified to SAVE_ERROR_TO option.

Regards,

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

  3. Re: Add tuples_skipped to pg_stat_progress_copy

    torikoshia <torikoshia@oss.nttdata.com> — 2024-01-22T16:02:15Z

    On 2024-01-17 14:47, Masahiko Sawada wrote:
> On Wed, Jan 17, 2024 at 2:22 PM torikoshia <torikoshia@oss.nttdata.com> 
> wrote:
>> 
>> Hi,
>> 
>> 132de9968840c introduced SAVE_ERROR_TO option to COPY and enabled to
>> skip malformed data, but there is no way to watch the number of 
>> skipped
>> rows during COPY.
>> 
>> Attached patch adds tuples_skipped to pg_stat_progress_copy, which
>> counts the number of skipped tuples because source data is malformed.
>> If SAVE_ERROR_TO is not specified, this column remains zero.
>> 
>> The advantage would be that users can quickly notice and stop COPYing
>> when there is a larger amount of skipped data than expected, for
>> example.
>> 
>> As described in commit log, it is expected to add more choices for
>> SAVE_ERROR_TO like 'log' and using such options may enable us to know
>> the number of skipped tuples during COPY, but exposed in
>> pg_stat_progress_copy would be easier to monitor.
>> 
>> 
>> What do you think?
> 
> +1
> 
> The patch is pretty simple. Here is a comment:
> 
> +       (if <literal>SAVE_ERROR_TO</literal> is specified, otherwise 
> zero).
> +      </para></entry>
> +     </row>
> 
> To be precise, this counter only advances when a value other than
> 'ERROR' is specified to SAVE_ERROR_TO option.

Thanks for your comment and review!

Updated the patch according to your comment and option name change by 
b725b7eec.


BTW, based on this patch, I think we can add another option which 
specifies the maximum tolerable number of malformed rows.
I remember this was discussed in [1], and feel it would be useful when 
loading 'dirty' data but there is a limit to how dirty it can be.
Attached 0002 is WIP patch for this(I haven't added doc yet).

This may be better discussed in another thread, but any comments(e.g. 
necessity of this option, option name) are welcome.


[1] 
https://www.postgresql.org/message-id/752672.1699474336%40sss.pgh.pa.us

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation

  4. Re: Add tuples_skipped to pg_stat_progress_copy

    Masahiko Sawada <sawada.mshk@gmail.com> — 2024-01-24T08:05:29Z

    On Tue, Jan 23, 2024 at 1:02 AM torikoshia <torikoshia@oss.nttdata.com> wrote:
>
> On 2024-01-17 14:47, Masahiko Sawada wrote:
> > On Wed, Jan 17, 2024 at 2:22 PM torikoshia <torikoshia@oss.nttdata.com>
> > wrote:
> >>
> >> Hi,
> >>
> >> 132de9968840c introduced SAVE_ERROR_TO option to COPY and enabled to
> >> skip malformed data, but there is no way to watch the number of
> >> skipped
> >> rows during COPY.
> >>
> >> Attached patch adds tuples_skipped to pg_stat_progress_copy, which
> >> counts the number of skipped tuples because source data is malformed.
> >> If SAVE_ERROR_TO is not specified, this column remains zero.
> >>
> >> The advantage would be that users can quickly notice and stop COPYing
> >> when there is a larger amount of skipped data than expected, for
> >> example.
> >>
> >> As described in commit log, it is expected to add more choices for
> >> SAVE_ERROR_TO like 'log' and using such options may enable us to know
> >> the number of skipped tuples during COPY, but exposed in
> >> pg_stat_progress_copy would be easier to monitor.
> >>
> >>
> >> What do you think?
> >
> > +1
> >
> > The patch is pretty simple. Here is a comment:
> >
> > +       (if <literal>SAVE_ERROR_TO</literal> is specified, otherwise
> > zero).
> > +      </para></entry>
> > +     </row>
> >
> > To be precise, this counter only advances when a value other than
> > 'ERROR' is specified to SAVE_ERROR_TO option.
>
> Thanks for your comment and review!
>
> Updated the patch according to your comment and option name change by
> b725b7eec.

Thanks! The patch looks good to me. I'm going to push it tomorrow,
barring any objections.

>
>
> BTW, based on this patch, I think we can add another option which
> specifies the maximum tolerable number of malformed rows.
> I remember this was discussed in [1], and feel it would be useful when
> loading 'dirty' data but there is a limit to how dirty it can be.
> Attached 0002 is WIP patch for this(I haven't added doc yet).

Yeah, it could be a good option.

> This may be better discussed in another thread, but any comments(e.g.
> necessity of this option, option name) are welcome.

I'd recommend forking a new thread for this option. As far as I
remember, there also was an opinion that "reject limit" stuff is not
very useful.

Regards,

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

  5. Re: Add tuples_skipped to pg_stat_progress_copy

    torikoshia <torikoshia@oss.nttdata.com> — 2024-01-25T02:25:33Z

    On 2024-01-24 17:05, Masahiko Sawada wrote:
> On Tue, Jan 23, 2024 at 1:02 AM torikoshia <torikoshia@oss.nttdata.com> 
> wrote:
>> 
>> On 2024-01-17 14:47, Masahiko Sawada wrote:
>> > On Wed, Jan 17, 2024 at 2:22 PM torikoshia <torikoshia@oss.nttdata.com>
>> > wrote:
>> >>
>> >> Hi,
>> >>
>> >> 132de9968840c introduced SAVE_ERROR_TO option to COPY and enabled to
>> >> skip malformed data, but there is no way to watch the number of
>> >> skipped
>> >> rows during COPY.
>> >>
>> >> Attached patch adds tuples_skipped to pg_stat_progress_copy, which
>> >> counts the number of skipped tuples because source data is malformed.
>> >> If SAVE_ERROR_TO is not specified, this column remains zero.
>> >>
>> >> The advantage would be that users can quickly notice and stop COPYing
>> >> when there is a larger amount of skipped data than expected, for
>> >> example.
>> >>
>> >> As described in commit log, it is expected to add more choices for
>> >> SAVE_ERROR_TO like 'log' and using such options may enable us to know
>> >> the number of skipped tuples during COPY, but exposed in
>> >> pg_stat_progress_copy would be easier to monitor.
>> >>
>> >>
>> >> What do you think?
>> >
>> > +1
>> >
>> > The patch is pretty simple. Here is a comment:
>> >
>> > +       (if <literal>SAVE_ERROR_TO</literal> is specified, otherwise
>> > zero).
>> > +      </para></entry>
>> > +     </row>
>> >
>> > To be precise, this counter only advances when a value other than
>> > 'ERROR' is specified to SAVE_ERROR_TO option.
>> 
>> Thanks for your comment and review!
>> 
>> Updated the patch according to your comment and option name change by
>> b725b7eec.
> 
> Thanks! The patch looks good to me. I'm going to push it tomorrow,
> barring any objections.

Thanks!

>> 
>> BTW, based on this patch, I think we can add another option which
>> specifies the maximum tolerable number of malformed rows.
>> I remember this was discussed in [1], and feel it would be useful when
>> loading 'dirty' data but there is a limit to how dirty it can be.
>> Attached 0002 is WIP patch for this(I haven't added doc yet).
> 
> Yeah, it could be a good option.
> 
>> This may be better discussed in another thread, but any comments(e.g.
>> necessity of this option, option name) are welcome.
> 
> I'd recommend forking a new thread for this option. As far as I
> remember, there also was an opinion that "reject limit" stuff is not
> very useful.

OK, I'll make another thread for this.


-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation

  6. Re: Add tuples_skipped to pg_stat_progress_copy

    Masahiko Sawada <sawada.mshk@gmail.com> — 2024-01-25T05:57:32Z

    On Thu, Jan 25, 2024 at 11:25 AM torikoshia <torikoshia@oss.nttdata.com> wrote:
>
> On 2024-01-24 17:05, Masahiko Sawada wrote:
> > On Tue, Jan 23, 2024 at 1:02 AM torikoshia <torikoshia@oss.nttdata.com>
> > wrote:
> >>
> >> On 2024-01-17 14:47, Masahiko Sawada wrote:
> >> > On Wed, Jan 17, 2024 at 2:22 PM torikoshia <torikoshia@oss.nttdata.com>
> >> > wrote:
> >> >>
> >> >> Hi,
> >> >>
> >> >> 132de9968840c introduced SAVE_ERROR_TO option to COPY and enabled to
> >> >> skip malformed data, but there is no way to watch the number of
> >> >> skipped
> >> >> rows during COPY.
> >> >>
> >> >> Attached patch adds tuples_skipped to pg_stat_progress_copy, which
> >> >> counts the number of skipped tuples because source data is malformed.
> >> >> If SAVE_ERROR_TO is not specified, this column remains zero.
> >> >>
> >> >> The advantage would be that users can quickly notice and stop COPYing
> >> >> when there is a larger amount of skipped data than expected, for
> >> >> example.
> >> >>
> >> >> As described in commit log, it is expected to add more choices for
> >> >> SAVE_ERROR_TO like 'log' and using such options may enable us to know
> >> >> the number of skipped tuples during COPY, but exposed in
> >> >> pg_stat_progress_copy would be easier to monitor.
> >> >>
> >> >>
> >> >> What do you think?
> >> >
> >> > +1
> >> >
> >> > The patch is pretty simple. Here is a comment:
> >> >
> >> > +       (if <literal>SAVE_ERROR_TO</literal> is specified, otherwise
> >> > zero).
> >> > +      </para></entry>
> >> > +     </row>
> >> >
> >> > To be precise, this counter only advances when a value other than
> >> > 'ERROR' is specified to SAVE_ERROR_TO option.
> >>
> >> Thanks for your comment and review!
> >>
> >> Updated the patch according to your comment and option name change by
> >> b725b7eec.
> >
> > Thanks! The patch looks good to me. I'm going to push it tomorrow,
> > barring any objections.
>
> Thanks!

Pushed (commit 729439607).

Regards,

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