Re: API bug in DetermineTimeZoneOffset()

Tom Lane wrote:

> 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.

It would be most useful if the API of these functions did not rely on
global variables in this way, at least not in GUC variables. (I think
the HasCTZSet/CTimeZone business is not as bad as session_timezone).
Doing that would ease future work to move all this code to src/common/
from where the ecpg implementation could also pick it up. (AFAIR I
recently noticed that there's also frontend code that wants to do
datetime processing, pg_basebackup maybe?). Last I checked, those
global variables were problematic, and the GUC variables were the
hardest to handle of the bunch. So, perhaps, instead of having the code
check session_timezone explicitely, have the caller pass it down.

This consideration probably shouldn't drive a backpatchable fix,
however.

> 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.

Yep, sounds plausible.

Thanks,

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services