Re: Pgoutput not capturing the generated columns

From: Shubham Khanna <khannashubham1197(at)gmail(dot)com>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Rajendra Kumar Dangwal <dangwalrajendra888(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, euler(at)eulerto(dot)com
Subject: Re: Pgoutput not capturing the generated columns
Date: 2024-07-11 06:16:51
Message-ID: CAHv8RjKQKCU_WLwEXjWBZ2bD5QUH0Vx_TXx0Ph-Q0uEsw0iVqQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jul 10, 2024 at 4:22 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Shubham/Shlok, I was thinking some more about the suggested new
> BitMapSet (BMS) idea of patch 0001 that changes the 'columns' meaning
> to include generated cols also where necessary.
>
> I feel it is a bit risky to change lots of code without being 100%
> confident it will still be in the final push. It's also going to make
> the reviewing job harder if stuff gets added and then later removed.
>
> IMO it might be better to revert all the patches (mostly 0001, but
> also parts of subsequent patches) to their pre-BMS-change ~v14* state.
> Then all the BMS "improvement" can be kept isolated in a new patch
> 0004.
>
> Some more reasons to split this off into a separate patch are:
>
> * The BMS change is essentially a redesign/cleanup of the code but is
> nothing to do with the actual *functionality* of the new "generated
> columns" feature.
>
> * Apart from the BMS change I think the rest of the patches are nearly
> stable now. So it might be good to get it all finished so the BMS
> change can be tackled separately.
>
> * By isolating the BMS change, then we will be able to see exactly
> what is the code cost/benefit (e.g. removal of redundant code versus
> adding new logic) which is part of the judgement to decide whether to
> do it this way or not.
>
> * By isolating the BMS change, then it makes it convenient for testing
> before/after in case there are any performance concerns
>
> * By isolating the BMS change, if some unexpected obstacle is
> encountered that makes it unfeasible then we can just throw away patch
> 0004 and everything else (patches 0001,0002,0003) will still be good
> to go.

As suggested, I have created a separate patch for the Bitmapset(BMS)
idea of patch 0001 that changes the 'columns' meaning to include
generated cols also where necessary.
Please refer to the updated v17 Patches here in [1]. See [1] for the
changes added.

[1] https://www.postgresql.org/message-id/CAHv8RjJ0gAUd62PvBRXCPYy2oTNZWEY-Qe8cBNzQaJPVMZCeGA%40mail.gmail.com

Thanks and Regards,
Shubham Khanna.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2024-07-11 06:30:15 PG_TEST_EXTRA and meson
Previous Message Shubham Khanna 2024-07-11 06:12:58 Re: Pgoutput not capturing the generated columns