clintropolis commented on code in PR #16626:
URL: https://github.com/apache/druid/pull/16626#discussion_r1644929539
##########
processing/src/main/java/org/apache/druid/query/expression/ExprUtils.java:
##########
@@ -45,13 +46,23 @@ static DateTimeZone toTimeZone(final Expr timeZoneArg)
}
static PeriodGranularity toPeriodGranularity(
+ final Expr timeExpressionArg,
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.eval(bindings).asString(), timeExpressionArg,
iae.getMessage()
Review Comment:
`(+ __time -86400000)` is the native expression for `TIMESTAMPADD(DAY, -1,
__time)`, there isn't a `TIMESTAMPADD` function in native layer.
It might be nicer to include `TIME_FLOOR` part, you need to include the
wrapping `Expr` instead of just passing it the first arg,
`computeGranularity(final Expr expr, final List<Expr> args, final
Expr.ObjectBinding bindings)` and then pass that into this method
`computeGranularity(this, args, bindings)`. This will give you an error message
like
```
Invalid period['PT1D'] specified for expression[timestamp_floor(("__time" +
-86400000), 'PT1D', null, 'UTC')]: [Invalid format: "PT1D" is malformed at "1D"]
```
which seems like it would help clear up a bit even if the native expression
doesn't look exactly like the SQL, and help figure out which part of a query if
it had both time_floor and time_ceil, or multiples
Finally, I would recommend using `expr.stringify()` instead of `asString`
for printing period arg (and any other expr)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
