Skip to content

Commit

Permalink
apache#1362 Return null for string/datetime and -1 for non-negative-l…
Browse files Browse the repository at this point in the history 
…ong type returning expression functions
  • Loading branch information
@navis
navis committed Feb 1, 2019
1 parent 8f092f0 commit b0ad1a0
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 55 deletions.
71 changes: 38 additions & 33 deletions common/src/main/java/io/druid/math/expr/DateTimeFunctions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package io.druid.math.expr;

import com.google.common.base.Throwables;
import com.metamx.common.IAE;
import com.metamx.common.ISE;
import io.druid.common.DateTimes;
Expand Down Expand Up @@ -170,8 +169,8 @@ public ValueDesc apply(List<Expr> args, TypeResolver bindings)
@Override
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
ExprEval param = args.get(0).eval(bindings);
return ExprEval.of(granularity.bucketStart(param.asDateTime()));
final DateTime dateTime = args.get(0).eval(bindings).asDateTime();
return ExprEval.of(dateTime == null ? null : granularity.bucketStart(dateTime));
}
};
}
Expand All @@ -194,8 +193,8 @@ public ValueDesc apply(List<Expr> args, TypeResolver bindings)
@Override
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
ExprEval param = args.get(0).eval(bindings);
return ExprEval.of(granularity.bucketEnd(param.asDateTime()));
final DateTime dateTime = args.get(0).eval(bindings).asDateTime();
return ExprEval.of(dateTime == null ? null : granularity.bucketEnd(dateTime));
}
};
}
Expand Down Expand Up @@ -228,6 +227,7 @@ public Function create(List<Expr> args)
final DateTimeZone timeZone = args.size() == 3
? JodaUtils.toTimeZone(Evals.getConstantString(args.get(2)))
: null;
final Period period = JodaUtils.toPeriod(Evals.getConstantString(args.get(1)));
return new Child()
{
@Override
Expand All @@ -240,8 +240,7 @@ public ValueDesc apply(List<Expr> args, TypeResolver bindings)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime base = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
Period period = JodaUtils.toPeriod(Evals.getConstantString(args.get(1)));
return evaluate(base, period);
return base == null ? ExprEval.of(base) : evaluate(base, period);
}
};
}
Expand Down Expand Up @@ -340,7 +339,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.dayOfWeek().getAsText(locale));
return ExprEval.of(time == null ? null : time.dayOfWeek().getAsText(locale));
}
};
}
Expand All @@ -357,7 +356,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.getDayOfMonth());
return ExprEval.of(time == null ? -1 :time.getDayOfMonth());
}
};
}
Expand All @@ -374,7 +373,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.dayOfMonth().getMaximumValue());
return ExprEval.of(time == null ? -1 :time.dayOfMonth().getMaximumValue());
}
};
}
Expand All @@ -391,7 +390,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.getDayOfWeek());
return ExprEval.of(time == null ? -1 :time.getDayOfWeek());
}
};
}
Expand All @@ -408,7 +407,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.getDayOfYear());
return ExprEval.of(time == null ? -1 :time.getDayOfYear());
}
};
}
Expand All @@ -426,7 +425,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.getWeekOfWeekyear());
return ExprEval.of(time == null ? -1 :time.getWeekOfWeekyear());
}
};
}
Expand All @@ -444,7 +443,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.getWeekyear());
return ExprEval.of(time == null ? -1 :time.getWeekyear());
}
};
}
Expand All @@ -461,7 +460,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.getHourOfDay());
return ExprEval.of(time == null ? -1 : time.getHourOfDay());
}
};
}
Expand All @@ -478,7 +477,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.getMonthOfYear());
return ExprEval.of(time == null ? -1 :time.getMonthOfYear());
}
};
}
Expand All @@ -495,7 +494,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.monthOfYear().getAsText(locale));
return ExprEval.of(time == null ? null : time.monthOfYear().getAsText(locale));
}
};
}
Expand All @@ -512,7 +511,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.getYear());
return ExprEval.of(time == null ? -1 : time.getYear());
}
};
}
Expand All @@ -529,7 +528,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.dayOfMonth().withMinimumValue());
return ExprEval.of(time == null ? -1 :time.dayOfMonth().withMinimumValue().getDayOfMonth());
}
};
}
Expand All @@ -546,7 +545,7 @@ protected Function evaluate(final DateTimeZone timeZone, final Locale locale)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime time = Evals.toDateTime(args.get(0).eval(bindings), timeZone);
return ExprEval.of(time.dayOfMonth().withMaximumValue());
return ExprEval.of(time == null ? -1 :time.dayOfMonth().withMaximumValue().getDayOfMonth());
}
};
}
Expand Down Expand Up @@ -606,12 +605,8 @@ public ValueDesc apply(List<Expr> args, TypeResolver bindings)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
ExprEval value = args.get(0).eval(bindings);
try {
return eval(Evals.toDateTime(value, formatter));
}
catch (Exception e) {
return failed(e);
}
DateTime dateTime = Evals.toDateTime(value, formatter);
return dateTime == null ? invalid(value) : eval(dateTime);
}
};
}
Expand All @@ -620,9 +615,9 @@ public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)

