You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by hyunsik <gi...@git.apache.org> on 2014/12/06 15:31:19 UTC

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

GitHub user hyunsik opened a pull request:

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

    TAJO-1234: Rearrange timezone in date/time types.

    

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

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

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

    https://github.com/apache/tajo/pull/290.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 #290
    
----
commit 9e24edbfbaa75bcb7aae1b83e097450048b788bd
Author: Hyunsik Choi <hy...@apache.org>
Date:   2014-12-06T14:30:28Z

    TAJO-1234: Rearrange timezone in date/time types.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/290#discussion_r21434576
  
    --- Diff: tajo-common/src/test/java/org/apache/tajo/datum/TestTimestampDatum.java ---
    @@ -151,7 +150,7 @@ public final void testTimestampConstructor() {
         assertEquals(datum2, datum);
     
         for (int i = 0; i < 100; i++) {
    -      TimeZone timeZone = TimeZone.getTimeZone("UTC");
    +      TimeZone timeZone = TimeZone.getTimeZone("GMT");
    --- End diff --
    
    How about using TajoConstants.UTC_TIMEZONE?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/290#issuecomment-65927813
  
    I've updated the documentation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:

    https://github.com/apache/tajo/pull/290#issuecomment-66032054
  
    Thanks @hyunsik 
    
    +1 for the patch.
    
    All unit test cases finished successfully on three timezones such as Asia/Seoul, America/Toronto,Europe/Paris.
    
    After considering @jihoonson's comment, push it. :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/290#issuecomment-65928449
  
    I found that some unit tests are sensitive to local time zone. I'll update the patch soon 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/290#issuecomment-65938801
  
    I've updated the patch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/290#issuecomment-65926497
  
    This is an urgent issue. So, I would be happy if anyone review it as soon as possible.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:

    https://github.com/apache/tajo/pull/290#issuecomment-66032354
  
    @blrunner, thanks for your testing. 
    In my computer, unit tests failed due to another problem of my machine, so I couldn't verify the patch. 
    The patch looks good to me, too. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/290#issuecomment-65966314
  
    In addition, I added the table property 'timezone'. You can use it in your ``CREATE TABLE`` statement as follows:
    
    ```sql
    CREATE EXTERNAL TABLE table1 (
     t_timestamp  TIMESTAMP,
     t_time    TIME,
     t_date    DATE
    ) USING TEXTFILE WITH('text.delimiter'='|', 'timezone'='ASIA/Seoul') LOCATION '/path-to-table/'
    ```
    
    It will translate timestamp and time value by a specified time zone. I also added the detailed documentation about time zone property.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/290#issuecomment-66033616
  
    Thank you all guys for the quick review.
    
    @jihoonson TajoConstants.UTC_TIMEZONE is used for a system default config. In contrast, the unit tests were written to test the behavior of different time zones. So, I'll change the constant name to SYSTEM_DEFAULT_TIMEZONE.
    
    I'll commit it shortly due to unit test failure. If there are further feedback, I'll reflect them later in another jira.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/290#issuecomment-65965102
  
    Java 6 does not recognize 'UTC+9' or some location-based time zone like 'Asia/Seoul' because it has scarce time zone databases.
    
    I fixed the problematic unit tests to use the prefix 'GMT' instead of 'UTC'.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-1234: Rearrange timezone in date/time type...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/290#issuecomment-65926468
  
    I've updated the patch. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---