[
https://issues.apache.org/jira/browse/DRILL-5332?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul Rogers updated DRILL-5332:
-------------------------------
Description:
The following code in {{DateVector}} is worrisome:
{code}
@Override
public DateTime getObject(int index) {
org.joda.time.DateTime date = new org.joda.time.DateTime(get(index),
org.joda.time.DateTimeZone.UTC);
date =
date.withZoneRetainFields(org.joda.time.DateTimeZone.getDefault());
return date;
}
{code}
This code takes a date/time value stored in a value vector, converts it to UTC,
then strips the time zone and replaces it with local time. The result it a
"timestamp" in Java format (seconds since the epoch), but not really, it really
the time since the epoch, as if the epoch had started in the local time zone
rather than UTC.
This is the kind of fun & games that people used to do in Java with the
{{Date}} type before the advent of Joda time (and the migration of Joda into
Java 8.)
It is, in short, very bad practice and nearly impossible to get right.
Further, converting a pure date (since this is a {{DateVector}}) into a
date/time is fraught with peril. A date has no corresponding time. 1 AM on
Friday in one time zone might be 11 PM on Thursday in another. Converting from
dates to times is very difficult.
If the {{DateVector}} corresponds to a date, then it should be simple date with
no implied time zone and no implied relationship to time. If there is to be a
mapping of time, it must be to a {{LocalTime}} (in Joda and Java 8) that has no
implied time zone.
Note that this code directly contradicts the statement in [Drill
documentation|http://drill.apache.org/docs/date-time-and-timestamp/]: "Drill
stores values in Coordinated Universal Time (UTC)." Actually, even the
documentation is questionable: what does it mean to store a date in UTC because
of the above issues?
was:
The following code in {{DateVector}} is worrisome:
{code}
@Override
public DateTime getObject(int index) {
org.joda.time.DateTime date = new org.joda.time.DateTime(get(index),
org.joda.time.DateTimeZone.UTC);
date =
date.withZoneRetainFields(org.joda.time.DateTimeZone.getDefault());
return date;
}
{code}
This code takes a date/time value stored in a value vector, converts it to UTC,
then strips the time zone and replaces it with local time. The result it a
"timestamp" in Java format (seconds since the epoch), but not really, it really
the time since the epoch, as if the epoch had started in the local time zone
rather than UTC.
This is the kind of fun & games that people used to do in Java with the
{{Date}} type before the advent of Joda time (and the migration of Joda into
Java 8.)
It is, in short, very bad practice and nearly impossible to get right.
Further, converting a pure date (since this is a {{DateVector}}) into a
date/time is fraught with peril. A date has no corresponding time. 1 AM on
Friday in one time zone might be 11 PM on Thursday in another. Converting from
dates to times is very difficult.
If the {{DateVector}} corresponds to a date, then it should be simple date with
no implied time zone and no implied relationship to time. If there is to be a
mapping of time, it must be to a {{LocalTime}} (in Joda and Java 8) that has no
implied time zone.
> DateVector support uses questionable conversions to a time
> ----------------------------------------------------------
>
> Key: DRILL-5332
> URL: https://issues.apache.org/jira/browse/DRILL-5332
> Project: Apache Drill
> Issue Type: Bug
> Affects Versions: 1.9.0
> Reporter: Paul Rogers
>
> The following code in {{DateVector}} is worrisome:
> {code}
> @Override
> public DateTime getObject(int index) {
> org.joda.time.DateTime date = new org.joda.time.DateTime(get(index),
> org.joda.time.DateTimeZone.UTC);
> date =
> date.withZoneRetainFields(org.joda.time.DateTimeZone.getDefault());
> return date;
> }
> {code}
> This code takes a date/time value stored in a value vector, converts it to
> UTC, then strips the time zone and replaces it with local time. The result it
> a "timestamp" in Java format (seconds since the epoch), but not really, it
> really the time since the epoch, as if the epoch had started in the local
> time zone rather than UTC.
> This is the kind of fun & games that people used to do in Java with the
> {{Date}} type before the advent of Joda time (and the migration of Joda into
> Java 8.)
> It is, in short, very bad practice and nearly impossible to get right.
> Further, converting a pure date (since this is a {{DateVector}}) into a
> date/time is fraught with peril. A date has no corresponding time. 1 AM on
> Friday in one time zone might be 11 PM on Thursday in another. Converting
> from dates to times is very difficult.
> If the {{DateVector}} corresponds to a date, then it should be simple date
> with no implied time zone and no implied relationship to time. If there is to
> be a mapping of time, it must be to a {{LocalTime}} (in Joda and Java 8) that
> has no implied time zone.
> Note that this code directly contradicts the statement in [Drill
> documentation|http://drill.apache.org/docs/date-time-and-timestamp/]: "Drill
> stores values in Coordinated Universal Time (UTC)." Actually, even the
> documentation is questionable: what does it mean to store a date in UTC
> because of the above issues?
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
