Re: Pgoutput not capturing the generated columns
Peter Smith <smithpb2250@gmail.com>
Attachments
- PS_NITPICKS_GENCOLS_v360001.txt (text/plain)
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.
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Doc: Generated column replication.
- 6252b1eaf82b 18.0 landed
-
Rename pubgencols_type to pubgencols in pg_publication.
- 75eb9766ec20 18.0 landed
-
Doc: Fix column name in pg_publication catalog.
- 117f9f328e0f 18.0 landed
-
Fix buildfarm failure introduced by commit e65dbc9927.
- b35434b134b1 18.0 landed
-
Change publication's publish_generated_columns option type to enum.
- e65dbc9927fb 18.0 landed
-
Fix \dRp+ output when describing publications with a lower server version.
- 991974bb4888 18.0 landed
-
Replicate generated columns when 'publish_generated_columns' is set.
- 7054186c4ebe 18.0 landed
-
Doc: Update the behavior of generated columns in Logical Replication.
- 5b0c46ea0932 18.0 landed
-
Replicate generated columns when specified in the column list.
- 745217a051a9 18.0 landed
-
Doc: Generated columns are skipped for logical replication.
- 7a089f6e6a14 17.0 cited