On Tue, Jul 2, 2024 at 10:59 AM Peter Smith <smithpb2...@gmail.com> wrote: > > 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
I have fixed this issue in the latest Patches. Please refer to the updated v15 Patches here in [1]. See [1] for the changes added. [1] https://www.postgresql.org/message-id/CAHv8Rj%2B%3Dhn--ALJQvzzu7meX3LuO3tJKppDS7eO1BGvNFYBAbg%40mail.gmail.com Thanks and Regards, Shubham Khanna.
