On Fri, 2011-01-14 at 12:04 -0500, Tom Lane wrote:
> Tatsuo Ishii <ishii@postgresql.org> writes:
> >> Review:
> >> The only possible point of concern I see here is the naming of the C
> >> identifier. Everything else in class 40 uses ERRCODE_T_R_whatever,
> >> with T_R standing for transaction rollback. It's not obvious to me
> >> that that convention has any real value, but perhaps we ought to
> >> follow it here for the sake of consistency?
>
> > Yeah. Actually at first I used "T_R" convention. After a few seconds
> > thought, I realized that "T_R" is not appropreate by the same reason
> > you feel. Possible other argument might be "Terminating connection
> > always involves transaction rollback. So using T_R is ok". I'm not
> > sure this argument is reasonable enough though.
>
> This is not only a matter of some macro name or other. According to the
> SQL standard, class 40 itself is defined as "transaction rollback".
> If the error condition can't reasonably be regarded as a subcase of
> that, you're making a bad choice of SQLSTATE code.
This whole thing is confused. No change is appropriate here at all.
We issue ERRCODE_T_R_SERIALIZATION_FAILURE almost all of the time for
recovery conflicts.
We issue ERRCODE_ADMIN_SHUTDOWN only if the conflict is non-retryable,
which occurs if someone drops the database that the user was connected
to when they get kicked off. That code exists specifically to inform the
user that they *cannot* reconnect. So pgpool should not be trying to
trap that error and reconnect.
So the whole basis for the existence of this patch is moot.
-- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
