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

sql: default time zone should be customizable #16029

Closed
myaniu opened this issue May 18, 2017 · 11 comments
Closed

sql: default time zone should be customizable #16029

myaniu opened this issue May 18, 2017 · 11 comments
Labels
A-sql-execution Relating to SQL execution. A-sql-semantics C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) no-issue-activity O-community Originated from the community

Comments

@myaniu
Copy link

myaniu commented May 18, 2017

cockroach version : 1.0
when cockroach started, default time zone is wrong.
my mac's default time zone is UTC+8
cockroach's default time zone is UTC

I170519 06:05:59.177794 1 util/log/clog.go:1011 [config] file created at: 2017/05/19 06:05:59
I170519 06:05:59.177794 1 util/log/clog.go:1011 [config] running on machine: df-macpro-home
I170519 06:05:59.177794 1 util/log/clog.go:1011 [config] binary: CockroachDB CCL v1.0 (darwin amd64, built 2017/05/18 21:50:42, go1.8.1)
I170519 06:05:59.177794 1 util/log/clog.go:1011 [config] arguments: [cockroach start --insecure --host=localhost]

@bdarnell
Copy link
Contributor

The default time zone in CockroachDB is always UTC and cannot (currently) be configured. We don't simply use the system time zone of the server because that may be configured differently across different nodes in the cluster, which would be very confusing. (Also, UTC is generally the right choice for a global application anyway) You can set the time zone for a session with the SET TIME ZONE command. Some client drivers have the option to set this automatically for you.

@leomkkwan
Copy link

Coming from Mysql, it will be very useful if we can set the default time zone on CockroachDB. Hopefully, the will be implement in the near future.

@bdarnell
Copy link
Contributor

Retitled this issue. In mysql, it's common for client drivers to support setting the session time zone automatically, but this appears to be less common for postgres, so there's probably more need for a global time zone setting than I previously thought.

We have cluster settings now, but I'm not sure if they're safe for the time zone setting as currently implemented. This would be the first setting (IIRC) that changes the semantics of SQL operations. Do we guarantee that newly-started nodes wait for the cluster settings to be available before serving any traffic, or is it possible for a node to serve its first few queries using the defaults? cc @dt

@dt
Copy link
Member

dt commented May 19, 2017

I think a node currently can start serving queries before receiving gossiped settings, or certainly at least before loading received settings, given that that happens on a dedicated goroutine.

We could introduce a wait group and block the serving startup, but, if a brief read of a default would cause a problem though for some setting, it suggests that changing the setting, which also happens though gossip and thus might arrive at a particular node a before or after some other node, would also be a problem, and the maybe that parameter just isn't a good fit for a runtime-changable setting.

We could introduce another class of setting that is read transactionally whenever it is used, but that would obviously be (prohibitively?) expensive.

@bdarnell
Copy link
Contributor

I think it's OK that changing the time zone would have inconsistent effects across the cluster, but right now it wouldn't be safe to use a non-default time zone in the presence of any sort of automated provisioning of new servers, even if you set it once and never change it.

However, the presence of limitations like the inability to change a setting at run time is one reason why setting a database-level time zone should be discouraged in favor of controlling the time zone in the application. It is low priority IMHO to provide this feature in the database.

@dt
Copy link
Member

dt commented May 19, 2017

Oh, and more specifically for this issue:
Given how unintuitive the behavior of pg's time types gets when using non-zero session offsets, I'm reluctant to introduce something to default sessions to non-zero offsets, particularly the ability to change it at runtime: that makes it real easy to break existing clients and their traffic in a subtle and potentially easy to miss way (silently shifting timestamps when read), which can get hard to clean up if they're writing those back to other tables / caches / etc, using an ORM that saves whole records on every .save call, etc

@knz
Copy link
Contributor

knz commented May 22, 2017

I think we have #15301 filed already for initial settings.

@dianasaur323 dianasaur323 added the O-community Originated from the community label May 25, 2017
@dianasaur323 dianasaur323 added this to the Later milestone May 25, 2017
@bdarnell bdarnell changed the title when cockroach started, default time zone is wrong. sql: default time zone should be customizable Jul 13, 2017
@Marza
Copy link

Marza commented Nov 7, 2017

I think that only allowing UTC on database level makes sense, if you want to present data in some other timezone (possibly multiple timezones) then that is up to the application/frontend to convert and present.

@bdarnell
Copy link
Contributor

bdarnell commented Jan 2, 2018

The postgres ALTER USER foo SET timezone='...' statement (#21151) may be the best solution here, given the issues with cluster setting initialization.

@knz knz added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Apr 27, 2018
@knz knz added A-sql-semantics A-sql-execution Relating to SQL execution. labels May 15, 2018
@petermattis petermattis removed this from the Later milestone Oct 5, 2018
@github-actions
Copy link

github-actions bot commented Jun 9, 2021

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

@knz
Copy link
Contributor

knz commented Jun 9, 2021

This issue is superseded by the following:

@knz knz closed this as completed Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-execution Relating to SQL execution. A-sql-semantics C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) no-issue-activity O-community Originated from the community
Projects
None yet
Development

No branches or pull requests

8 participants