|Summary:||Resolve Timezone ambiguity|
|Product:||Slony-I||Reporter:||Christopher Browne <cbbrowne>|
|Component:||slon||Assignee:||Christopher Browne <cbbrowne>|
Description Christopher Browne 2010-11-01 14:19:31 UTC
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.
Comment 1 Christopher Browne 2010-12-08 12:02:32 UTC
Intent: Do a).
Comment 2 Christopher Browne 2010-12-08 14:38:23 UTC
Set up a branch for this on GitHub... https://github.com/cbbrowne/slony1-engine/commits/bug163 Took an initial swing at implementation: https://github.com/cbbrowne/slony1-engine/commit/42a4d2ed275d4d2c1b08f49b5c079e8c6a790258 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.
Comment 3 Steve Singer 2010-12-13 14:00:27 UTC
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. upgradeSchema() 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.
Comment 4 Christopher Browne 2010-12-16 12:38:16 UTC
Comment 5 Christopher Browne 2010-12-30 09:00:21 UTC
In principle, this may be committable; can folks take a poke at the branch https://github.com/cbbrowne/slony1-engine/tree/bug163 ???
Comment 6 Christopher Browne 2011-01-12 13:16:33 UTC