Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.10.1
    • Component/s: SQL Shell
    • Labels:
      None

      Description

      See the title.
      You can reproduce the bug as follows:

      default> select current_date, current_time();
      ?current_date,  ?current_time_1
      -------------------------------
      2015-03-19,  14:55:39.533999
      (1 rows, 0.043 sec, 0 B selected)
      default>
      default> \set timezone GMT+1
      default> select current_date, current_time();
      ?current_date,  ?current_time_1
      -------------------------------
      2015-03-19,  14:55:55.532
      (1 rows, 0.005 sec, 0 B selected)
      default>
      default> SET TIME ZONE 'GMT+1';
      OK
      default> select current_date, current_time();
      ?current_date,  ?current_time_1
      -------------------------------
      2015-03-19,  06:56:33.213
      (1 rows, 0.005 sec, 0 B selected)
      

        Activity

        Hide
        charsyam DaeMyung Kang added a comment -

        This is because of TajoCli has two client side session variable.
        one is SessionConnection's sessionVarsCache(Actually TajoResultSetBase tries to find TIMEZONE value from here.)
        another is in TajoCliContext.

        and There is another Problem, Tajo SessionVariable can't distinguish insensitive cases.

        1. timezone just is saved as timezone, TajoResultSetBase tries to find "TIMEZONE"
        2. TajoCli pass SessionConnection's sessioVarsCache not TajoCliContext.
        3. ClientSide Variables are saved in TajoCliContext.

        So, I think there are some solutions.
        1. if you want to use TajoCliContext not to communication with TAJO master,
        we need merging interface to use this.

        or

        2. removing ClientSideVar.

        Which one is better? Jihoon Son[Hyunsik Choi

        Show
        charsyam DaeMyung Kang added a comment - This is because of TajoCli has two client side session variable. one is SessionConnection's sessionVarsCache(Actually TajoResultSetBase tries to find TIMEZONE value from here.) another is in TajoCliContext. and There is another Problem, Tajo SessionVariable can't distinguish insensitive cases. 1. timezone just is saved as timezone, TajoResultSetBase tries to find "TIMEZONE" 2. TajoCli pass SessionConnection's sessioVarsCache not TajoCliContext. 3. ClientSide Variables are saved in TajoCliContext. So, I think there are some solutions. 1. if you want to use TajoCliContext not to communication with TAJO master, we need merging interface to use this. or 2. removing ClientSideVar. Which one is better? Jihoon Son [ Hyunsik Choi
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user charsyam opened a pull request:

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

        TAJO-1419: Tsql session command doesn't work

        This has some problems.
        1] case sensitive.
        -> Tajo Session Vars distinguish session key name.
        -> so "\set timezone GMT+1" is not valid command
        -> "\set TIMEZONE GMT+1" is valid.

        2] client side var
        -> currently client side vars are stored in TajoCliContext with set command
        -> but ResultSet just using client session information that are stored in Server Session.
        -> and "SET TIME ZONE 'GMT+1'" command save TIMEZONE Setting as Server Session.
        so If timezone value is stored in client side, it is strange.

        so, I changed TIMEZONE from CLISIDE_VAR to SERVER_VAR.

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

        $ git pull https://github.com/charsyam/tajo feature/TAJO-1419

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

        https://github.com/apache/tajo/pull/452.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 #452


        commit 22750cdcc11c42f7117b543a2933477d0e9be63d
        Author: clark.kang <clark.kang@kakao.com>
        Date: 2015-03-23T03:41:18Z

        fix TAJO-1419


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user charsyam opened a pull request: https://github.com/apache/tajo/pull/452 TAJO-1419 : Tsql session command doesn't work This has some problems. 1] case sensitive. -> Tajo Session Vars distinguish session key name. -> so "\set timezone GMT+1" is not valid command -> "\set TIMEZONE GMT+1" is valid. 2] client side var -> currently client side vars are stored in TajoCliContext with set command -> but ResultSet just using client session information that are stored in Server Session. -> and "SET TIME ZONE 'GMT+1'" command save TIMEZONE Setting as Server Session. so If timezone value is stored in client side, it is strange. so, I changed TIMEZONE from CLISIDE_VAR to SERVER_VAR. You can merge this pull request into a Git repository by running: $ git pull https://github.com/charsyam/tajo feature/ TAJO-1419 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/452.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 #452 commit 22750cdcc11c42f7117b543a2933477d0e9be63d Author: clark.kang <clark.kang@kakao.com> Date: 2015-03-23T03:41:18Z fix TAJO-1419
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on the pull request:

        https://github.com/apache/tajo/pull/452#issuecomment-92007354

        Hi @charsyam, sorry for late review.

        In Tajo, there are three different configurations for timezone. One of them is the session configuration which you removed. We should not remove it.
        For more detailed explanation, please see http://tajo.apache.org/docs/current/time_zone.html.

        In this issue, the problem is that ```\set``` command does not work. We should investigate problems from there.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/452#issuecomment-92007354 Hi @charsyam, sorry for late review. In Tajo, there are three different configurations for timezone. One of them is the session configuration which you removed. We should not remove it. For more detailed explanation, please see http://tajo.apache.org/docs/current/time_zone.html . In this issue, the problem is that ```\set``` command does not work. We should investigate problems from there.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user charsyam commented on the pull request:

        https://github.com/apache/tajo/pull/452#issuecomment-92056624

        @jihoonson, Thank you for your review

        There are some problems.
        Actually, in Tajo,

        1. Client Side Var is not working with ResultSet now.
        Because, getCliSideVar() and getClientSideSessionVars() see different vairable map;

        2. in "SET TIME ZONE 'GMT+1' case
        It is parsed and executed in Server Side. but in tajo, There is no ability to set client side var from server.
        so, currently, SET TIME ZONE commands just set TIMEZONE into Server Side Variable.

        That's why I just change TIMEZONE as Server Side Var.

        Show
        githubbot ASF GitHub Bot added a comment - Github user charsyam commented on the pull request: https://github.com/apache/tajo/pull/452#issuecomment-92056624 @jihoonson, Thank you for your review There are some problems. Actually, in Tajo, 1. Client Side Var is not working with ResultSet now. Because, getCliSideVar() and getClientSideSessionVars() see different vairable map; 2. in "SET TIME ZONE 'GMT+1' case It is parsed and executed in Server Side. but in tajo, There is no ability to set client side var from server. so, currently, SET TIME ZONE commands just set TIMEZONE into Server Side Variable. That's why I just change TIMEZONE as Server Side Var.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on the pull request:

        https://github.com/apache/tajo/pull/452#issuecomment-92070150

        Thank you for investigating this issue.
        IMO, the problems you found are all bugs, so, we need to fix them.
        Please refer to https://issues.apache.org/jira/browse/TAJO-1234 and https://issues.apache.org/jira/browse/TAJO-1241.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/452#issuecomment-92070150 Thank you for investigating this issue. IMO, the problems you found are all bugs, so, we need to fix them. Please refer to https://issues.apache.org/jira/browse/TAJO-1234 and https://issues.apache.org/jira/browse/TAJO-1241 .
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user charsyam commented on the pull request:

        https://github.com/apache/tajo/pull/452#issuecomment-94243867

        @jihoonson I changed TIMEZONE's type to DEFAULT

        Show
        githubbot ASF GitHub Bot added a comment - Github user charsyam commented on the pull request: https://github.com/apache/tajo/pull/452#issuecomment-94243867 @jihoonson I changed TIMEZONE's type to DEFAULT
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on the pull request:

        https://github.com/apache/tajo/pull/452#issuecomment-94260999

        Thanks @charsyam. We had a discussion about the session variable system in Tajo, and finally found that the variable type of ```TIMEZONE``` should be ```DEFAULT``` rather than ```CLI_SIDE_VAR```.
        This patch looks good to me, but I think that it would be great if we have a test code for this change.
        Would you mind adding a test?

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/452#issuecomment-94260999 Thanks @charsyam. We had a discussion about the session variable system in Tajo, and finally found that the variable type of ```TIMEZONE``` should be ```DEFAULT``` rather than ```CLI_SIDE_VAR```. This patch looks good to me, but I think that it would be great if we have a test code for this change. Would you mind adding a test?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on the pull request:

        https://github.com/apache/tajo/pull/452#issuecomment-94423419

        Thanks @charsyam. Newly added tests look great, but it will be perfect if you add a line to verify query results which are changed according to the timezone session value.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/452#issuecomment-94423419 Thanks @charsyam. Newly added tests look great, but it will be perfect if you add a line to verify query results which are changed according to the timezone session value.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user charsyam commented on the pull request:

        https://github.com/apache/tajo/pull/452#issuecomment-94481546

        @jihoonson I added new testcases for TIMEZONE in CLI Thanks.

        Show
        githubbot ASF GitHub Bot added a comment - Github user charsyam commented on the pull request: https://github.com/apache/tajo/pull/452#issuecomment-94481546 @jihoonson I added new testcases for TIMEZONE in CLI Thanks.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on the pull request:

        https://github.com/apache/tajo/pull/452#issuecomment-94568443

        +1 LGTM!
        I'll commit shortly.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/452#issuecomment-94568443 +1 LGTM! I'll commit shortly.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

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

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

        Committed branches 0.10.1 and master.
        Thanks for your contribution!

        Show
        jihoonson Jihoon Son added a comment - Committed branches 0.10.1 and master. Thanks for your contribution!
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-CODEGEN-build #321 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/321/)
        TAJO-1419: Tsql session command doesn't work. (jihoonson: rev 2a5d9128702d5ef5ab16405c0e4f4b296fa1d896)

        • tajo-core/src/test/resources/results/TestTajoCli/testHelpSessionVars.result
        • CHANGES
        • tajo-common/src/main/java/org/apache/tajo/SessionVars.java
        • tajo-core/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #321 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/321/ ) TAJO-1419 : Tsql session command doesn't work. (jihoonson: rev 2a5d9128702d5ef5ab16405c0e4f4b296fa1d896) tajo-core/src/test/resources/results/TestTajoCli/testHelpSessionVars.result CHANGES tajo-common/src/main/java/org/apache/tajo/SessionVars.java tajo-core/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #683 (See https://builds.apache.org/job/Tajo-master-build/683/)
        TAJO-1419: Tsql session command doesn't work. (jihoonson: rev 2a5d9128702d5ef5ab16405c0e4f4b296fa1d896)

        • tajo-core/src/test/resources/results/TestTajoCli/testHelpSessionVars.result
        • CHANGES
        • tajo-core/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java
        • tajo-common/src/main/java/org/apache/tajo/SessionVars.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #683 (See https://builds.apache.org/job/Tajo-master-build/683/ ) TAJO-1419 : Tsql session command doesn't work. (jihoonson: rev 2a5d9128702d5ef5ab16405c0e4f4b296fa1d896) tajo-core/src/test/resources/results/TestTajoCli/testHelpSessionVars.result CHANGES tajo-core/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java tajo-common/src/main/java/org/apache/tajo/SessionVars.java

          People

          • Assignee:
            charsyam DaeMyung Kang
            Reporter:
            jihoonson Jihoon Son
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development