From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Shubham Khanna <khannashubham1197(at)gmail(dot)com> |
Cc: | vignesh C <vignesh21(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Rajendra Kumar Dangwal <dangwalrajendra888(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, euler(at)eulerto(dot)com |
Subject: | Re: Pgoutput not capturing the generated columns |
Date: | 2024-10-04 04:05:52 |
Message-ID: | CAHut+Ps9ksUQsRNWJtAj-SrMqfQq4pixfA2DSNOU3+nk_hPmTg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Shubham, here are my review comments for v36-0001.
======
1. General - merge patches
It is long past due when patches 0001 and 0002 should've been merged.
AFAIK the split was only because historically these parts had
different authors. But, keeping them separated is not helpful anymore.
======
src/backend/catalog/pg_publication.c
2.
Bitmapset *
-pub_collist_validate(Relation targetrel, List *columns)
+pub_collist_validate(Relation targetrel, List *columns, bool pubgencols)
Since you removed the WARNING, this parameter 'pubgencols' is unused
so it should also be removed.
======
src/backend/replication/pgoutput/pgoutput.c
3.
/*
- * If the publication is FOR ALL TABLES then it is treated the same as
- * if there are no column lists (even if other publications have a
- * list).
+ * To handle cases where the publish_generated_columns option isn't
+ * specified for all tables in a publication, we must create a column
+ * list that excludes generated columns. So, the publisher will not
+ * replicate the generated columns.
*/
- if (!pub->alltables)
+ if (!(pub->alltables && pub->pubgencols))
I still found that comment hard to understand. Does this mean to say
something like:
------
Process potential column lists for the following cases:
a. Any publication that is not FOR ALL TABLES.
b. When the publication is FOR ALL TABLES and
'publish_generated_columns' is false.
A FOR ALL TABLES publication doesn't have user-defined column lists,
so all columns will be replicated by default. However, if
'publish_generated_columns' is set to false, column lists must still
be created to exclude any generated columns from being published
------
======
src/test/regress/sql/publication.sql
4.
+SET client_min_messages = 'WARNING';
+CREATE TABLE gencols (a int, gen1 int GENERATED ALWAYS AS (a * 2) STORED);
AFAIK you don't need to keep changing 'client_min_messages',
particularly now that you've removed the WARNING message that was
previously emitted.
~
5.
nit - minor comment changes.
======
Please refer to the attachment which implements any nits from above.
======
Kind Regards,
Peter Smith.
Fujitsu Austrlia.
Attachment | Content-Type | Size |
---|---|---|
PS_NITPICKS_GENCOLS_v360001.txt | text/plain | 4.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Smith | 2024-10-04 04:12:55 | Re: Pgoutput not capturing the generated columns |
Previous Message | Michael Paquier | 2024-10-04 03:55:45 | Re: Rename PageData to XLogPageData |