Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> On Tue, 2011-02-01 at 11:01 -0600, Kevin Grittner wrote:
>> My compiler doesn't.
>
> Strange. Maybe it requires -O2?
That's not it; I see -O2 in my compiles.
At any rate, I think the default clause is the best place to quash
the warning because that leaves us with a warning if changes leave a
path through the other options without setting xid. In other words,
unconditionally setting a value before the switch could prevent
warnings on actual bugs, and I don't see such a risk when it's on
the default.
>> A small push dealing with all the above issues and adding a
>> little to comments:
> Looks good. It also looks like it contains a bugfix for
> subtransactions, right?
I think it fixes a bug introduced in the push from late yesterday.
In reviewing what went into the last push yesterday, it looked like
I might have introduced an assertion failure for the case where
there is a write to a row within a subtransaction and then row is
read again after leaving the subtransaction. I didn't actually
confirm that through testing, because it looked like the safe
approach was better from a performance standpoint, anyway. That
last check for "our own xid" after finding the top level xid comes
before acquiring the LW lock and doing an HTAB lookup which aren't
necessary in that case. Re-reading a row within the same transaction
seems likely enough to make it worth that quick test before doing
more expensive things.
It did throw a scare into me, though. The last thing I want to do
is destabilize things at this juncture. I'll try to be more
conservative with changes from here out.
-Kevin