On Wed, 27 Nov 2024 at 08:50, Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Nov 27, 2024 at 3:30 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > > Hi, here are some review comments for patch v7-0001. > > > > ====== > > src/backend/replication/logical/relation.c > > > > logicalrep_report_missing_or_gen_attrs: > > > > 1. > > +/* > > + * If attempting to replicate missing or generated columns, report an > > error. > > + * Prioritize 'missing' errors if both occur though the prioritization is > > + * random. > > + */ > > > > That part "though the prioritization is random" seems wrongly worded > > because there is nothing random here. > > > > I guess the intention was something like "This prioritization design > > choice was arbitrary.", but TBH it may be better not to give any > > reason at all. > > > > I think we should give a reason so that if we come across any scenario > or add another one in the future, it will be easier to make the > decision. I'll change the patch to use 'arbitrary' instead of random.
There is a buildfarm failure in [1]. One of the new tests added to verify the log for the "incompatible generated columns" issue was incorrect. Specifically, the check qr/ERROR: ( [A-Z0-9]:) should have been updated to qr/ERROR: ( [A-Z0-9]+:), which is consistent with similar checks elsewhere in the codebase. The attached patch contains the necessary changes to address this issue. https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2024-11-27%2004%3A17%3A03 Regards, Vignesh
diff --git a/src/test/subscription/t/011_generated.pl b/src/test/subscription/t/011_generated.pl index 66e6d8da5a..b1b87cf85e 100644 --- a/src/test/subscription/t/011_generated.pl +++ b/src/test/subscription/t/011_generated.pl @@ -361,7 +361,7 @@ $node_subscriber->safe_psql( # Verify that an error occurs. my $offset = -s $node_subscriber->logfile; $node_subscriber->wait_for_log( - qr/ERROR: ( [A-Z0-9]:)? logical replication target relation "public.t1" has incompatible generated columns: "c2", "c3"/, + qr/ERROR: ( [A-Z0-9]+:)? logical replication target relation "public.t1" has incompatible generated columns: "c2", "c3"/, $offset); # cleanup
