On Sat, Jan 21, 2023 at 02:45:40AM +0100, Matthias van de Meent wrote:
> Let me think about it. I think it would work, but I'm not sure that's
> a great option - it adds backend state that we would need to add
> cleanup handles for. But then again, COPY ... TO could use TRIGGER to
> trigger actual COPY FROM statements, which would also break progress
> reporting in a vanilla instance without extensions.
>
> I'm not sure what the right thing to do is here.
Simply disabling COPY reporting for file_fdw does not sound appealing
to me, because that can be really useful for users. As long as you
rely on two code paths that call separately the progress reporting,
things are doomed with the current infrastructure. For example,
thinking about some fancy cases, you could you create an index that
uses a function as expression, function that includes utility commands
that do progress reporting. Things would equally go wrong in the
progress view.
What are the assertions/warnings that get triggered in file_fdw?
Joining together file_fdw with a plain COPY is surely a fancy case,
even if COPY TO would allow that.
>> Would you do anything different in the master branch, with no
>> compatibility constraints ? I think the progress reporting would still
>> be limited to one row per backend, not one per CopyFrom().
>
> I think I would at least introduce another parameter to BeginCopyFrom
> for progress reporting (instead of relying on pstate != NULL), like
> how we have a bit in reindex_index's params->options that specifies
> whether we want progress reporting (which is unset for parallel
> workers iirc).
This makes sense, independently of the discussion about what should be
done with cross-runs of these APIs. This could be extended with
user-controllable options for each one of them. It does not take care
of the root of the problem, just bypasses it.
--
Michael