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

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

Closed
hyunsik wants to merge 7 commits into
apache:masterfrom
hyunsik:TAJO-1234
Closed

TAJO-1234: Rearrange timezone in date/time types.#290
hyunsik wants to merge 7 commits into
apache:masterfrom
hyunsik:TAJO-1234

Conversation

@hyunsik
Copy link
Copy Markdown
Member

@hyunsik hyunsik commented Dec 6, 2014

See the jira (https://issues.apache.org/jira/browse/TAJO-1234). I explained there.

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Dec 7, 2014

I've updated the patch.

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Dec 7, 2014

This is an urgent issue. So, I would be happy if anyone reviews it as soon as possible.

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Dec 7, 2014

I've updated the documentation.

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Dec 7, 2014

I found that some unit tests are sensitive to local time zone. I'll update the patch soon

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Dec 7, 2014

I've updated the patch.

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Dec 8, 2014

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'.

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Dec 8, 2014

In addition, I added the table property 'timezone'. You can use it in your CREATE TABLE statement as follows:

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about using TajoConstants.UTC_TIMEZONE?

@blrunner
Copy link
Copy Markdown
Contributor

blrunner commented Dec 8, 2014

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

@jihoonson
Copy link
Copy Markdown
Contributor

@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.

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Dec 8, 2014

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 DEFAULT_SYSTEM_TIMEZONE.

I'll commit it shortly due to unit test failure. If there are further feedback, I'll reflect them later in another jira.

@asfgit asfgit closed this in facd1dd Dec 8, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants