Hi,
On 4/5/23 4:24 PM, Robert Haas wrote:
> On Tue, Apr 4, 2023 at 8:33 PM Jeff Davis <pgsql@j-davis.com> wrote:
>> For comments, I agree that WalSndWakeup() clearly needs a comment
>> update. The call site in ApplyWalRecord() could also use a comment. You
>> could add a comment at every call site, but I don't think that's
>> necessary if there's a good comment over WalSndWakeup().
>
> Right, we don't want to go overboard, but I think putting some of the
> text you wrote above for the commit message, or something with a
> similar theme, in the comment for WalSndWakeup() would be quite
> helpful. We want people to understand why the physical and logical
> cases are different.
Gave it a try in V61 posted up-thread.
> I agree with you that ApplyWalRecord() is the other place where we
> need a good comment. I think the one in v60 needs more word-smithing.
> It should probably be a bit more detailed and clear about not only
> what we're doing but why we're doing it.
Gave it a try in V61 posted up-thread.
>
> Now that I understand what's going on here a bit better, I'm inclined
> to think that this patch is basically fine. At least, I don't see any
> obvious problem with it.
Thanks for the review and feedback!
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com