Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

TAJO-1241: Change default client and table time zone behavior. #295

Closed
wants to merge 15 commits into from

Conversation

hyunsik
Copy link
Member

@hyunsik hyunsik commented Dec 11, 2014

By default, TajoClient uses GMT as client time zone unless session variable TIMEZONE is specified. Also, by default Table uses GMT as table time zone unless table property timezone is specified.

This patch changes these default behavior as follows:

  • TajoClient will use TimeZone.getDefault() by default.
  • Table implicitly uses tajo.timezone of TajoConf by default.
    • In other words, this default time zone does not affect the table property in catalog.

I also added the documentation about time zone.

…into TZ_BUG

Conflicts:
	tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java
	tajo-client/src/main/proto/ClientProtos.proto
	tajo-core/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java
	tajo-core/src/main/java/org/apache/tajo/master/TajoMasterClientService.java
	tajo-core/src/test/java/org/apache/tajo/engine/query/TestSetSessionQuery.java
	tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone1.result
	tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone2.result
	tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone3.result
throw new RuntimeException("No such session variable" + varname);
void updateSessionVarsCache(Map<String, String> variables) {
synchronized (sessionVarsCache) {
this.sessionVarsCache.clear();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why all cached variables are cleared before update.
Whenever a set command is executed, a new hash map instance is passed from the client.
(Please see SetCommand.updateSessionVariable().)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because we use synchronized feature against sessionVarsCache. In this case, if we replace the object by another object, the synchronization will not work correctly. It will cause some concurrency problem.

@jihoonson
Copy link
Contributor

Hi @hyunsik,
In overall, the patch looks good.
I left one trivial comment and a question.
I'll wait for your reply.
Thanks.

@jihoonson
Copy link
Contributor

+1!
Thanks for your reply.

@asfgit asfgit closed this in 758927e Dec 12, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants