Subject: Re: bug: copy progress reporting of backends which run multiple COPYs

From: Matthias van de Meent <[email-masked]>
Date: Sat, 21 Jan 2023 01:51:28 +0100

On Thu, 19 Jan 2023 at 06:47, Justin Pryzby <[email-masked]> wrote:
>
> pg_stat_progress_copy was added in v14 (8a4f618e7, 9d2d45700).
>
> But if a command JOINs file_fdw tables, the progress report gets bungled
> up.  This will warn/assert during file_fdw tests.

I don't know what to do with that other than disabling COPY progress
reporting for file_fdw, i.e. calls to BeginCopyFrom that don't supply
a pstate. This is probably the best option, because a table backed by
file_fdw would also interfere with COPY TO's progress reporting.

Attached a patch that solves this specific issue in a
binary-compatible way. I'm not super happy about relying on behavior
of callers of BeginCopyFrom (assuming that users that run copy
concurrently will not provide a ParseState* to BeginCopyFrom), but it
is what it is.

Kind regards,

Matthias van de Meent

---

From: Ted Yu <[email-masked]>
Date: Fri, 20 Jan 2023 17:03:43 -0800

> concurrently will not provide a ParseState* to BeginCopyFrom), but it
Hi,
In `BeginCopyFrom`, I see the following :

        if (pstate)
        {
                cstate->range_table = pstate->p_rtable;
                cstate->rteperminfos = pstate->p_rteperminfos;

Is it possible to check range_table / rteperminfos so that we don't
introduce the bool field ?

Cheers

---

From: Matthias van de Meent <[email-masked]>
Date: Sat, 21 Jan 2023 02:22:12 +0100

On Sat, 21 Jan 2023 at 02:04, Ted Yu <[email-masked]> wrote:
>
>
>
> On Fri, Jan 20, 2023 at 4:51 PM Matthias van de Meent <[email-masked]> wrote:
>>
> Is it possible to check range_table / rteperminfos so that we don't introduce the bool field ?

I think yes, but I'm not sure we can depend on rteperminfos to be set,
and the same for p_rtable. I also don't think it's a good idea for
code clarity: there is no good reason why the (un)availability of
either range_table or rteperminfos would allow progress reporting - it
would require additional extensive documentation around both the usage
sites and the field itself. Adding a well-named field provides a much
better experience in my opinion.

If someone were opposed to adding that field in backbranches I'm fine
with using one of these instead, assuming additional clear
documentation is added as well.

- Matthias