Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
On Wed, Oct 22, 2014 at 11:32:41AM -0400, Tom Lane wrote: I don't have a strong opinion about which of the above things to do ... what's your preference? I think it's better for the future if me make a clean cut. Yes, the option keeps compatability a little bit higher, but that doesn't matter that much as the codes won't be used by the server anyway. Besides, they may eventually change and then we have to make sure to remember ecpg's copy again. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
On Wed, Oct 15, 2014 at 09:50:16AM -0400, Tom Lane wrote: The same thought had occurred to me. Probably the main use of the datetime parsing code in ecpg is for interpreting outputs from the server, and (at least by default) the server doesn't use timezone abbreviations when printing timestamps. So maybe that's largely dead code anyhow. I would not propose back-patching such a change, but we could try it in 9.5 and see if anyone complains. Agreed on all accounts. A less drastic remedy would be to remove just those abbreviations whose meaning has actually changed over time. Eventually that might be all of them ... but in the meantime, we could at least argue that we weren't breaking any case that worked well before. This is what your patch did, right? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
Michael Meskes mes...@postgresql.org writes: On Wed, Oct 15, 2014 at 09:50:16AM -0400, Tom Lane wrote: The same thought had occurred to me. Probably the main use of the datetime parsing code in ecpg is for interpreting outputs from the server, and (at least by default) the server doesn't use timezone abbreviations when printing timestamps. So maybe that's largely dead code anyhow. I would not propose back-patching such a change, but we could try it in 9.5 and see if anyone complains. Agreed on all accounts. A less drastic remedy would be to remove just those abbreviations whose meaning has actually changed over time. Eventually that might be all of them ... but in the meantime, we could at least argue that we weren't breaking any case that worked well before. This is what your patch did, right? No, I did not touch ecpg's set of tokens at all, just changed the representation of datetktbl to match the new backend coding. I figured we could discuss behavioral changes separately. I don't have a strong opinion about which of the above things to do ... what's your preference? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
On 15.10.2014 00:26, Tom Lane wrote: * I've not touched ecpg except for cosmetic changes to keep the struct definitions in sync, and to fix the previously-mentioned bogus free() attempt. I doubt that it would be worth teaching ecpg how to access the zic timezone database --- the problem of configuring where to find those files seems like more trouble than it's worth given the lack of complaints. I'm not sure what we should do about the obsolete timezone abbreviations in its table. Maybe we should just remove thme for the new release. Yes, that might break some applications, but then the server doesn't know these either, so the applications might break anyway. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
Michael Meskes mes...@postgresql.org writes: On 15.10.2014 00:26, Tom Lane wrote: * I've not touched ecpg except for cosmetic changes to keep the struct definitions in sync, and to fix the previously-mentioned bogus free() attempt. I doubt that it would be worth teaching ecpg how to access the zic timezone database --- the problem of configuring where to find those files seems like more trouble than it's worth given the lack of complaints. I'm not sure what we should do about the obsolete timezone abbreviations in its table. Maybe we should just remove thme for the new release. Yes, that might break some applications, but then the server doesn't know these either, so the applications might break anyway. The same thought had occurred to me. Probably the main use of the datetime parsing code in ecpg is for interpreting outputs from the server, and (at least by default) the server doesn't use timezone abbreviations when printing timestamps. So maybe that's largely dead code anyhow. I would not propose back-patching such a change, but we could try it in 9.5 and see if anyone complains. A less drastic remedy would be to remove just those abbreviations whose meaning has actually changed over time. Eventually that might be all of them ... but in the meantime, we could at least argue that we weren't breaking any case that worked well before. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
... and here is a draft patch for the timezone abbreviation data files. I changed all the abbreviations for which the parent zone had used more than one GMT offset since 1970. That seemed like a good cutoff to avoid wasting cycles on ancient history, especially since the IANA people themselves don't make any large promises about the quality of their data before 1970. Although zones in the Russian Federation are the majority of zones that had abbreviation changes, a quick look at this patch shows that they're hardly the only ones. We've been sticking our heads in the sand about this problem for quite a while :-( regards, tom lane diff --git a/src/timezone/tznames/America.txt b/src/timezone/tznames/America.txt index 54b51fe..9e62732 100644 *** a/src/timezone/tznames/America.txt --- b/src/timezone/tznames/America.txt *** AMT-14400# Amazon Time *** 47,53 # (America/Cuiaba) # (America/Manaus) # (America/Porto_Velho) ! ART-10800# Argentina Time # (America/Argentina/Buenos_Aires) # (America/Argentina/Cordoba) # (America/Argentina/Tucuman) --- 47,53 # (America/Cuiaba) # (America/Manaus) # (America/Porto_Velho) ! ARTAmerica/Argentina/Buenos_Aires # Argentina Time # (America/Argentina/Buenos_Aires) # (America/Argentina/Cordoba) # (America/Argentina/Tucuman) *** ART-10800# Argentina Time *** 58,64 # (America/Argentina/Mendoza) # (America/Argentina/Rio_Gallegos) # (America/Argentina/Ushuaia) ! ARST-7200 D # Argentina Summer Time # CONFLICT! AST is not unique # Other timezones: # - AST: Arabic Standard Time (Asia) --- 58,64 # (America/Argentina/Mendoza) # (America/Argentina/Rio_Gallegos) # (America/Argentina/Ushuaia) ! ARSTAmerica/Argentina/Buenos_Aires # Argentina Summer Time # CONFLICT! AST is not unique # Other timezones: # - AST: Arabic Standard Time (Asia) *** GMT 0# Greenwich Mean Time *** 228,234 # (Etc/GMT) # (Europe/Dublin) # (Europe/London) ! GYT-14400# Guyana Time # (America/Guyana) HADT -32400 D # Hawaii-Aleutian Daylight Time # (America/Adak) --- 228,234 # (Etc/GMT) # (Europe/Dublin) # (Europe/London) ! GYTAmerica/Guyana # Guyana Time # (America/Guyana) HADT -32400 D # Hawaii-Aleutian Daylight Time # (America/Adak) *** PST-28800# Pacific Standard Time *** 285,299 # (Pacific/Pitcairn) PYST -10800 D # Paraguay Summer Time # (America/Asuncion) ! PYT-14400# Paraguay Time # (America/Asuncion) ! SRT-10800# Suriname Time # (America/Paramaribo) UYST-7200 D # Uruguay Summer Time # (America/Montevideo) UYT-10800# Uruguay Time # (America/Montevideo) ! VET-16200# Venezuela Time (caution: this used to mean -14400) # (America/Caracas) WGST-7200 D # Western Greenland Summer Time # (America/Godthab) --- 285,299 # (Pacific/Pitcairn) PYST -10800 D # Paraguay Summer Time # (America/Asuncion) ! PYTAmerica/Asuncion # Paraguay Time # (America/Asuncion) ! SRTAmerica/Paramaribo # Suriname Time # (America/Paramaribo) UYST-7200 D # Uruguay Summer Time # (America/Montevideo) UYT-10800# Uruguay Time # (America/Montevideo) ! VETAmerica/Caracas # Venezuela Time # (America/Caracas) WGST-7200 D # Western Greenland Summer Time # (America/Godthab) diff --git a/src/timezone/tznames/Antarctica.txt b/src/timezone/tznames/Antarctica.txt index 5a03250..2359020 100644 *** a/src/timezone/tznames/Antarctica.txt --- b/src/timezone/tznames/Antarctica.txt *** CLST -10800 D # Chile Summer Time *** 16,26 CLT-14400# Chile Time # (America/Santiago) # (Antarctica/Palmer) ! DAVT25200# Davis Time (Antarctica) # (Antarctica/Davis) DDUT36000# Dumont-d`Urville Time (Antarctica) # (Antarctica/DumontDUrville) ! MAWT18000
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
I wrote:
I got interested in the problem discussed in
http://www.postgresql.org/message-id/20714.1412456...@sss.pgh.pa.us
to wit:
It's becoming clear to me that our existing design whereby zone
abbreviations represent fixed GMT offsets isn't really good enough.
I've been wondering whether we could change things so that, for instance,
EDT means daylight time according to America/New_York and the system
would consult the zic database to find out what the prevailing GMT offset
was in that zone on that date. This would be a lot more robust in the
face of the kind of foolishness we now see actually goes on.
Attached is an updated patch for this, incorporating my previous work on
changing the representation of datetkn. The code changes are complete
I believe, but I've not touched the user documentation yet, nor updated
the tznames data files except for changing MSK for testing purposes.
Some notes:
* There wasn't any reasonable way to return the required information about
a dynamic zone abbreviation from DecodeSpecial(). However, on looking
closely I found that of the existing callers of DecodeSpecial(), only two
actually relied on it to find both timezone abbreviations and regular
keywords. The other callers only wanted one case or the other. So it
seemed to me that it'd be best to split the abbreviation-lookup behavior
apart from keyword-lookup. DecodeSpecial() now does only the latter, and
there's a new function DecodeTimezoneAbbrev() to do the former. This
avoids touching any code that doesn't care about timezone abbreviations,
and should be a bit faster for those cases too (but about the same speed
otherwise).
* I found that there were pre-existing bugs in DetermineTimeZoneOffset()
for cases in which a zone changed offset but neither the preceding nor
following time segment was marked as DST time. This caused strange
behaviors for cases like Europe/Moscow's 2011 and 2014 time changes.
This is not terribly surprising because we never thought about zone
changes other than simple DST spring-forward/fall-back changes when
that code was written.
* I've not touched ecpg except for cosmetic changes to keep the struct
definitions in sync, and to fix the previously-mentioned bogus free()
attempt. I doubt that it would be worth teaching ecpg how to access the
zic timezone database --- the problem of configuring where to find those
files seems like more trouble than it's worth given the lack of
complaints. I'm not sure what we should do about the obsolete timezone
abbreviations in its table.
* timetz_zone() and DecodeTimeOnly() are not terribly consistent about
how they resolve timezones when not given a date. The former resolves
based on the value of time(NULL), which is a moving target even within
a transaction (which is why the function is marked volatile I guess).
The latter gets today's date as of start of transaction (so at least
it's stable) and then merges that m/d/y with the h/m/s from the time
value. That behavior does not seem terribly well thought out either:
on the day of, or day before or after, a DST change it's likely to
produce strange results, especially if the session timezone is different
from the zone specified in the input. I think we ought to consider
changing one or both of these, though it's not material for back-patching.
* In HEAD, we might want to extend the pg_timezone_abbrevs view to have
a column that shows the underlying zone for a dynamic abbreviation.
The attached patch just shows the abbreviation's behavior as of current
time (defined as now()), which is sort of good enough but certainly not
the whole truth.
Comments?
regards, tom lane
diff --git a/contrib/btree_gist/btree_ts.c b/contrib/btree_gist/btree_ts.c
index a13dcc8..b9c2b49 100644
*** a/contrib/btree_gist/btree_ts.c
--- b/contrib/btree_gist/btree_ts.c
*** tstz_dist(PG_FUNCTION_ARGS)
*** 200,226
**/
! static Timestamp
tstz_to_ts_gmt(TimestampTz ts)
{
! Timestamp gmt;
! int val,
! tz;
!
! gmt = ts;
! DecodeSpecial(0, gmt, val);
!
! if (ts DT_NOEND ts DT_NOBEGIN)
! {
! tz = val * 60;
!
! #ifdef HAVE_INT64_TIMESTAMP
! gmt -= (tz * INT64CONST(100));
! #else
! gmt -= tz;
! #endif
! }
! return gmt;
}
--- 200,210
**/
! static inline Timestamp
tstz_to_ts_gmt(TimestampTz ts)
{
! /* No timezone correction is needed, since GMT is offset 0 by definition */
! return (Timestamp) ts;
}
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 073104d..bb23b12 100644
*** a/src/backend/utils/adt/date.c
--- b/src/backend/utils/adt/date.c
*** timetz_zone(PG_FUNCTION_ARGS)
*** 2695,2718
pg_tz *tzp;
/*
! * Look up the requested timezone. First we look in the date token table
! * (to handle cases like EST), and if that fails, we look in the
! *
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
On Tue, Oct 7, 2014 at 5:05 PM, Tom Lane t...@sss.pgh.pa.us wrote:
typedef struct
{
! char token[TOKMAXLEN + 1]; /* now always null-terminated */
char type;
! int32 value;
} datetkn;
Being entirely new to this code, now makes me think of the current
timestamp. I think this word can be removed to reduce ambiguity.
+ /* use strncmp so that we match truncated tokens */
result = strncmp(key, position-token, TOKMAXLEN);
In your proposal you wanted to remove crufty code that deals with
non-null-terminated token strings. Is this some of that crufty code? Can
it be removed?
-- Chris
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
Chris Bandy bandy.ch...@gmail.com writes: On Tue, Oct 7, 2014 at 5:05 PM, Tom Lane t...@sss.pgh.pa.us wrote: + /* use strncmp so that we match truncated tokens */ result = strncmp(key, position-token, TOKMAXLEN); In your proposal you wanted to remove crufty code that deals with non-null-terminated token strings. Is this some of that crufty code? Can it be removed? Yeah, I had hoped to simplify these things to just strcmp, but on closer inspection that didn't work, because some of the keywords in the table are truncated at TOKMAXLEN (10 characters). If we just made these strcmp then the code would stop recognizing e.g. milliseconds. I thought briefly about widening TOKMAXLEN so that there were no truncated keywords in the table, but that seems risky from a backwards-compatibility standpoint: as it stands, the code will accept milliseconds, millisecond, or millisecon, and there might possibly be applications out there that depend on that. In any case I'd much rather keep the array stride at 16 bytes for speed reasons; and who's to say we might not put in some even-longer keywords in the future? Another alternative we should maybe consider is leaving the definition of the token field alone (ie, still not guaranteed null terminated) which'd leave us with one free byte per datetkn entry. I can't think of a likely reason to need another 1-byte field though, and the existing definition is not without risk. That comment that was there about don't change this to strlcpy was there because somebody broke it awhile back, or at least submitted a patch that would've broken it if it'd been accepted. People are too used to null-terminated strings in C; a field definition that violates that norm is just trouble waiting to happen. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
On 10/6/14, 6:19 PM, Jim Nasby wrote: FWIW, I agree for timestamptz, but I do wish we had a timestamp datatype that stored the exact timezone in effect when the data was entered. That can really, REALLY save your rear if you screw up either timezone in postgresql.conf, or the server's timezone. The part that seems hard (at least to me) is the question of how to actually store the timezone, because I don't think storing the text string America/Central is going to cut it. :/ For the archives... there's an extension that does what I'd been talking about: http://pgxn.org/dist/timestampandtz/. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
On 10/5/14, 5:42 PM, Tom Lane wrote: Gavin Flower gavinflo...@archidevsys.co.nz writes: The use of an /as_at_date/ is far more problematic. The idea relates to how existing date/times should be treated with respect to the date/time that a pg database is updated with new time zone data files. In the simplest form: there would be a function in pg that would return the date/time a new time zone data file was entered into the system, so that application software can manually correct when the stored GMT date/time was stored incorrectly because the wring GMT offset was used due to the updated time zone data files not being in place. Alternatively, pg could offer to do the correction in a one-off action at the time the new zone data files were updated. Right now there's basically no way to do something like that, since what we store for timestamptz is just a UTC time instant, with no record of what GMT offset was involved much less exactly how the offset was specified in the input. We'd probably have to (at least) double the on-disk size of timestamptz values to record that ... which seems like a mighty high price to pay to fix a corner case. Not to mention that nobody's going to be willing to break on-disk compatibility of timestamptz for this. FWIW, I agree for timestamptz, but I do wish we had a timestamp datatype that stored the exact timezone in effect when the data was entered. That can really, REALLY save your rear if you screw up either timezone in postgresql.conf, or the server's timezone. The part that seems hard (at least to me) is the question of how to actually store the timezone, because I don't think storing the text string America/Central is going to cut it. :/ -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
On 06/10/14 10:33, Tom Lane wrote:
I got interested in the problem discussed in
http://www.postgresql.org/message-id/20714.1412456...@sss.pgh.pa.us
to wit:
It's becoming clear to me that our existing design whereby zone
abbreviations represent fixed GMT offsets isn't really good enough.
I've been wondering whether we could change things so that, for instance,
EDT means daylight time according to America/New_York and the system
would consult the zic database to find out what the prevailing GMT offset
was in that zone on that date. This would be a lot more robust in the
face of the kind of foolishness we now see actually goes on.
Here is a fairly detailed design sketch for a solution:
1. Allow tznames entries to consist of an abbreviation and the name of
a zic timezone, for example
MSK Europe/Moscow
instead of the current scheme whereby an abbreviation is defined by a
daylight-savings flag and a numeric GMT offset. When an abbreviation is
defined this way, the implied offset and DST flag are looked up
dynamically as described below. (In my message quoted above, I'd imagined
that we'd write a DST flag and a zone name, but it turns out this does not
work because there are cases where the DST property has changed over time.
Yes, really. So this design mandates that we derive the DST flag by
looking into the zic timezone data.) Note that we'll still allow the old
style of entries, and indeed prefer that way for cases where an
abbreviation has never changed meaning, because:
* We need that anyway for forwards compatibility of existing custom
abbreviations files.
* It's a lot cheaper to interpret a fixed-meaning zone abbreviation using
the existing logic than to do it as I propose here, so we shouldn't spend
the extra cycles unless necessary.
* Converting every one of the existing abbreviation-file entries would be
really tedious, so I don't want to do it where not necessary.
Also note that this doesn't touch the aspect of the existing design
whereby there are multiple potential abbreviations files. We still have
the problem that the same abbreviation can be in use in different
timezones, so we have to let users configure which zone they mean by a
given abbreviation.
2. To interpret such an abbreviation in the context of timestamptz input,
look up the referenced zic timezone, and use the meaning of the
abbreviation that prevailed at or most recently before the local time
indicated by the rest of the timestamptz string. If the abbreviation was
never used before that time in the given zone, use its earliest later
interpretation; or if it was never used at all (ie bad configuration file)
throw error. Note that this is different from what happens if you give
the underlying zone name directly. It's always been the case that you
could say, for instance, EST to force interpretation of a datetime as
standard time even when DST is in force, or EDT to force the opposite
interpretation, and this definition preserves that behavior.
3. In the context of timetz input, we only have a time of day not a full
datetime to look at, so it's not entirely clear what to do. We could
throw an error, but that would result in rejecting some inputs currently
considered valid. Perhaps we don't really care, since we consider timetz
a deprecated type anyway. If that doesn't seem OK, we could assume
today's date and the given time-of-day and look up the abbreviation's
meaning as described above. This would mean that the meaning of, say,
'15:00 MSK'::timetz would change over time --- but that happens now,
whenever we change the contents of the abbreviations file entry for MSK,
so maybe this isn't as horrid as it sounds.
4. I've eyeballed the relevant code a bit, and it seems that the only
implementation aspect that isn't perfectly straightforward is figuring
out how to cram a zic timezone reference into a datetkn table entry.
I suggest that before tackling this feature proper, we bring struct
datetkn into the 21st century by widening it from 12 to 16 bytes, along
the lines of
typedef struct
{
chartoken[TOKMAXLEN + 1]; /* now always null-terminated */
chartype;
int32 value;
} datetkn;
and getting rid of all of the very crufty code that deals with
non-null-terminated token strings and cramming values that don't really
fit into a char-sized field into value. (We might save more code bytes
that way than we spend on the wider token-table entries :-( ... and we'll
certainly make the code less ugly.) Having done that, the value can be
large enough to be an index into additional storage appended to a
TimeZoneAbbrevTable. I imagine it pointing at a struct like this:
struct DynamicTimeZoneAbbrev
{
const pg_tz *tz; /* zic timezone, or NULL if not yet looked up */
charname[1]; /* zone name (variable length string)
};
We'd resolve the timezone name into a pg_tz pointer only upon first use of
a dynamic abbreviation, since we don't want to force loading
Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations
Gavin Flower gavinflo...@archidevsys.co.nz writes: The use of an /as_at_date/ is far more problematic. The idea relates to how existing date/times should be treated with respect to the date/time that a pg database is updated with new time zone data files. In the simplest form: there would be a function in pg that would return the date/time a new time zone data file was entered into the system, so that application software can manually correct when the stored GMT date/time was stored incorrectly because the wring GMT offset was used due to the updated time zone data files not being in place. Alternatively, pg could offer to do the correction in a one-off action at the time the new zone data files were updated. Right now there's basically no way to do something like that, since what we store for timestamptz is just a UTC time instant, with no record of what GMT offset was involved much less exactly how the offset was specified in the input. We'd probably have to (at least) double the on-disk size of timestamptz values to record that ... which seems like a mighty high price to pay to fix a corner case. Not to mention that nobody's going to be willing to break on-disk compatibility of timestamptz for this. In any case, my proposal is just about being able to correctly interpret historical timezone abbreviations during input, not about changing what we store as datetime values. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
