Hi Shubham,
As you can see, most of my recent review comments for patch 0001 are
only cosmetic nitpicks. But, there is still one long-unanswered design
question from a month ago [1, #G.2]
A lot of the patch code of pgoutput.c and proto.c and logicalproto.h
is related to the introduction and passing everywhere of new
'include_generated_columns' function parameters. These same functions
are also always passing "BitMapSet *columns" representing the
publication column list.
My question was about whether we can't make use of the existing BMS
parameter instead of introducing all the new API parameters.
The idea might go something like this:
* If 'include_generated_columns' option is specified true and if no
column list was already specified then perhaps the relentry->columns
can be used for a "dummy" column list that has everything including
all the generated columns.
* By doing this:
-- you may be able to avoid passing the extra
'include_gernated_columns' everywhere
-- you may be able to avoid checking for generated columns deeper in
the code (since it is already checked up-front when building the
column list BMS)
~~
I'm not saying this design idea is guaranteed to work, but it might be
worth considering, because if it does work then there is potential to
make the current 0001 patch significantly shorter.
======
[1] https://www.postgresql.org/message-id/CAHut%2BPsuJfcaeg6zst%3D6PE5uyJv_UxVRHU3ck7W2aHb1uQYKng%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
