On Wed, Oct 20, 2021 at 10:25 AM Dilip Kumar <dilipbal...@gmail.com> wrote:
>
> On Mon, Oct 18, 2021 at 10:48 AM Amit Kapila <amit.kapil...@gmail.com> wrote:
>
> >
> > Today, I have looked at this patch again and slightly changed a
> > comment, one of the function name and variable name. Do, let me know
> > if you or others have any suggestions for better names or otherwise? I
> > think we should backpatch this to 14 as well where this code was
> > introduced.
> >
>
> bool
> -IsSubTransactionAssignmentPending(void)
> +IsTopTransactionIdLogged(void)
> {
> /* wal_level has to be logical */
> if (!XLogLogicalInfoActive())
> @@ -6131,19 +6131,20 @@ IsSubTransactionAssignmentPending(void)
> if (!TransactionIdIsValid(GetCurrentTransactionIdIfAny()))
> return false;
>
> - /* and it should not be already 'assigned' */
> - return !CurrentTransactionState->assigned;
> + /* and it should not be already 'logged' */
> + return !CurrentTransactionState->topXidLogged;
> }
>
> I have one comment here, basically, you have changed the function name
> to "IsTopTransactionIdLogged", but it still behaves like
> IsTopTransactionIdLogPending. Now with the new name, it should return
> (CurrentTransactionState->topXidLogged) instead of
> (!CurrentTransactionState->topXidLogged).
>Valid point but I think the change suggested by you won't be sufficient. We also need to change all the other checks in that function to return true which will make it a bit awkward. So instead, we can change the function name to IsTopTransactionIdLogPending(). Does that make sense? -- With Regards, Amit Kapila.
v4-0001-Replace-XLOG_INCLUDE_XID-flag-with-a-more-localiz.patch
Description: Binary data
