Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jan 28, 2020 at 2:29 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Well, yeah, that's exactly my point. But in my book, "refuse to do
>> anything" should be "elog(ERROR)", not "invoke undefined behavior".
>> An actual abort() call might be all right here, in that at least
>> we'd know what would happen and we could debug it once we got hold
>> of a stack trace. But pg_unreachable() is not that. Basically, if
>> there's *any* circumstance, broken code or not, where control could
>> reach a pg_unreachable() call, you did it wrong.
> I don't really agree. I think such defensive coding is more than
> justified when the input is coming from a file on disk or some other
> external source where it might have been corrupted.
There's certainly an argument to be made that an elog() call is an
unjustified expenditure of code space and we should just do an abort()
(but still not pg_unreachable(), IMO). However, what I'm really on about
here is that CreateDestReceiver is out of step with nigh-universal project
practice. If it's not worth having an elog() here, then there are
literally hundreds of other elog() calls that we ought to be nuking on
the same grounds. I don't really want to run around and have a bunch
of debates about exactly which extant elog() calls are effectively
unreachable and which are not. That's not always very clear, and even
if it is clear today it might not be tomorrow. The minute somebody calls
CreateDestReceiver with a non-constant argument, the issue becomes open
again. And I'd rather not have to stop and think hard about the tradeoff
between elog() and abort() when I write such functions in future.
So basically, my problem with this is that I don't think it's a coding
style we want to encourage, because it's too fragile. And there's no
good argument (like performance) to leave it that way. I quite agree
with you that there are places like tuple deforming where we're taking
more chances than I'd like --- but there is a noticeable performance
cost to being paranoid there.
regards, tom lane
