Skip to content

Change connection initialization timezone to UTC #513

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

Merged
merged 1 commit into from
Nov 17, 2019

Conversation

aloucks
Copy link
Contributor

@aloucks aloucks commented Nov 16, 2019

Although CockroachDB supports the postgres wire protocol, it doesn't appear to support using the GMT timezone (at least not on Windows). However, cockroach does accept UTC. This PR provides a way to configure the timezone connection property.

I've left the default as GMT but I think UTC is probably a better option. The native client defaults to unspecified:

timezone (string)
Sets the time zone for displaying and interpreting time stamps. If not explicitly set, the server initializes this variable to the time zone specified by its system environment. See Section 8.5.3 for more information.

https://www.postgresql.org/docs/9.1/runtime-config-client.html

@sfackler
Copy link
Owner

The connection configuration options match what libpq exposes. I think you can set this in the options key: https://docs.rs/tokio-postgres/0.5.0-alpha.1/tokio_postgres/config/struct.Config.html#method.options.

@aloucks
Copy link
Contributor Author

aloucks commented Nov 16, 2019

Using options didn't work. The hard-coded timezone is set when the connection is initialized in startup, which is causing the failure.

async fn startup<S, T>(stream: &mut StartupStream<S, T>, config: &Config) -> Result<(), Error>
where
S: AsyncRead + AsyncWrite + Unpin,
T: AsyncRead + AsyncWrite + Unpin,
{
let mut params = vec![("client_encoding", "UTF8"), ("timezone", "GMT")];

I verified that libpq does not exhibit this behavior by testing with diesel (using libpq-sys).

Would you be open to removing the timezone param from the initial startup? I can adjust the PR to to do this instead.

@sfackler
Copy link
Owner

That will break serialization and deserialization of time values.

@sfackler
Copy link
Owner

It might be best to just change the timezone to UTC if cockroach supports that since it's equivalent to GMT.

@aloucks aloucks changed the title Allow timezone to be configured via connection properties Change connection initialization timezone to UTC Nov 17, 2019
@aloucks
Copy link
Contributor Author

aloucks commented Nov 17, 2019

@sfackler I updated the default to be UTC.

@sfackler sfackler merged commit 1884b85 into sfackler:master Nov 17, 2019
@sfackler
Copy link
Owner

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants