Hi,
On Fri, Dec 19, 2025 at 12:32:49PM +0530, Ashutosh Bapat wrote:
> On Thu, Dec 18, 2025 at 11:52 PM Bertrand Drouvot
> <[email protected]> wrote:
> >
> > I think that my example was confusing due to "size_t *bytes_filtered". I
> > think
> > that what we could do is something like:
> >
> > "
> > typedef void (*LogicalDecodeReportStatsCB)(
> > LogicalDecodingContext *ctx,
> > LogicalDecodeEventType event_type,
> > bool *filtered,
> > bool *txn_sent);
> > "
> >
> > Note that there is no more size_t.
> >
>
> Thanks for the clarification. It fixes the problem of filteredBytes
> divergence. Since the core is calling stats callback, the problem of
> plugin not calling the function at appropriate places is also not
> there.
Yeah.
> IIUC, it still has some problems from the previous solution and
> some new problems as explained below.
>
> > Then for, for example in change_cb_wrapper(), we could do:
> >
> > "
> > ctx->callbacks.change_cb(ctx, txn, relation, change);
> >
> > if (ctx->callbacks.report_stats_cb)
> > {
> > bool filtered = false;
> >
> > ctx->callbacks.report_stats_cb(ctx, LOGICALDECODE_CHANGE,
> > &filtered,
> > NULL);
> >
> > if (filtered)
> > cache->filteredBytes += ReorderBufferChangeSize(change);
> > }
> > "
> >
> > The plugin would need to "remember" that it filtered (so that it can
> > reply to the callback). It could do that by adding say
> > "last_event_filtered" to
> > it's output_plugin_private structure.
>
>
> Why does the core send NULL for the second parameter? Does the output
> plugin have to take care of NULL references too?
It was just a quick example. I was more focused on demonstrating the concept
than
the exact API details.
>
> I think the core will end up calling this or similar stanza at every
> callback since it won't know when the output plugin will have
> statistics to report.
Yes.
> That's more complexity and wasted CPU cycles in core.
I think that should be negligible as compared to what the logical decoding is
already doing at those places.
> > That's more work on the plugin side and we would probably need to provide
> > some
> > examples from our side.
>
> Andres is objecting to this exact thing. IIUC, the code changes there
> were far simpler than this proposal. Am I missing something?
You are right. My main motivation with this idea was to avoid the APIs break.
But maybe that's not worth it.
> I don't think there will be an output plugin which wouldn't want to
> take advantage of the statistics. The easier it is for them to adopt
> the statistics, as is with my proposal, the better. With this proposal
> output plugins have to do more work if they want to support
> statistics. That itself will create a barrier for them to adopt the
> statistics. We want the output plugins to support statistics so that
> users can benefit. Let's make it easier for the output plugins to
> implement them.
That was the main point. With your proposal, the APIs break will occur (and so
the plugin will need some changes) even if they don't want the stats. But, if
we are confident that most (all?) would want to use it, then I agree that your
proposal is better and that's fine by me to move forward with yours.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
