API bug in DetermineTimeZoneOffset()

DetermineTimeZoneOffset thinks that if the passed pg_tz parameter is
equal to session_timezone, it should pay attention to HasCTZSet/CTimeZone
and allow those to override the pg_tz. The folly of this is revealed by
bug #8572, wherein timestamptz input that explicitly specifies a timezone
name is taken to be in the "brute force" zone CTimeZone, if the named zone
is by chance the same zone as the session_timezone that had prevailed
before the "brute force" zone was set. (A "brute force" timezone setting
is a simple numeric offset from GMT, rather than a named zone, which is
looked up in the Olsen database.)

I think that we should change this function to follow the API convention
used by timestamp2tm(), namely that one passes a NULL pointer if one
would like session_timezone/HasCTZSet/CTimeZone to control the result.
A non-null pointer should mean to use that zone specification, period.

This bug is of long standing, so I'm inclined to back-patch the fix.
Now, that's possibly problematic if there are any third-party modules
calling DetermineTimeZoneOffset and passing session_timezone, because
it would mean that they'd stop honoring "brute force" zone settings.
However, I suspect that this feature is practically unused in the field,
else we'd have heard complaints before now. In any case, the possibility
of creating more bugs shouldn't stop us from fixing the bug we've got;
and any other change in DetermineTimeZoneOffset's API would be even
more likely to break third-party modules.

One idea worth thinking about is to set session_timezone to NULL when
HasCTZSet is set true. This would prevent accidental use of an obsoleted
zone setting, and it would also mean that the coding pattern of passing
session_timezone to DetermineTimeZoneOffset would still work and do what
it used to in this scenario. However, it's always been the case up to now
that session_timezone is a valid zone of some sort, and I'm afraid that
there might be code out there that will crash if we set it to NULL.
So I'm inclined not to do this, or at least not in the back branches.

Thoughts?

regards, tom lane