[
https://issues.apache.org/jira/browse/DRILL-7926?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17386495#comment-17386495
]
ASF GitHub Bot commented on DRILL-7926:
---------------------------------------
paul-rogers commented on a change in pull request #2268:
URL: https://github.com/apache/drill/pull/2268#discussion_r675818522
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/DateTypeFunctions.java
##########
@@ -470,43 +500,73 @@ public void eval() {
@Param DateHolder left;
@Param DateHolder right;
@Output IntervalHolder out;
+ @Inject ContextInformation contextInfo;
@Override
public void setup() {
}
@Override
public void eval() {
- long diff = left.value - right.value;
- long days = diff /
org.apache.drill.exec.vector.DateUtilities.daysToStandardMillis;
- out.months = (int) (days /
org.apache.drill.exec.vector.DateUtilities.monthToStandardDays);
- out.days = (int) (days %
org.apache.drill.exec.vector.DateUtilities.monthToStandardDays);
- out.milliseconds = (int) (diff %
org.apache.drill.exec.vector.DateUtilities.daysToStandardMillis);
+ java.time.OffsetDateTime from =
java.time.Instant.ofEpochMilli(right.value).atOffset(java.time.ZoneOffset.UTC);
+ java.time.OffsetDateTime to =
java.time.Instant.ofEpochMilli(left.value).atOffset(java.time.ZoneOffset.UTC);
+ java.time.Duration duration =
java.time.Duration.between(from.toLocalTime(), to.toLocalTime());
+ java.time.Period period;
+
+ if (from.isAfter(to) &&
duration.compareTo(java.time.Duration.ZERO) > 0) {
+ // negative period and positive duration
+ period = java.time.Period.between(from.toLocalDate(),
to.toLocalDate().plusDays(1));
+ duration = duration.minusDays(1);
+ } else if (from.isBefore(to) &&
duration.compareTo(java.time.Duration.ZERO) < 0) {
+ // positive period and negative duration
+ period = java.time.Period.between(from.toLocalDate(),
to.toLocalDate().minusDays(1));
+ duration = duration.plusDays(1);
+ } else {
+ period = java.time.Period.between(from.toLocalDate(),
to.toLocalDate());
+ }
+
+ out.months = (int) period.toTotalMonths();
+ out.days = period.getDays();
+ out.milliseconds = (int) duration.toMillis();
}
}
@FunctionTemplate(name = "age", scope =
FunctionTemplate.FunctionScope.SIMPLE, nulls = NullHandling.NULL_IF_NULL)
public static class AgeDate2Function implements DrillSimpleFunc {
@Param DateHolder right;
- @Workspace long queryStartDate;
+ @Workspace java.time.OffsetDateTime queryStartDate;
@Output IntervalHolder out;
@Inject ContextInformation contextInfo;
@Override
public void setup() {
int timeZoneIndex = contextInfo.getRootFragmentTimeZone();
- org.joda.time.DateTimeZone timeZone =
org.joda.time.DateTimeZone.forID(org.apache.drill.exec.expr.fn.impl.DateUtility.getTimeZone(timeZoneIndex));
- org.joda.time.DateTime now = new
org.joda.time.DateTime(contextInfo.getQueryStartTime(), timeZone);
- queryStartDate = (new org.joda.time.DateMidnight(now.getYear(),
now.getMonthOfYear(), now.getDayOfMonth(), timeZone)).getMillis();
+ java.time.ZoneId zoneId =
java.time.ZoneId.of(org.apache.drill.exec.expr.fn.impl.DateUtility.getTimeZone(timeZoneIndex));
+ java.time.ZonedDateTime dt =
java.time.Instant.ofEpochMilli(contextInfo.getQueryStartTime()).atZone(zoneId);
+ queryStartDate = java.time.OffsetDateTime.of(dt.toLocalDate(),
java.time.LocalTime.MIDNIGHT, java.time.ZoneOffset.UTC);
}
@Override
public void eval() {
- long diff = queryStartDate - right.value;
- long days = diff /
org.apache.drill.exec.vector.DateUtilities.daysToStandardMillis;
- out.months = (int) (days /
org.apache.drill.exec.vector.DateUtilities.monthToStandardDays);
- out.days = (int) (days %
org.apache.drill.exec.vector.DateUtilities.monthToStandardDays);
- out.milliseconds = (int) (diff %
org.apache.drill.exec.vector.DateUtilities.daysToStandardMillis);
+ java.time.OffsetDateTime dt =
java.time.Instant.ofEpochMilli(right.value).atOffset(java.time.ZoneOffset.UTC);
Review comment:
@oleg-zinovev, as @dzamo has clearly suggested, Drill's date/time code
is, to put it kindly, a mess. We mix zone-less DATE and TIME with a
local-time-based TIMESTAMP, then we carelessly cast from local time to UTC.
Every bit of time-related code does its own hacks as bugs are discovered. Would
be great to actually fix this stuff, but all we are usually able to do is
muddle though as best we can for the task at hand.
Code which knows it is using UTC generally converts from local time to UTC.
Code which knows it is working with local time generally does local time
calculations. The problem is the zone-less DATE and TIME. Java Time does
provide classes for those, but historically Drill code simply converted them to
local time.
Further, historically, local time was represented using classes meant for
UTC, further adding to the confusion. Drill timestamps are similar to Unix
timestamps, in that they are milliseconds from 1970-01-01, but they use
1970-01-01'T'00:00:00 *local time* as the "epoch", not the Unix epoch.
(The problem is that, in most modern distributed systems, local time is so
much of a problem that everything, except Drill, runs in UTC. Sigh...)
Here, as @dzamo suggests, we have to make some assumptions:
* All TIMESTAMP fields are in local time. (If a field was in UTC in the
input file, it must have been converted to local time by the corresponding
reader.)
* All DATE fields are converted to midnight local time.
* The query start timestamp is converted to local time.
* AGE is the number of calendar days, local time, between the query start
time and the data column.
Can we instead do the math in UTC? No, we can't. Your birthday, for example,
occurs from midnight to midnight in whatever timezone you happen to be in at
that time. If you were born in London, but now live in San Francisco, your
birthday does not occur in London time and thus from 4PM the day before to 4PM
on your birthday in San Francisco. Said another way, you are a year older on
midnight of your birthday in whatever timezone you happen to be in at that
moment.
Dates are generally difficult; Drill, because of how vague it has been,
simply adds to the difficulty.
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/DateTypeFunctions.java
##########
@@ -470,43 +500,73 @@ public void eval() {
@Param DateHolder left;
@Param DateHolder right;
@Output IntervalHolder out;
+ @Inject ContextInformation contextInfo;
@Override
public void setup() {
}
@Override
public void eval() {
- long diff = left.value - right.value;
- long days = diff /
org.apache.drill.exec.vector.DateUtilities.daysToStandardMillis;
- out.months = (int) (days /
org.apache.drill.exec.vector.DateUtilities.monthToStandardDays);
- out.days = (int) (days %
org.apache.drill.exec.vector.DateUtilities.monthToStandardDays);
- out.milliseconds = (int) (diff %
org.apache.drill.exec.vector.DateUtilities.daysToStandardMillis);
+ java.time.OffsetDateTime from =
java.time.Instant.ofEpochMilli(right.value).atOffset(java.time.ZoneOffset.UTC);
+ java.time.OffsetDateTime to =
java.time.Instant.ofEpochMilli(left.value).atOffset(java.time.ZoneOffset.UTC);
+ java.time.Duration duration =
java.time.Duration.between(from.toLocalTime(), to.toLocalTime());
+ java.time.Period period;
+
+ if (from.isAfter(to) &&
duration.compareTo(java.time.Duration.ZERO) > 0) {
+ // negative period and positive duration
+ period = java.time.Period.between(from.toLocalDate(),
to.toLocalDate().plusDays(1));
+ duration = duration.minusDays(1);
+ } else if (from.isBefore(to) &&
duration.compareTo(java.time.Duration.ZERO) < 0) {
+ // positive period and negative duration
+ period = java.time.Period.between(from.toLocalDate(),
to.toLocalDate().minusDays(1));
+ duration = duration.plusDays(1);
+ } else {
+ period = java.time.Period.between(from.toLocalDate(),
to.toLocalDate());
+ }
+
+ out.months = (int) period.toTotalMonths();
+ out.days = period.getDays();
+ out.milliseconds = (int) duration.toMillis();
}
}
@FunctionTemplate(name = "age", scope =
FunctionTemplate.FunctionScope.SIMPLE, nulls = NullHandling.NULL_IF_NULL)
public static class AgeDate2Function implements DrillSimpleFunc {
@Param DateHolder right;
- @Workspace long queryStartDate;
+ @Workspace java.time.OffsetDateTime queryStartDate;
@Output IntervalHolder out;
@Inject ContextInformation contextInfo;
@Override
public void setup() {
int timeZoneIndex = contextInfo.getRootFragmentTimeZone();
- org.joda.time.DateTimeZone timeZone =
org.joda.time.DateTimeZone.forID(org.apache.drill.exec.expr.fn.impl.DateUtility.getTimeZone(timeZoneIndex));
- org.joda.time.DateTime now = new
org.joda.time.DateTime(contextInfo.getQueryStartTime(), timeZone);
- queryStartDate = (new org.joda.time.DateMidnight(now.getYear(),
now.getMonthOfYear(), now.getDayOfMonth(), timeZone)).getMillis();
+ java.time.ZoneId zoneId =
java.time.ZoneId.of(org.apache.drill.exec.expr.fn.impl.DateUtility.getTimeZone(timeZoneIndex));
+ java.time.ZonedDateTime dt =
java.time.Instant.ofEpochMilli(contextInfo.getQueryStartTime()).atZone(zoneId);
+ queryStartDate = java.time.OffsetDateTime.of(dt.toLocalDate(),
java.time.LocalTime.MIDNIGHT, java.time.ZoneOffset.UTC);
Review comment:
@oleg-zinovev, thanks for the explanation. If
`contextInfo.getQueryStartTime()` is the same for all fragments, then we're
good on the synchronization aspect.
There is another comment later from @dzamo about time zones which seems to
provide a solution to that aspect.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
> The " age " function is not working properly.
> ---------------------------------------------
>
> Key: DRILL-7926
> URL: https://issues.apache.org/jira/browse/DRILL-7926
> Project: Apache Drill
> Issue Type: Bug
> Components: Functions - Drill
> Affects Versions: 1.17.0, 1.18.0
> Reporter: Александр Глухов
> Priority: Blocker
> Attachments: image-2021-05-13-16-17-21-154.png, screenshot-1.png,
> screenshot-2.png
>
>
> The " age " function is not working properly.
> *Playback steps*
> {code:sql}
> select extract(year from m. "years") `years` from (select age('2021-05-13',
> '2007-07-02') `years") m{code}
> *Expected result* 13.
> *Actual result* 14.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
