Re: Proposal: Filter irrelevant change before reassemble transactions during logical decoding

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Ajin Cherian <itsajin(at)gmail(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Proposal: Filter irrelevant change before reassemble transactions during logical decoding
Date: 2025-04-23 03:49:01
Message-ID: CAHut+Puv_RigeDAQNFHPAYAEPYP3pY69ZLwYZCtA58m2eFQP+Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Ajin.

Here are some v17-0003 review comments (aka some v16-0003 comments
that were not yet addressed or rejected)

On Fri, Apr 11, 2025 at 4:05 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
...
> ======
> Commit message
>
> 2. missing commit message

Not yet addressed.

> ~~~
>
> 8.
> # Insert, delete, and update tests for restricted publication tables
> $log_location = -s $node_publisher->logfile;
> $node_publisher->safe_psql('postgres', "INSERT INTO insert_only_table
> VALUES (1, 'to be inserted')");
> $node_publisher->safe_psql('postgres', "UPDATE insert_only_table SET
> data = 'updated' WHERE id = 1");
> $logfile = slurp_file($node_publisher->logfile, $log_location);
> ok($logfile =~ qr/Filtering UPDATE/,
> 'unpublished UPDATE is filtered');
>
> $log_location = -s $node_publisher->logfile;
> $node_publisher->safe_psql('postgres', "DELETE FROM insert_only_table
> WHERE id = 1");
> $logfile = slurp_file($node_publisher->logfile, $log_location);
> ok($logfile =~ qr/Filtering DELETE/,
> 'unpublished DELETE is filtered');
>
> $log_location = -s $node_publisher->logfile;
> $node_publisher->safe_psql('postgres', "INSERT INTO delete_only_table
> VALUES (1, 'to be deleted')");
> $logfile = slurp_file($node_publisher->logfile, $log_location);
> ok($logfile =~ qr/Filtering INSERT/,
> 'unpublished INSERT is filtered');
>
> ~
...
> 8b.
> Change the comment or rearrange the tests so they are in the same
> order as described
>

The comment was changed, and now says "Insert, update, and delete
tests ..." but still, the "Filtering INSERT" test is last. IMO, that
test should come first to match the comment.

> ~
>
> 8c.
> Looking at the expected logs I wondered if it might be nicer for the
> pgoutput_filter_change doing this logging to also emit the relation
> name.
>

Not yet addressed

======
Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-04-23 04:55:31 Re: Recent pg_rewind test failures in buildfarm
Previous Message Peter Smith 2025-04-23 03:11:29 Re: Proposal: Filter irrelevant change before reassemble transactions during logical decoding