ReadBuffer() error checking
Neil Conway
buffer it returns is valid. The vast majority of ReadBuffer() call sites
make this assumption, but some went to the trouble of checking that the
returned buffer was valid and elog'ing if it was not. I've removed the
error checking from the latter since it is dead code.
I thought about adding an assertion (or even a precautionary
elog(ERROR)) to ReadBuffer to verify that the returned buffer is indeed
valid, but I didn't end up doing it. Feel free to raise your hand if you
think this is a good idea.
Barring any objections, I'll apply the attached patch to HEAD tomorrow.
-Neil
Tom Lane
> AFAIK, ReadBuffer() will elog on error, so callers can assume that the
> buffer it returns is valid. The vast majority of ReadBuffer() call sites
> make this assumption, but some went to the trouble of checking that the
> returned buffer was valid and elog'ing if it was not. I've removed the
> error checking from the latter since it is dead code.
Agreed. I get the impression that at one time it was not so, but
certainly for the last many years there's been no need to test.
> I thought about adding an assertion (or even a precautionary
> elog(ERROR)) to ReadBuffer to verify that the returned buffer is indeed
> valid, but I didn't end up doing it. Feel free to raise your hand if you
> think this is a good idea.
Nah; considering that the return statements invoke
BufferDescriptorGetBuffer, you'll probably get a core dump anyway
if there's something wrong ;-)
A related issue in the same general area is that the smgr code is
currently implemented to elog on error, but its API still reflects
an assumption that it will return a failure indication. Changing
the API is a larger change than I want to see during late beta,
but it's a cleanup that would be reasonable to undertake during
a future development cycle, if you're interested.
regards, tom lane
---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majo...@postgresql.org
Neil Conway
> Agreed. I get the impression that at one time it was not so, but
> certainly for the last many years there's been no need to test.
Patch applied.
> A related issue in the same general area is that the smgr code is
> currently implemented to elog on error, but its API still reflects
> an assumption that it will return a failure indication. Changing
> the API is a larger change than I want to see during late beta,
> but it's a cleanup that would be reasonable to undertake during
> a future development cycle, if you're interested.
Sure, I'll take a look at it once we branch for 8.0.
-Neil
---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match