tajo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Hyunsik Choi <hyunsik.c...@gmail.com>
Subject Re: Timezone issues
Date Thu, 20 Nov 2014 10:44:22 GMT
Those parts have poor documentation.

I agree with your investigation. I also could find many misuse of timezone
in many parts. We should make them clear and fix them in this chance.

I just got off the plane, and I'm still on the road. So, I'll give more
comments tomorrow.

- hyunsik

On Thu, Nov 20, 2014 at 5:24 PM, Jaewoong Jung <jungjw@gmail.com> wrote:

> 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message