tajo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jaewoong Jung <jun...@gmail.com>
Subject Re: Timezone issues
Date Thu, 20 Nov 2014 08:24:21 GMT
Wow, this seemingly trivial issue has surprisingly many problems involved.

The most critical one, though, is TimeMeta class. Presumably because
it is poorly documented, it is being used for two different purposes
in Tajo code base. Some code treats it as a date time representation,
which I believe is the original intention, but some treat it as the
human-readable equivalent of TimeDatum by completely ignoring
date-related fields.

For example, DateTimeUtil.date2j(long julianDate, TimeMeta tm), which
converts a julian timestamp to a TimeMeta value, doesn't touch
dayOfMonth, monthOfYear, or years values and just puts all values for
hour and above units in hours field.

There are other minor problems like incorrect comments and absent
default values, but the most critical one is misuse of TimeMeta.

I'll try to break up my patches so that each has a clear and
easy-to-understand goal. Sending this heads-up to let you know
there'll be more issues filed and patches sent than you might expect.

On Wed, Nov 19, 2014 at 10:56 PM, Jaewoong Jung <jungjw@gmail.com> wrote:
> Yeah, after some more research, I found that TImeDatum is a somewhat
> ambiguous data type.
>
> Its original purpose is to represent a time of a day, i.e. hh:mm:ss
> part of an instant. So, it could be viewed as instant data though some
> may argue it's incomplete to fully represent an instant. Anyway, given
> that TimestampDatum has limitation in terms of the time range it can
> represent, and given that Tajo doesn't have DateTime data type,
> clients should be allowed to use it with a timezone.
>
> I'll change the direction and try to address the issue by fixing the
> underflow error.
>
> Thanks for the input. :)
>
> On Wed, Nov 19, 2014 at 10:24 PM, Jong-young Park <eminency@gmail.com> wrote:
>> Hi, Jaewoong.
>>
>> To express time period value, IntervalDatum is existing as I know.
>>
>> So I think it is right that TimeDatum is for some time point.
>>
>> And TimestampDatum seems it is doing both roles of DateDatum and TimeDatum.
>>
>> Regards,
>> Jongyoung.
>>
>> On Wed Nov 19 2014 at 오후 5:23:40 Jaewoong Jung <jungjw@gmail.com> wrote:
>>
>>> It turns out, TAJO-1191 is slightly more complicated than I thought.
>>> (https://issues.apache.org/jira/browse/TAJO-1191)
>>>
>>> Basically, it's about whether TimeDatum may have a timezone tied with
>>> it. I **believe** TimeDatum is originally designed to hold a time
>>> period value, not an instant. (TimestampDatum seems to be the
>>> canonical container for instants.) So, it doesn't make sense to apply
>>> any timezones to TimeDatum values, but it's being done in a few
>>> places. And, that's why the test is failing on my machines.
>>>
>>> I'm going to try to fix it by removing all timezone-related references
>>> around the class, but I want to check my assumption with you before I
>>> proceed.
>>>
>>> What do you think about it?
>>>

Mime
View raw message