There are many comments in the code which use "serializable
transaction" to mean "transaction snapshot based transaction". This
doesn't matter much as long as REPEATABLE READ and SERIALIZABLE
transaction isolation levels behave identically, but will be
confusing and inaccurate when there is any difference between the
two. In a similar way, the static bool registered_serializable in
snapmgr.c will become misleading, and the IsXactIsoLevelSerializable
macro is bound to lead to confusion.
The patch to implement true serializable transactions using SSI
renames/rewords these things to avoid confusion. However, there are
seven files which are changed only for this reason, and another
where there is one "real" change of two lines hidden in the midst of
dozens of lines of such wording changes. I find it distracting to
have all this mixed in, and I fear that it will be a time-waster for
anyone reviewing the meat of the patch. I'd like to submit a
"no-op" patch to cover these issues in advance of the CF, to get
them out of the way. Joe suggested that I pose the issue to the
-hackers list.
I could knock out a couple other files from the main patch if people
considered it acceptable to enable the SHMQueueIsDetached function
now, which the patch uses in several places within asserts. I would
remove the #ifdef NOT_USED from around the (very short) function,
and add it to the .h file.
The changes to the comments and local variables seem pretty safe.
The change of IsXactIsoLevelSerializable to
IsXactIsoLevelXactSnapshotBased (or whatever name the community
prefers) could break the compile of external code; but the fix would
be pretty obvious. It's hard to see how the change of a local
variable name would present a lot of risk. So I see this as an
extremely low-risk no-op patch to lay the groundwork for the main
patch, so that it is easier to read.
Thoughts?
-Kevin