-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[IOTDB-872] Use system timezone in CLI (Session) #1846
Conversation
merge master
} | ||
RpcUtils.verifySuccess(resp.getStatus()); | ||
return resp.getTimeZone(); | ||
return ZoneId.systemDefault().getId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is something wrong with the logic.
It should be the default value that the server stores when the client is not registered with the server.
} | ||
|
||
private synchronized void setTimeZone(String zoneId) | ||
public synchronized void setTimeZone(String zoneId) | ||
throws StatementExecutionException, IoTDBConnectionException { | ||
TSSetTimeZoneReq req = new TSSetTimeZoneReq(sessionId, zoneId); | ||
TSStatus resp; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There may be a bug here, because the value stored by the server does not necessarily change when an error is encountered, but the local cached zoneId does
session/src/test/java/org/apache/iotdb/session/pool/SessionPoolTest.java
Show resolved
Hide resolved
Consider add some setting timezone tests in |
Added~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Remove the default timezone in server side.
Using the system default time zone of each client to decide the time zone of server operation.