Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> The current definition is:
>
> #define IsXactIsoLevelSerializable (XactIsoLevel >=
> XACT_REPEATABLE_READ)
>
> ...which is certainly a bit odd, since you'd think it would be
> comparing against XACT_SERIALIZABLE given the name.
Precisely why I want to rename it. ;-)
> IsXactIsoLevelRepeatableRead()?
Since the SSI implementation of a fully serializable transaction
isolation level needs to do everything that the snapshot isolation
of REPEATABLE READ does, plus a wee bit more, it is convenient to
have a macro with the same semantics; just a less confusing name.
I don't see anywhere in the code where there's a need to test for
*just* REPEATABLE READ -- anything done for that also needs to be
done for SERIALIZABLE.
> XactUsesPerXactSnapshot()?
That seems unambiguous. I think I prefer it to
IsXactIsoLevelXactSnapshotBased, so if there are no objections, I'll
switch to XactUsesPerXactSnapshot. The current code uses a macro
without parentheses; are you suggesting that the new code add those?
> Or, inverting the sense of it, XactUsesPerStatementSnapshot()?
I don't see anywhere that the code is throwing an exclamation point
in front of the macro name, so inverting it seems like a bad idea.
I'd rather go from:
if (IsXactIsoLevelSerializable)
to:
if (XactUsesPerXactSnapshot)
than:
if (!XactUsesPerStatementSnapshot)
Given the suggested name above, IsXactIsoLevelFullySerializable no
longer seems, well, symmetrical. How do you feel about
XactIsFullySerializable?
Names starting with IsXactIsoLevel seem more technically correct,
but the names get long enough that it seems to me that the meaning
gets a bit lost in the jumble of words -- which is why I like the
shorter suggested name. Any other opinions out there?
-Kevin