Daniel Wood <dwood@salesforce.com> writes:
> Is the main difference between:
> if (s->blockState != TBLOCK_SUBINPROGESS)
> elog(*FATAL*, ...
> vs
> Assert(s->blockState == TBLOCK_SUBINPROGRESS);
> the fact that in both cases:
> a) the situation is unexpected, as in no user code can create this;
> b) however, if you want the check to always be done in production
> because of paranoia, or because a failure after this would be harder to
> figure out, or because you want to log more info, like the exact value
> of blockState, then you need to use the elog(FATAL, ...) way of doing it?
It's a bit of a judgment call really. The test-and-elog approach is the
thing to use if you want the check to be there in production builds; if
you think it's sufficient to check it in debug builds, then an Assert
is appropriate. And, as you say, the elog does offer an opportunity to
log some additional info beyond "this should not have happened".
Another point is that an Assert forces a database-wide restart, whereas
elog(FATAL) only forces the current session to quit --- in this respect,
elog(PANIC) is a closer approximation to Assert.
I wouldn't necessarily claim that there's been a great deal of
uniformity in past decisions about which way to code can't-happen checks
;-)
> Given the example:
> elog(ERROR, "StartTransactionCommand: unexpected state %s", ...
> vs
> elog(FATAL, "CommitTransactionCommand: unexpected state %s", ...
> why is one considered fatal but in the other case handle-able?
I didn't look at the code but it may be that the Commit case is past the
point where it's safe to abort the transaction (although if so, it
really oughta be a PANIC). Or it might just be randomness; although
I think we did once go through and look at all the FATAL calls to see if
that was an appropriate labeling.
regards, tom lane