Re: pgsql: Document XLOG_INCLUDE_XID a little better

Dilip Kumar <dilipbalaut@gmail.com>

From: Dilip Kumar <dilipbalaut@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Robert Haas <robertmhaas@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-10-20T04:55:05Z
Lists: pgsql-hackers
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).

And the caller should also be changed accordingly.  Other changes look fine.


-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com



Commits

  1. Move MarkCurrentTransactionIdLoggedIfAny() out of the critical section.

  2. Replace XLOG_INCLUDE_XID flag with a more localized flag.

  3. Document XLOG_INCLUDE_XID a little better

  4. Immediately WAL-log subtransaction and top-level XID association.