On Wed, Oct 20, 2021 at 10:25 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Oct 18, 2021 at 10:48 AM Amit Kapila <amit.kapila16@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.
