Bugzilla – Bug 163
Resolve Timezone ambiguity
Last modified: 2011-01-12 13:16:33 PST
You need to
before you can comment on or make changes to this bug.
There is a concern out there as to what happens if Slony-I is operating in a
timezone that has the Daylight Savings Time discontinuity.
Some ways of addressing this include:
a) Changing Slony-I tables that have TIMESTAMP values to, instead, have
TIMESTAMP WITH TIMEZONE.
Downsides include that this involves a number of alterations to existing
tables, with a conversion ambiguity. (e.g. - what should be done with existing
data? This seems notably risky if the existing timestamps surround the
b) Have the slon "health check" (see bug #156) look at the timezone in
TimeZone, and warn if it has a discontinuity.
Open question: UTC and GMT would be acceptable timezones; what others are
c) Have slon process expressly use UTC as its timezone.
This might change logging output, and be unpopular to users accustomed to
operating Slony in some other timezone.
Intent: Do a).
Set up a branch for this on GitHub...
Took an initial swing at implementation:
1. Change TIMESTAMP to TIMESTAMPTZ in slony1_base.sql
2. Add logic to upgradeSchema() that looks for tables with "timestamp without
time zone" and changes them to "timestamp with time zone"
This has a couple of complications...
The view sl_status depends on the underlying tables; if you try to alter them,
this errors out as follows:
Dec 8 17:01:23 cbbrowne postgres: [3-1] ERROR: cannot alter type of a
column used by a view or rule
Dec 8 17:01:23 cbbrowne postgres: [3-2] DETAIL: rule _RETURN on view
sl_status depends on column "ev_timestamp"
Dec 8 17:01:23 cbbrowne postgres: [3-3] STATEMENT: alter table
sl_event alter column ev_timestamp set data type timestamp with time zone;
Consequently, I added logic to pull the definition of sl_status, using
pg_get_viewdef(). So the logic is...
- Capture view definition for sl_status
- Drop view sl_status
- Do all necessary alterations
- Recreate sl_status
Note also that this requires changing the schema to the Slony cluster's schema,
so pg_get_viewdef() and the subsequent recreation are working against the same
Consequence: change the function to set search_path appropriately. That seems
like something we might want to do to all the Slony functions.
In slony1_funcs.sql log_switch_start()
The reason why the now() needed a typecast was because now() returns a
timestampz and we were storing just a timestamp. Since we are now
storing the full timestampz we don't need the typecast.
My reading of this function is that it will take ANY table in the slony
schema that has a timestamp column and replace it with a timestampz
column. I don't think this query is restricting things to the set of
slony tables that exist today.
If in the future (maybe version 2.2 or 3.5) we add a new column/table
and we REALLY do want a timestamp with no time zone then this
upgradeSchema() function will still alter the table.
If anyone has added a custom table to their slony schema then this query
will alter their table as well. (I don't want to encourage people to be
adding their own tables to the slony schema; I just don't like the idea
of using pattern matching to any table/column in the schema as the
target for an alter table). I'd be more comfortable if we filtered on
the explicit slony tables we have in 2.0
This section should also have a comment a to what it's for ie
-- When upgrading to 2.1 the 'timestamp' columns get changed to
-- timestampz since we now store the timezone
I have not tested this verison of the patch just reviewed it.
Made the changes suggested by Steve...
In principle, this may be committable; can folks take a poke at the branch
Merged into master...
Required a touch-up :-(