protected abstract ExprEval eval(DateTime date);

protected ExprEval failed(Exception ex)
protected ExprEval invalid(ExprEval value)
{
throw Throwables.propagate(ex);
throw new IllegalArgumentException("Invalid value " + value.value());
}
}

Expand All @@ -639,7 +634,7 @@ public ValueDesc eval(List<Expr> args, TypeResolver bindings)
@Override
protected ExprEval eval(DateTime date)
{
return ExprEval.of(date.getMillis(), ValueDesc.LONG);
return ExprEval.of(date.getMillis());
}
}

Expand Down Expand Up @@ -682,6 +677,9 @@ protected Function toFunction(Map<String, Object> parameter)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
DateTime dateTime = Evals.toDateTime(args.get(0).eval(bindings), formatter);
if (dateTime == null) {
return ExprEval.of((DateTime) null);
}
return ExprEval.of(timeZone == null ? dateTime : new DateTime(dateTime.getMillis(), timeZone));
}
};
Expand Down Expand Up @@ -714,7 +712,7 @@ protected ExprEval eval(DateTime date)
}

@Override
protected final ExprEval failed(Exception ex)
protected ExprEval invalid(ExprEval value)
{
return ExprEval.of(false);
}
Expand Down Expand Up @@ -750,10 +748,10 @@ protected Function toFunction(Map<String, Object> parameter)
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
ExprEval eval = args.get(0).eval(bindings);
if (eval.isNull()) {
DateTime dateTime = Evals.toDateTime(eval, formatter);
if (dateTime == null) {
return ExprEval.of((String) null);
}
DateTime dateTime = Evals.toDateTime(eval, formatter);
if (prevValue == null || dateTime.getMillis() != prevTime) {
prevTime = dateTime.getMillis();
prevValue = outputFormat.print(dateTime);
Expand Down Expand Up @@ -789,7 +787,14 @@ public Function create(List<Expr> args)
@Override
public ExprEval apply(List<Expr> args, Expr.NumericBinding bindings)
{
final DateTime dateTime = Evals.toDateTime(args.get(1).eval(bindings), timeZone);
final ExprEval param = args.get(1).eval(bindings);
final DateTime dateTime = Evals.toDateTime(param, timeZone);
if (dateTime == null) {
if (unit == Unit.MINUTE || unit == Unit.EPOCH) {
throw new IllegalArgumentException("Invalid value " + param.value());
}
return ExprEval.of(-1);
}

switch (unit) {
case MILLIS:
Expand Down
56 changes: 36 additions & 20 deletions common/src/main/java/io/druid/math/expr/Evals.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,35 +427,51 @@ public Number apply(Comparable input)

static DateTime toDateTime(ExprEval arg, DateTimeFormatter formatter)
{
ValueDesc type = arg.type();
DateTimeZone timeZone = formatter.getZone();
if (type.isString()) {
return formatter.parseDateTime(arg.asString());
if (arg.isNull()) {
return null;
}
if (type.isDateTime()) {
return timeZone == null ? arg.dateTimeValue() : arg.dateTimeValue().withZone(timeZone);
final ValueDesc type = arg.type();
final DateTimeZone timeZone = formatter.getZone();
try {
if (type.isString()) {
return formatter.parseDateTime(arg.asString());
}
if (type.isDateTime()) {
return timeZone == null ? arg.dateTimeValue() : arg.dateTimeValue().withZone(timeZone);
}
return DateTimes.withZone(arg.asLong(), timeZone);
}
catch (Exception e) {
return null;
}
return DateTimes.withZone(arg.asLong(), timeZone);
}

static DateTime toDateTime(ExprEval arg, DateTimeZone timeZone)
{
ValueDesc type = arg.type();
if (type.isString()) {
final String string = arg.stringValue();
if (StringUtils.isNumeric(string)) {
return new DateTime(Long.valueOf(string), timeZone);
} else {
return timeZone == null
? defaultFormat.parseDateTime(string)
: defaultFormat.withZone(timeZone).parseDateTime(string);
}
if (arg.isNull()) {
return null;
}
final ValueDesc type = arg.type();
try {
if (type.isString()) {
final String string = arg.stringValue();
if (StringUtils.isNumeric(string)) {
return new DateTime(Long.valueOf(string), timeZone);
} else {
return timeZone == null
? defaultFormat.parseDateTime(string)
: defaultFormat.withZone(timeZone).parseDateTime(string);
}

}
if (type.isDateTime()) {
return timeZone == null ? arg.dateTimeValue() : arg.dateTimeValue().withZone(timeZone);
}
return DateTimes.withZone(arg.asLong(), timeZone);
}
if (type.isDateTime()) {
return timeZone == null ? arg.dateTimeValue() : arg.dateTimeValue().withZone(timeZone);
catch (Exception e) {
return null;
}
return DateTimes.withZone(arg.asLong(), timeZone);
}

public static Pair<String, Expr> splitSimpleAssign(String expression)
Expand Down
45 changes: 43 additions & 2 deletions common/src/test/java/io/druid/math/expr/EvalTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,31 @@ public void testLongEval()
+ ")", bindings
)
);
// invalid
Assert.assertNull(
evalString(
"time_format("
+ "'2016-11-17 AM 10:11:39.662+0900', "
+ "format='yyyy-MM-dd a hh:mm:ss.SSSZZ', "
+ "locale='ko', "
+ "out.format='MM-dd-yyyy a hh:mm:ss.SSSZ', "
+ "out.locale='us', "
+ "out.timezone='PST'"
+ ")", bindings
)
);
Assert.assertNull(
evalString(
"time_format("
+ "'2016-11-31 오전 10:11:39.662+0900', "
+ "format='yyyy-MM-dd a hh:mm:ss.SSSZZ', "
+ "locale='ko', "
+ "out.format='MM-dd-yyyy a hh:mm:ss.SSSZ', "
+ "out.locale='us', "
+ "out.timezone='PST'"
+ ")", bindings
)
);

long time = new DateTime("2016-11-17T10:11:39.662+0900").getMillis();
// format (long to string)
Expand Down Expand Up @@ -471,7 +496,7 @@ public void testDatetimeFunctions()
Assert.assertEquals("Friday", evalString("dayname(x)", bindings));
Assert.assertEquals("3월", evalString("monthname(x, ,'ko')", bindings));
Assert.assertEquals("금요일", evalString("dayname(x, ,'ko')", bindings));
Assert.assertEquals(new DateTime("2016-03-31T22:25:00", home), evalDateTime("last_day(x)", bindings));
Assert.assertEquals(31, evalLong("last_day(x)", bindings));
Assert.assertEquals(new DateTime("2016-03-08T01:25:00", home), evalDateTime("add_time(x, '3D 3H')", bindings));
Assert.assertEquals(new DateTime("2016-03-03T19:22:00", home), evalDateTime("sub_time(x, '1D 3H 3m')", bindings));

Expand All @@ -486,9 +511,25 @@ public void testDatetimeFunctions()
Assert.assertEquals("Friday", evalString("dayname(x, 'UTC', )", bindings));
Assert.assertEquals("3月", evalString("monthname(x, 'UTC','ja')", bindings));
Assert.assertEquals("金曜日", evalString("dayname(x, 'UTC','ja')", bindings));
Assert.assertEquals(new DateTime("2016-03-31T13:25:00Z"), evalDateTime("last_day(x, 'UTC')", bindings));
Assert.assertEquals(31, evalLong("last_day(x, 'UTC')", bindings));
Assert.assertEquals(new DateTime("2016-03-07T16:25:00Z"), evalDateTime("add_time(x, '3D 3H', 'UTC')", bindings));
Assert.assertEquals(new DateTime("2016-03-03T10:22:00Z"), evalDateTime("sub_time(x, '1D 3H 3m', 'UTC')", bindings));

// invalids
bindings = Parser.withMap(ImmutableMap.of("x", "xxxx"));
Assert.assertEquals(-1, evalLong("dayofmonth(x)", bindings));
Assert.assertEquals(-1, evalLong("lastdayofmonth(x)", bindings));
Assert.assertEquals(-1, evalLong("dayofyear(x)", bindings));
Assert.assertEquals(-1, evalLong("hour(x)", bindings));
Assert.assertEquals(-1, evalLong("month(x)", bindings));
Assert.assertEquals(-1, evalLong("year(x)", bindings));
Assert.assertNull(evalString("monthname(x)", bindings));
Assert.assertNull(evalString("dayname(x)", bindings));
Assert.assertNull(evalString("monthname(x, ,'ko')", bindings));
Assert.assertNull(evalString("dayname(x, ,'ko')", bindings));
Assert.assertEquals(-1, evalLong("last_day(x)", bindings));
Assert.assertNull(evalDateTime("add_time(x, '3D 3H')", bindings));
Assert.assertNull(evalDateTime("sub_time(x, '1D 3H 3m')", bindings));
}

@Test
Expand Down

0 comments on commit b0ad1a0

Please sign in to comment.