Re: Pgoutput not capturing the generated columns

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Shubham Khanna <khannashubham1197(at)gmail(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Rajendra Kumar Dangwal <dangwalrajendra888(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "euler(at)eulerto(dot)com" <euler(at)eulerto(dot)com>
Subject: Re: Pgoutput not capturing the generated columns
Date: 2024-07-18 23:29:02
Message-ID: CAHut+PtoDDRHpboPVSFWAdqt_LKNHgLLmHsMFfG+2P1WgkPbgw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi, here are some review comments for patch v19-0003

======
src/backend/catalog/pg_publication.c

1.
/*
* Translate a list of column names to an array of attribute numbers
* and a Bitmapset with them; verify that each attribute is appropriate
* to have in a publication column list (no system or generated attributes,
* no duplicates). Additional checks with replica identity are done later;
* see pub_collist_contains_invalid_column.
*
* Note that the attribute numbers are *not* offset by
* FirstLowInvalidHeapAttributeNumber; system columns are forbidden so this
* is okay.
*/
static void
publication_translate_columns(Relation targetrel, List *columns,
int *natts, AttrNumber **attrs)

~

I though the above comment ought to change: /or generated
attributes/or virtual generated attributes/

IIRC this was already addressed back in v16, but somehow that fix has
been lost (???).

======
src/backend/replication/logical/tablesync.c

fetch_remote_table_info:
nitpick - missing end space in this comment /* TODO: use
ATTRIBUTE_GENERATED_VIRTUAL*/

======

2.
(in patch v19-0001)
+# tab3:
+# publisher-side tab3 has generated col 'b'.
+# subscriber-side tab3 has generated col 'b', using a different computation.

(here, in patch v19-0003)
# tab3:
-# publisher-side tab3 has generated col 'b'.
-# subscriber-side tab3 has generated col 'b', using a different computation.
+# publisher-side tab3 has stored generated col 'b' but
+# subscriber-side tab3 has DIFFERENT COMPUTATION stored generated col 'b'.

It has become difficult to review these TAP tests, particularly when
different patches are modifying the same comment. e.g. I post
suggestions to modify comments for patch 0001. Those get addressed OK,
only to vanish in subsequent patches like has happened in the above
example.

Really this patch 0003 was only supposed to add the word "stored", not
revert the entire comment to something from an earlier version. Please
take care that all comment changes are carried forward correctly from
one patch to the next.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2024-07-18 23:39:08 Re: Built-in CTYPE provider
Previous Message Thomas Simpson 2024-07-18 23:08:06 Enhance pg_dump multi-threaded streaming (WAS: Re: filesystem full during vacuum - space recovery issues)