Re: pgsql: Document XLOG_INCLUDE_XID a little better

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.