Re: Proposal for better support of time-varying timezone abbreviations

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: Proposal for better support of time-varying timezone abbreviations
Date: 2014-10-07 22:05:20
Message-ID: 28124.1412719520@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> 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
> {
> char token[TOKMAXLEN + 1]; /* now always null-terminated */
> char type;
> 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.)

Attached is a proposed patch for this part. It turned out to be quite
a bit more exciting (not in a good way) than I'd expected. Somewhere
along the way, somebody decided that the easiest way to get from point A
to point B was to insert a minus sign into the FROMVAL() macro, meaning
that FROMVAL() and TOVAL() were not inverses as one would rationally
expect. There were various ways that one could deal with this bit of
dirty laundry; in particular I considered flipping the sign definition
of TZ/DTZ entry values. It seemed better in the end to keep the sign
the same (matching the IANA code's convention for timezone offset sign)
and negate the results at the call sites where needed. Since, in another
bit of weird design, all those call sites already had a multiplication
by MINS_PER_HOUR (the wrong spelling of "60", btw, but I digress) that
needed to be got rid of, this didn't result in touching more places
than I would have had to anyway.

Much worse for the present effort: I was reminded that ecpg contains
a *hard wired* list of known zone abbreviations and their GMT offsets;
a list that doesn't seem to have been updated since around 2003. I'm
not sure what a reasonable fix would be for that. ecpg can't assume it
has access to the timezone database, probably, so bringing it up to
speed with what I propose to do in the backend doesn't seem feasible.
For the moment I didn't change the data there, even though a lot of
the entries are obsolete.

The attached patch also fixes a possible free()-of-uninitialized-pointer
problem in PGTYPEStimestamp_defmt_scan().

regards, tom lane

Attachment Content-Type Size
rationalize-datetkn-1.patch text/x-diff 51.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2014-10-07 23:34:09 Re: Promise index tuples for UPSERT
Previous Message Andres Freund 2014-10-07 20:31:26 Re: lwlock contention with SSI