diff --git a/processing/src/main/java/org/apache/druid/query/expression/ExprUtils.java b/processing/src/main/java/org/apache/druid/query/expression/ExprUtils.java index e2bd808d7b9f..be513b402489 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/ExprUtils.java +++ b/processing/src/main/java/org/apache/druid/query/expression/ExprUtils.java @@ -20,6 +20,7 @@ package org.apache.druid.query.expression; import org.apache.druid.common.config.NullHandling; +import org.apache.druid.error.InvalidInput; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.granularity.PeriodGranularity; @@ -45,13 +46,25 @@ static DateTimeZone toTimeZone(final Expr timeZoneArg) } static PeriodGranularity toPeriodGranularity( + final Expr wrappingExpr, final Expr periodArg, @Nullable final Expr originArg, @Nullable final Expr timeZoneArg, final Expr.ObjectBinding bindings ) { - final Period period = new Period(periodArg.eval(bindings).asString()); + final Period period; + try { + period = new Period(periodArg.eval(bindings).asString()); + } + catch (IllegalArgumentException iae) { + throw InvalidInput.exception( + "Invalid period[%s] specified for expression[%s]: [%s]", + periodArg.stringify(), + wrappingExpr.stringify(), + iae.getMessage() + ); + } final DateTime origin; final DateTimeZone timeZone; @@ -69,7 +82,7 @@ static PeriodGranularity toPeriodGranularity( final Object value = originArg.eval(bindings).valueOrDefault(); if (value instanceof String && NullHandling.isNullOrEquivalent((String) value)) { // We get a blank string here, when sql compatible null handling is enabled - // and expression contains empty string for for origin + // and expression contains empty string for origin // e.g timestamp_floor(\"__time\",'PT1M','','UTC') origin = null; } else { diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java index cfd63f1ea615..3c5102ae7a2c 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java @@ -63,7 +63,7 @@ static class TimestampCeilExpr extends ExprMacroTable.BaseScalarMacroFunctionExp TimestampCeilExpr(final TimestampCeilExprMacro macro, final List args) { super(macro, args); - this.granularity = getGranularity(args, InputBindings.nilBindings()); + this.granularity = getGranularity(this, args, InputBindings.nilBindings()); } @Nonnull @@ -113,9 +113,14 @@ public int hashCode() } } - private static PeriodGranularity getGranularity(final List args, final Expr.ObjectBinding bindings) + private static PeriodGranularity getGranularity( + final Expr expr, + final List args, + final Expr.ObjectBinding bindings + ) { return ExprUtils.toPeriodGranularity( + expr, args.get(1), args.size() > 2 ? args.get(2) : null, args.size() > 3 ? args.get(3) : null, @@ -135,7 +140,7 @@ static class TimestampCeilDynamicExpr extends ExprMacroTable.BaseScalarMacroFunc @Override public ExprEval eval(final ObjectBinding bindings) { - final PeriodGranularity granularity = getGranularity(args, bindings); + final PeriodGranularity granularity = getGranularity(this, args, bindings); long argTime = args.get(0).eval(bindings).asLong(); long bucketStartTime = granularity.bucketStart(argTime); if (argTime == bucketStartTime) { diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java index a243273b8f01..02eed7327f17 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java @@ -56,9 +56,14 @@ public Expr apply(final List args) } } - private static PeriodGranularity computeGranularity(final List args, final Expr.ObjectBinding bindings) + private static PeriodGranularity computeGranularity( + final Expr expr, + final List args, + final Expr.ObjectBinding bindings + ) { return ExprUtils.toPeriodGranularity( + expr, args.get(1), args.size() > 2 ? args.get(2) : null, args.size() > 3 ? args.get(3) : null, @@ -73,7 +78,7 @@ public static class TimestampFloorExpr extends ExprMacroTable.BaseScalarMacroFun TimestampFloorExpr(final TimestampFloorExprMacro macro, final List args) { super(macro, args); - this.granularity = computeGranularity(args, InputBindings.nilBindings()); + this.granularity = computeGranularity(this, args, InputBindings.nilBindings()); } /** @@ -170,7 +175,7 @@ public static class TimestampFloorDynamicExpr extends ExprMacroTable.BaseScalarM @Override public ExprEval eval(final ObjectBinding bindings) { - final PeriodGranularity granularity = computeGranularity(args, bindings); + final PeriodGranularity granularity = computeGranularity(this, args, bindings); return ExprEval.of(granularity.bucketStart(args.get(0).eval(bindings).asLong())); } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java index 933baaac9ba8..8eb9541961c6 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java @@ -200,7 +200,7 @@ private void ready() state = CalcitePlanner.State.STATE_2_READY; - // If user specify own traitDef, instead of default default trait, + // If user specifies own traitDef, instead of default trait, // register the trait def specified in traitDefs. if (this.traitDefs == null) { planner.addRelTraitDef(ConventionTraitDef.INSTANCE); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java index 3203ae9915e5..d1f256207d54 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.apache.druid.common.config.NullHandling; +import org.apache.druid.error.DruidExceptionMatcher; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.granularity.Granularities; @@ -130,6 +131,28 @@ public void testExpressionContainingNull() ); } + @Test + public void testTimeCeilExpressionContainingInvalidPeriod() + { + testQueryThrows( + "SELECT TIME_CEIL(__time, 'PT1Y') FROM foo", + DruidExceptionMatcher.invalidInput().expectMessageContains( + "Invalid period['PT1Y'] specified for expression[timestamp_ceil(\"__time\", 'PT1Y', null, 'UTC')]" + ) + ); + } + + @Test + public void testTimeFloorExpressionContainingInvalidPeriod() + { + testQueryThrows( + "SELECT TIME_FLOOR(TIMESTAMPADD(DAY, -1, __time), 'PT1D') FROM foo", + DruidExceptionMatcher.invalidInput().expectMessageContains( + "Invalid period['PT1D'] specified for expression[timestamp_floor((\"__time\" + -86400000), 'PT1D', null, 'UTC')]" + ) + ); + } + @Test public void testValuesContainingNull() {