Uploaded image for project: 'Tajo'
  1. Tajo
  2. TAJO-1241

Change default client and table time zone behavior

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.10.0
    • Component/s: Storage
    • Labels:
      None

      Description

      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 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.
      http://people.apache.org/~hyunsik/timezone/time_zone.html

      1. TAJO-1241.patch
        58 kB
        Hyunsik Choi
      2. TAJO-1241_3.patch
        66 kB
        Hyunsik Choi
      3. TAJO-1241_2.patch
        66 kB
        Hyunsik Choi

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Tajo-master-build #498 (See https://builds.apache.org/job/Tajo-master-build/498/)
          TAJO-1241: Change documentaiton url of devel and current versions. (hyunsik: rev 758927e5aed12d390b02198525b812b838cd2ba3)

          • tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone2.result
          • tajo-client/src/main/java/org/apache/tajo/client/QueryClientImpl.java
          • tajo-core/src/main/java/org/apache/tajo/master/TajoMasterClientService.java
          • tajo-core/src/test/java/org/apache/tajo/engine/eval/ExprTestBase.java
          • tajo-docs/src/main/sphinx/configuration/service_config_defaults.rst
          • tajo-core/src/test/resources/results/TestTajoCli/testHelpSessionVars.result
          • tajo-docs/src/main/sphinx/index.rst
          • tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/DatePartFromTimestamp.java
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java
          • tajo-common/src/main/java/org/apache/tajo/SessionVars.java
          • tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/ToTimestampText.java
          • tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java
          • tajo-core/src/test/java/org/apache/tajo/engine/query/TestSelectQuery.java
          • tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone3.result
          • tajo-docs/src/main/sphinx/time_zone.rst
          • tajo-core/src/test/java/org/apache/tajo/TajoTestingCluster.java
          • tajo-core/src/test/java/org/apache/tajo/engine/query/TestSetSessionQuery.java
          • tajo-docs/src/main/sphinx/table_management/table_overview.rst
          • tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable4.sql
          • tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone1.result
          • tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/DatePartFromTime.java
          • tajo-core/src/test/java/org/apache/tajo/engine/function/TestDateTimeFunctions.java
          • tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/SeqScanExec.java
          • tajo-docs/src/main/sphinx/configuration/configuration_defaults.rst
          • tajo-storage/src/main/java/org/apache/tajo/storage/TextSerializerDeserializer.java
          • tajo-core/src/test/java/org/apache/tajo/QueryTestCaseBase.java
          • tajo-storage/src/main/java/org/apache/tajo/storage/StorageUtil.java
          • tajo-core/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java
          • tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/CurrentDate.java
          • tajo-core/src/test/java/org/apache/tajo/engine/eval/TestSQLExpression.java
          • tajo-plan/src/main/java/org/apache/tajo/plan/util/PlannerUtil.java
          • tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable5.sql
          • tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable2.sql
          • tajo-core/src/test/resources/results/TestTajoCli/testSetTimezone1.result
          • CHANGES
          • tajo-plan/src/main/java/org/apache/tajo/plan/expr/CastEval.java
          • tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java
          • tajo-docs/src/main/sphinx/configuration/catalog-site-xml.rst
          • tajo-client/src/main/proto/ClientProtos.proto
          • tajo-plan/src/main/java/org/apache/tajo/plan/ExprAnnotator.java
          • tajo-docs/src/main/sphinx/configuration.rst
          • tajo-client/src/main/java/org/apache/tajo/client/TajoClientUtil.java
          • tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
          • tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/ToCharTimestamp.java
          • tajo-core/src/main/java/org/apache/tajo/master/GlobalEngine.java
          • tajo-docs/src/main/sphinx/configuration/tajo-site-xml.rst
          • tajo-client/src/main/java/org/apache/tajo/jdbc/TajoResultSetBase.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #498 (See https://builds.apache.org/job/Tajo-master-build/498/ ) TAJO-1241 : Change documentaiton url of devel and current versions. (hyunsik: rev 758927e5aed12d390b02198525b812b838cd2ba3) tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone2.result tajo-client/src/main/java/org/apache/tajo/client/QueryClientImpl.java tajo-core/src/main/java/org/apache/tajo/master/TajoMasterClientService.java tajo-core/src/test/java/org/apache/tajo/engine/eval/ExprTestBase.java tajo-docs/src/main/sphinx/configuration/service_config_defaults.rst tajo-core/src/test/resources/results/TestTajoCli/testHelpSessionVars.result tajo-docs/src/main/sphinx/index.rst tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/DatePartFromTimestamp.java tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java tajo-common/src/main/java/org/apache/tajo/SessionVars.java tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/ToTimestampText.java tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java tajo-core/src/test/java/org/apache/tajo/engine/query/TestSelectQuery.java tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone3.result tajo-docs/src/main/sphinx/time_zone.rst tajo-core/src/test/java/org/apache/tajo/TajoTestingCluster.java tajo-core/src/test/java/org/apache/tajo/engine/query/TestSetSessionQuery.java tajo-docs/src/main/sphinx/table_management/table_overview.rst tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable4.sql tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone1.result tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/DatePartFromTime.java tajo-core/src/test/java/org/apache/tajo/engine/function/TestDateTimeFunctions.java tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/SeqScanExec.java tajo-docs/src/main/sphinx/configuration/configuration_defaults.rst tajo-storage/src/main/java/org/apache/tajo/storage/TextSerializerDeserializer.java tajo-core/src/test/java/org/apache/tajo/QueryTestCaseBase.java tajo-storage/src/main/java/org/apache/tajo/storage/StorageUtil.java tajo-core/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/CurrentDate.java tajo-core/src/test/java/org/apache/tajo/engine/eval/TestSQLExpression.java tajo-plan/src/main/java/org/apache/tajo/plan/util/PlannerUtil.java tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable5.sql tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable2.sql tajo-core/src/test/resources/results/TestTajoCli/testSetTimezone1.result CHANGES tajo-plan/src/main/java/org/apache/tajo/plan/expr/CastEval.java tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java tajo-docs/src/main/sphinx/configuration/catalog-site-xml.rst tajo-client/src/main/proto/ClientProtos.proto tajo-plan/src/main/java/org/apache/tajo/plan/ExprAnnotator.java tajo-docs/src/main/sphinx/configuration.rst tajo-client/src/main/java/org/apache/tajo/client/TajoClientUtil.java tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/ToCharTimestamp.java tajo-core/src/main/java/org/apache/tajo/master/GlobalEngine.java tajo-docs/src/main/sphinx/configuration/tajo-site-xml.rst tajo-client/src/main/java/org/apache/tajo/jdbc/TajoResultSetBase.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Tajo-master-CODEGEN-build #139 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/139/)
          TAJO-1241: Change documentaiton url of devel and current versions. (hyunsik: rev 758927e5aed12d390b02198525b812b838cd2ba3)

          • tajo-plan/src/main/java/org/apache/tajo/plan/ExprAnnotator.java
          • tajo-core/src/test/java/org/apache/tajo/engine/query/TestSelectQuery.java
          • tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/ToTimestampText.java
          • tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone3.result
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java
          • tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/CurrentDate.java
          • tajo-common/src/main/java/org/apache/tajo/SessionVars.java
          • tajo-docs/src/main/sphinx/configuration.rst
          • tajo-client/src/main/proto/ClientProtos.proto
          • tajo-core/src/test/java/org/apache/tajo/engine/function/TestDateTimeFunctions.java
          • tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone1.result
          • tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java
          • tajo-docs/src/main/sphinx/configuration/tajo-site-xml.rst
          • tajo-docs/src/main/sphinx/configuration/configuration_defaults.rst
          • tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/ToCharTimestamp.java
          • tajo-storage/src/main/java/org/apache/tajo/storage/TextSerializerDeserializer.java
          • tajo-core/src/main/java/org/apache/tajo/master/GlobalEngine.java
          • tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable4.sql
          • tajo-core/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java
          • tajo-core/src/main/java/org/apache/tajo/master/TajoMasterClientService.java
          • tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java
          • CHANGES
          • tajo-docs/src/main/sphinx/time_zone.rst
          • tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/DatePartFromTime.java
          • tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
          • tajo-core/src/test/java/org/apache/tajo/TajoTestingCluster.java
          • tajo-plan/src/main/java/org/apache/tajo/plan/util/PlannerUtil.java
          • tajo-client/src/main/java/org/apache/tajo/jdbc/TajoResultSetBase.java
          • tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/DatePartFromTimestamp.java
          • tajo-core/src/test/java/org/apache/tajo/engine/eval/TestSQLExpression.java
          • tajo-plan/src/main/java/org/apache/tajo/plan/expr/CastEval.java
          • tajo-core/src/test/resources/results/TestTajoCli/testHelpSessionVars.result
          • tajo-core/src/test/java/org/apache/tajo/engine/eval/ExprTestBase.java
          • tajo-core/src/test/resources/results/TestTajoCli/testSetTimezone1.result
          • tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable5.sql
          • tajo-docs/src/main/sphinx/table_management/table_overview.rst
          • tajo-docs/src/main/sphinx/configuration/service_config_defaults.rst
          • tajo-core/src/test/java/org/apache/tajo/QueryTestCaseBase.java
          • tajo-core/src/test/java/org/apache/tajo/engine/query/TestSetSessionQuery.java
          • tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/SeqScanExec.java
          • tajo-client/src/main/java/org/apache/tajo/client/QueryClientImpl.java
          • tajo-docs/src/main/sphinx/index.rst
          • tajo-docs/src/main/sphinx/configuration/catalog-site-xml.rst
          • tajo-client/src/main/java/org/apache/tajo/client/TajoClientUtil.java
          • tajo-storage/src/main/java/org/apache/tajo/storage/StorageUtil.java
          • tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone2.result
          • tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable2.sql
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #139 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/139/ ) TAJO-1241 : Change documentaiton url of devel and current versions. (hyunsik: rev 758927e5aed12d390b02198525b812b838cd2ba3) tajo-plan/src/main/java/org/apache/tajo/plan/ExprAnnotator.java tajo-core/src/test/java/org/apache/tajo/engine/query/TestSelectQuery.java tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/ToTimestampText.java tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone3.result tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/CurrentDate.java tajo-common/src/main/java/org/apache/tajo/SessionVars.java tajo-docs/src/main/sphinx/configuration.rst tajo-client/src/main/proto/ClientProtos.proto tajo-core/src/test/java/org/apache/tajo/engine/function/TestDateTimeFunctions.java tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone1.result tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java tajo-docs/src/main/sphinx/configuration/tajo-site-xml.rst tajo-docs/src/main/sphinx/configuration/configuration_defaults.rst tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/ToCharTimestamp.java tajo-storage/src/main/java/org/apache/tajo/storage/TextSerializerDeserializer.java tajo-core/src/main/java/org/apache/tajo/master/GlobalEngine.java tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable4.sql tajo-core/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java tajo-core/src/main/java/org/apache/tajo/master/TajoMasterClientService.java tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java CHANGES tajo-docs/src/main/sphinx/time_zone.rst tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/DatePartFromTime.java tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java tajo-core/src/test/java/org/apache/tajo/TajoTestingCluster.java tajo-plan/src/main/java/org/apache/tajo/plan/util/PlannerUtil.java tajo-client/src/main/java/org/apache/tajo/jdbc/TajoResultSetBase.java tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/DatePartFromTimestamp.java tajo-core/src/test/java/org/apache/tajo/engine/eval/TestSQLExpression.java tajo-plan/src/main/java/org/apache/tajo/plan/expr/CastEval.java tajo-core/src/test/resources/results/TestTajoCli/testHelpSessionVars.result tajo-core/src/test/java/org/apache/tajo/engine/eval/ExprTestBase.java tajo-core/src/test/resources/results/TestTajoCli/testSetTimezone1.result tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable5.sql tajo-docs/src/main/sphinx/table_management/table_overview.rst tajo-docs/src/main/sphinx/configuration/service_config_defaults.rst tajo-core/src/test/java/org/apache/tajo/QueryTestCaseBase.java tajo-core/src/test/java/org/apache/tajo/engine/query/TestSetSessionQuery.java tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/SeqScanExec.java tajo-client/src/main/java/org/apache/tajo/client/QueryClientImpl.java tajo-docs/src/main/sphinx/index.rst tajo-docs/src/main/sphinx/configuration/catalog-site-xml.rst tajo-client/src/main/java/org/apache/tajo/client/TajoClientUtil.java tajo-storage/src/main/java/org/apache/tajo/storage/StorageUtil.java tajo-core/src/test/resources/results/TestSQLAnalyzer/settimezone2.result tajo-core/src/test/resources/queries/TestSelectQuery/testTimezonedTable2.sql
          Hide
          hyunsik Hyunsik Choi added a comment -

          Thank you for the review. I just committed the latest patch to master branch.

          Show
          hyunsik Hyunsik Choi added a comment - Thank you for the review. I just committed the latest patch to master branch.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/tajo/pull/295

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/295
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on the pull request:

          https://github.com/apache/tajo/pull/295#issuecomment-66644587

          +1!
          Thanks for your reply.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/295#issuecomment-66644587 +1! Thanks for your reply.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hyunsik commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/295#discussion_r21684970

          — Diff: tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java —
          @@ -258,11 +232,10 @@ public String call(NettyClientBase client) throws ServiceException {
          }.withRetries();
          }

          • public String getCachedSessionVariable(final String varname) {
          • if (sessionVarsCache.containsKey(varname)) { - return sessionVarsCache.get(varname); - }

            else {

          • throw new RuntimeException("No such session variable" + varname);
            + void updateSessionVarsCache(Map<String, String> variables) {
            + synchronized (sessionVarsCache) {
            + this.sessionVarsCache.clear();
              • End diff –

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on a diff in the pull request: https://github.com/apache/tajo/pull/295#discussion_r21684970 — Diff: tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java — @@ -258,11 +232,10 @@ public String call(NettyClientBase client) throws ServiceException { }.withRetries(); } public String getCachedSessionVariable(final String varname) { if (sessionVarsCache.containsKey(varname)) { - return sessionVarsCache.get(varname); - } else { throw new RuntimeException("No such session variable" + varname); + void updateSessionVarsCache(Map<String, String> variables) { + synchronized (sessionVarsCache) { + this.sessionVarsCache.clear(); End diff – This is because we use synchronized feature against sessionVarsCache. In this case, if we replace the object, the synchronization will not work correctly. It will cause some concurrency problem.
          Hide
          tajoqa Tajo QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12686598/TAJO-1241_3.patch
          against master revision release-0.9.0-rc0-87-g3c273b5.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 15 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

          +1 checkstyle. The patch generated 0 code style errors.

          -1 findbugs. The patch appears to cause Findbugs (version 2.0.3) to fail.

          -1 release audit. The applied patch generated 184 release audit warnings.

          +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-common tajo-client tajo-common tajo-core tajo-plan tajo-storage.

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/543//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/543//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
          Findbugs results: https://builds.apache.org/job/PreCommit-TAJO-Build/543//findbugsResult
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/543//console

          This message is automatically generated.

          Show
          tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12686598/TAJO-1241_3.patch against master revision release-0.9.0-rc0-87-g3c273b5. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to cause Findbugs (version 2.0.3) to fail. -1 release audit. The applied patch generated 184 release audit warnings. +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-common tajo-client tajo-common tajo-core tajo-plan tajo-storage. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/543//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/543//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Findbugs results: https://builds.apache.org/job/PreCommit-TAJO-Build/543//findbugsResult Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/543//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on the pull request:

          https://github.com/apache/tajo/pull/295#issuecomment-66634540

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/295#issuecomment-66634540 Hi @hyunsik, In overall, the patch looks good. I left one trivial comment and a question. I'll wait for your reply. Thanks.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/295#discussion_r21682401

          — Diff: tajo-storage/src/main/java/org/apache/tajo/storage/StorageUtil.java —
          @@ -27,6 +27,7 @@
          import org.apache.tajo.catalog.Schema;
          import org.apache.tajo.catalog.TableMeta;
          import org.apache.tajo.catalog.proto.CatalogProtos;
          +import org.apache.tajo.conf.TajoConf;
          — End diff –

          Please remove the unused import.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/295#discussion_r21682401 — Diff: tajo-storage/src/main/java/org/apache/tajo/storage/StorageUtil.java — @@ -27,6 +27,7 @@ import org.apache.tajo.catalog.Schema; import org.apache.tajo.catalog.TableMeta; import org.apache.tajo.catalog.proto.CatalogProtos; +import org.apache.tajo.conf.TajoConf; — End diff – Please remove the unused import.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/295#discussion_r21681310

          — Diff: tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java —
          @@ -258,11 +232,10 @@ public String call(NettyClientBase client) throws ServiceException {
          }.withRetries();
          }

          • public String getCachedSessionVariable(final String varname) {
          • if (sessionVarsCache.containsKey(varname)) { - return sessionVarsCache.get(varname); - }

            else {

          • throw new RuntimeException("No such session variable" + varname);
            + void updateSessionVarsCache(Map<String, String> variables) {
            + synchronized (sessionVarsCache) {
            + this.sessionVarsCache.clear();
              • End diff –

          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().)

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/295#discussion_r21681310 — Diff: tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java — @@ -258,11 +232,10 @@ public String call(NettyClientBase client) throws ServiceException { }.withRetries(); } public String getCachedSessionVariable(final String varname) { if (sessionVarsCache.containsKey(varname)) { - return sessionVarsCache.get(varname); - } else { throw new RuntimeException("No such session variable" + varname); + void updateSessionVarsCache(Map<String, String> variables) { + synchronized (sessionVarsCache) { + this.sessionVarsCache.clear(); End diff – 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().)
          Hide
          tajoqa Tajo QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12686458/TAJO-1241_2.patch
          against master revision release-0.9.0-rc0-84-ga0d67bb.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 15 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

          +1 checkstyle. The patch generated 0 code style errors.

          -1 findbugs. The patch appears to introduce 259 new Findbugs (version 2.0.3) warnings.

          -1 release audit. The applied patch generated 121 release audit warnings.

          +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-common tajo-client tajo-common tajo-core tajo-plan tajo-storage.

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/541//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-plan.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-common.html
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/541//console

          This message is automatically generated.

          Show
          tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12686458/TAJO-1241_2.patch against master revision release-0.9.0-rc0-84-ga0d67bb. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 259 new Findbugs (version 2.0.3) warnings. -1 release audit. The applied patch generated 121 release audit warnings. +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-common tajo-client tajo-common tajo-core tajo-plan tajo-storage. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/541//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-plan.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/541//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-common.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/541//console This message is automatically generated.
          Hide
          tajoqa Tajo QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12686420/TAJO-1241.patch
          against master revision release-0.9.0-rc0-84-ga0d67bb.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 15 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

          +1 checkstyle. The patch generated 0 code style errors.

          -1 findbugs. The patch appears to introduce 259 new Findbugs (version 2.0.3) warnings.

          -1 release audit. The applied patch generated 250 release audit warnings.

          +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-common tajo-client tajo-common tajo-core tajo-plan tajo-storage.

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/540//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-plan.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-client.html
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/540//console

          This message is automatically generated.

          Show
          tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12686420/TAJO-1241.patch against master revision release-0.9.0-rc0-84-ga0d67bb. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 259 new Findbugs (version 2.0.3) warnings. -1 release audit. The applied patch generated 250 release audit warnings. +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-common tajo-client tajo-common tajo-core tajo-plan tajo-storage. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/540//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-plan.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/540//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-client.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/540//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user hyunsik opened a pull request:

          https://github.com/apache/tajo/pull/295

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

          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.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/hyunsik/tajo TAJO-1241

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/tajo/pull/295.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #295


          commit 021c872fbfedf90e5eb579dfe109590242fa1bae
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-09T01:00:24Z

          Add session statement to parser.

          commit 56b3016a579896e542a44289e59cc7cb769f8880
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-09T09:09:27Z

          TAJO-1238: Add SET SESSION and RESET statement.

          commit 46a99857a5adc7b30e532b6dd861157be8262d90
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-09T09:09:38Z

          Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1238

          commit 67b4bdedd3dae75db624e85783661c56ca3977cd
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-09T09:47:27Z

          Fix initlal bugs.

          commit 95ad5bea5019d195f0d5a8912f2072f1c18af43e
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-09T09:47:48Z

          Merge branch 'TAJO-1238' of github.com:hyunsik/tajo into TZ_BUG

          commit 73db3e1f0ca8ca7c062f49fc7cb747c8ee4ff120
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-10T00:56:30Z

          Fixed all bugs.

          commit e6239f36a4f70a2ad616f46381479d1a3275da31
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-10T05:05:56Z

          Fixed some compilation bug and removed the reserved word 'default' from antlr.

          commit 78078456483f769382da4785906dbbe08dda2ab6
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-10T05:15:52Z

          Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1238

          commit 467f1ef7e6ad0352700d4d122b703eec77823565
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-10T07:36:25Z

          Merge branch 'TAJO-1238' of github.com:hyunsik/tajo into TZ_BUG

          commit fb981a1ba64d0d233646edf716ceba678620c7bc
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-10T08:17:03Z

          Fixed test bugs.

          commit 601745d37bddcab39642b526a41de4e6bbc5926c
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-10T20:10:38Z

          Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo 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

          commit 2cd79ee333a19a0b177e5551f83dffa42689e1b5
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-10T20:10:45Z

          Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TZ_BUG

          commit e4a30e4a274ef74094957b391305260ddacbacc7
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2014-12-11T01:49:15Z

          Add documentation.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user hyunsik opened a pull request: https://github.com/apache/tajo/pull/295 TAJO-1241 : Change default client and table time zone behavior. 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. http://people.apache.org/~hyunsik/timezone/time_zone.html You can merge this pull request into a Git repository by running: $ git pull https://github.com/hyunsik/tajo TAJO-1241 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/295.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #295 commit 021c872fbfedf90e5eb579dfe109590242fa1bae Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-09T01:00:24Z Add session statement to parser. commit 56b3016a579896e542a44289e59cc7cb769f8880 Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-09T09:09:27Z TAJO-1238 : Add SET SESSION and RESET statement. commit 46a99857a5adc7b30e532b6dd861157be8262d90 Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-09T09:09:38Z Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1238 commit 67b4bdedd3dae75db624e85783661c56ca3977cd Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-09T09:47:27Z Fix initlal bugs. commit 95ad5bea5019d195f0d5a8912f2072f1c18af43e Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-09T09:47:48Z Merge branch ' TAJO-1238 ' of github.com:hyunsik/tajo into TZ_BUG commit 73db3e1f0ca8ca7c062f49fc7cb747c8ee4ff120 Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-10T00:56:30Z Fixed all bugs. commit e6239f36a4f70a2ad616f46381479d1a3275da31 Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-10T05:05:56Z Fixed some compilation bug and removed the reserved word 'default' from antlr. commit 78078456483f769382da4785906dbbe08dda2ab6 Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-10T05:15:52Z Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1238 commit 467f1ef7e6ad0352700d4d122b703eec77823565 Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-10T07:36:25Z Merge branch ' TAJO-1238 ' of github.com:hyunsik/tajo into TZ_BUG commit fb981a1ba64d0d233646edf716ceba678620c7bc Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-10T08:17:03Z Fixed test bugs. commit 601745d37bddcab39642b526a41de4e6bbc5926c Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-10T20:10:38Z Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo 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 commit 2cd79ee333a19a0b177e5551f83dffa42689e1b5 Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-10T20:10:45Z Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TZ_BUG commit e4a30e4a274ef74094957b391305260ddacbacc7 Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-11T01:49:15Z Add documentation.

            People

            • Assignee:
              hyunsik Hyunsik Choi
              Reporter:
              hyunsik Hyunsik Choi
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development