On Tue, Jan 17, 2023 at 3:34 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
> > > > I am a bit worried about the indirections that the wrappers and hooks > > create. Output plugins call OutputPluginUpdateProgress() in callbacks > > but I don't see why ReorderBufferProcessTXN() needs a callback to > > call OutputPluginUpdateProgress. > > > > Yeah, I think we can do it as we are doing the previous approach but > we need an additional wrapper (update_progress_cb_wrapper()) as the > current patch has so that we can add error context information. This > is similar to why we have a wrapper for all other callbacks like > change_cb_wrapper. > Ultimately OutputPluginUpdateProgress() will be called - which in turn will call ctx->update_progress. I don't see wrappers around OutputPluginWrite or OutputPluginPrepareWrite. But I see that those two are called always from output plugin, so indirectly those are called through a wrapper. I also see that update_progress_cb_wrapper() is similar, as far as wrapper is concerned, to ReorderBufferUpdateProgress() in the earlier patch. ReorderBufferUpdateProgress() looks more readable than the wrapper. If we want to keep the wrapper at least we should use a different variable name. update_progress is also there LogicalDecodingContext and will be indirectly called from ReorderBuffer::update_progress. Somebody might think that there's some recursion involved there. That's a mighty confusion. -- Best Wishes, Ashutosh Bapat
