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 discontinuity.)
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 potentially acceptable?
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 namespaces.
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 :-(