Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rust-postgres setting timezone to UTC on connecting is not appropriate #608

Closed
jmafc opened this issue Jun 4, 2020 · 8 comments
Closed

Comments

@jmafc
Copy link

jmafc commented Jun 4, 2020

If you issue the query SELECT CURRENT_TIMESTAMP in psql or from other language adapters such as psycopg2, the Postgres server will return a value adjusted to what it considers its timezone (specifically, based on the value in the timezone parameter in postgresql.conf). In psql the value returned shows the timezone adjustment to UTC, e.g., if the timezone in postgresql.conf is set to America/Denver (MST), then the value is suffixed with -07 (or -06 during DST).
rust-postgres overrides this normal situation by sending timezone=UTC when connecting to the server.

let mut params = vec![("client_encoding", "UTF8"), ("timezone", "UTC")];
This not only affects the values of the query above but any query that depends on the server's notion of "local time". For example, if a website shows a different result, based on whether it's past midnight in the local sense (i.e., based on the Postgres server's actual timezone location), if retrieved via rust-postgres the results will change starting at 0:00 UTC, e.g., possibly eight/seven hours earlier than desired if the server is located in the Pacific timezone. The only workaround is for the client to send a SET TIMEZONE timezone-value query before sending such a time-sensitive query.
Therefore I think the timezone parameter should not be sent at all, or if sent, it should be configurable (with the default being "don't send it").

@sfackler
Copy link
Owner

sfackler commented Jun 4, 2020

This is necessary to enable conversion of types to and from time datatypes. If you aren't using those, you can set your own timezone via the options component of the connection config: https://docs.rs/postgres/0.17.3/postgres/config/struct.Config.html#keys.

@jmafc
Copy link
Author

jmafc commented Jun 4, 2020

I am using the conversions, but I'm not sure why overriding the server's notion of its timezone is needed to convert timestamps. In psql, as mentioned above the server returns an indication of its "local time" in the form of the (+|-)nn suffix. In C libpq, IIRC a timestamptz value is also returned as a string with the same suffix. In psycopg2, as I showed in an email earlier, the timezone is also available, e.g., as tzinfo=psycopg2.tz.FixedOffsetTimezone(offset=-240, name=None), if I'm not mistaken, as the result of converting from the retrieved string with suffix.
I will try using options as suggested, but I still think overriding the server's timezone is not needed.

@sfackler
Copy link
Owner

sfackler commented Jun 4, 2020

The timezone offset is not included in the binary encoding of the values.

@jmafc
Copy link
Author

jmafc commented Jun 4, 2020

I find that hard to believe. Do you have a doc or source reference? The internal time defined in src/include/pgtime.h has a struct pg_tm with a long int tm_gmtoff member. Why would they encode TIMESTAMP WITH TIME ZONE without that crucial part?

@sfackler
Copy link
Owner

sfackler commented Jun 4, 2020

@jmafc
Copy link
Author

jmafc commented Jun 5, 2020

I did ask and Tom Lane replied (https://www.postgresql.org/message-id/1375425.1591317171%40sss.pgh.pa.us) that the binary format, as the on-disk format, doesn't include the offset (which makes sense because internally the server operates in UTC where the offset is zero). However, if a timezone is set, timestamptz_out will "rotate" (adjust the value) "for display purposes". We continued the discussion privately, and he conjectured that "possibly the maintainer feels that he wants equivalent results from either text or binary I/O ... which doesn't seem like totally a bad idea." So my question is whether rust-postgres supports text I/O or only binary and if the latter, why?
On another front, I tried to set the timezone in the Config that I pass to deadpool_postgres::Manager::new(), using the following:

config.options("-c 'SET timezone=\'America/New_York\';");

The equivalent string in psql does work, but when I try it (running under actix_web), the web page simply displays the following: An error occured while creating a new object: db error: FATAL: -c 'SET requires a value. I tried several variations with less quotes, double quotes instead of single quotes, unescaped single quotes, no semi-colon, and nothing worked. Any suggestions?

@jbg
Copy link

jbg commented Jul 23, 2020

@jmafc the -c is quite suspicious in that string considering that it's a psql command-line option and psql is not involved at all.

As far as I'm aware rust-postgres only makes use of the more efficient and more flexible binary PostgreSQL protocol.

FWIW, having your application behaviour depend on the system timezone configured on your database server seems a rather brittle approach. If midnight in a particular timezone is important to your application, then it may be a better idea to explicitly convert UTC timestamps to that time zone (you can do it in the database easily using AT TIME ZONE or do it in Rust with e.g. chrono-tz).

@jmafc
Copy link
Author

jmafc commented Jul 23, 2020

The binary may be "more efficient" but I don't think using it exclusively is the right approach precisely because it detracts from the flexibility desired by some users.

@jmafc jmafc closed this as completed Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants