Re: Pgoutput not capturing the generated columns - Mailing list pgsql-hackers
From | vignesh C |
---|---|
Subject | Re: Pgoutput not capturing the generated columns |
Date | |
Msg-id | CALDaNm18G3mUPud2X_X8gWk0QPd6VSdmiU=QYqcoL5LAVcPKWA@mail.gmail.com Whole thread Raw |
In response to | Pgoutput not capturing the generated columns (Rajendra Kumar Dangwal <dangwalrajendra888@gmail.com>) |
Responses |
Re: Pgoutput not capturing the generated columns
|
List | pgsql-hackers |
On Tue, 8 Oct 2024 at 11:37, Shubham Khanna <khannashubham1197@gmail.com> wrote: > > On Fri, Oct 4, 2024 at 9:36 AM Peter Smith <smithpb2250@gmail.com> wrote: > > > > 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. > > > > I have fixed all the given comments. Also, I have created a new 0003 > patch for the TAP-Tests related to the '011_generated.pl' file. I am > planning to merge 0001 and 0003 patches once they will get fixed. > The attached patches contain the required changes. There is inconsistency in replication when a generated column is specified in the column list. The generated column data is not replicated during initial sync whereas it is getting replicated during incremental sync: -- publisher CREATE TABLE t1(c1 int, c2 int GENERATED ALWAYS AS (c1 * 2) STORED) INSERT INTO t1 VALUES (1); CREATE PUBLICATION pub1 for table t1(c1, c2); --subscriber CREATE TABLE t1(c1 int, c2 int) CREATE SUBSCRIPTION sub1 connection 'dbname=postgres host=localhost port=5432' PUBLICATION pub1; -- Generate column data is not synced during initial sync postgres=# select * from t1; c1 | c2 ----+---- 1 | (1 row) -- publisher INSERT INTO t1 VALUES (2); -- Whereas generated column data is synced during incremental sync postgres=# select * from t1; c1 | c2 ----+---- 1 | 2 | 4 (2 rows) Regards, Vignesh
pgsql-hackers by date: