Hi,
On Wed, Sep 24, 2025 at 05:28:44PM +0530, Ashutosh Bapat wrote:
> On Wed, Sep 24, 2025 at 2:38 PM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > On Wed, Sep 24, 2025 at 12:47 PM Ashutosh Bapat
> > <ashutosh.bapat.oss@gmail.com> wrote:
> > >
> > > On Wed, Sep 24, 2025 at 10:12 AM shveta malik <shveta.malik@gmail.com> wrote:
> > > >
> > > > I tested the flows with
> > > > a) logical replication slot and get-changes.
> > > > b) filtered data flows: pub-sub creation with row_filters, 'publish'
> > > > options. I tried to verify plugin fields as compared to total_wal*
> > > > fields.
> > > > c) reset flow.
> > > >
> > > > While tests for a and c are present already. I don't see tests for b
> > > > anywhere when it comes to stats. Do you think we shall add a test for
> > > > filtered data using row-filter somewhere?
> > >
> > > Added a test in 028_row_filter. Please find it in the attached
> > > patchset.
> >
> > Test looks good.
>
> Thanks. Added to three more files. I think we have covered all the
> cases where filtering can occur.
>
> PFA patches.
Thanks for the new version!
A few random comments:
=== 1
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>plugin_filtered_bytes</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Amount of changes, from <structfield>total_wal_bytes</structfield>, filtered
+ out by the output plugin and not sent downstream. Please note that it
+ does not include the changes filtered before a change is sent to
+ the output plugin, e.g. the changes filtered by origin. The count is
+ maintained by the output plugin mentioned in
+ <structfield>plugin</structfield>.
I found "The count" somehow ambiguous. What about "This statistic" instead?
=== 2
+ subtransactions. These transactions are subset of transctions sent to
s/transctions/transactions
=== 3
+ the decoding plugin. Hence this count is expected to be lesser than or
s/be lesser/be less/? (not 100% sure)
=== 4
+extern Size ReorderBufferChangeSize(ReorderBufferChange *change);
Another approach could be to pass the change's size as an argument to the
callbacks? That would avoid to expose ReorderBufferChangeSize publicly.
=== 5
ctx->output_plugin_private = data;
+ ctx->stats = palloc0(sizeof(OutputPluginStats));
I was wondering if we need to free this in pg_decode_shutdown, but it looks
like it's done through FreeDecodingContext() anyway.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
