Interval expression check and evaluation issue

[dai-chen]

After undefined type introduced in PR #985, two interval expression issues are found in the following query.

POST _opendistro/_sql
{
  "query": """
   SELECT INTERVAL 60 * 60 * 24 * (NULL - FLOOR(NULL)) SECOND AS `TEMP(Test)(2744314424)(0)` FROM `calcs` HAVING (COUNT(1) > 0)
  """
}

1. Non-aggregate SELECT item check failed:

{
  "error": {
    "reason": "Invalid SQL query",
    "details": "Explicit GROUP BY clause is required because expression [Interval(value=*(*(*(60, 60), 24), -(null, FLOOR(null))), unit=SECOND)] contains non-aggregated column",
    "type": "SemanticCheckException"
  },
  "status": 400
}

2. If we bypass the check above, exception is thrown when interval value evaluation because input is undefined type.

[2021-01-14T14:56:18,909][INFO ][c.a.o.s.l.p.RestSqlAction] [186590df9563.ant.amazon.com] [06ce0988-5b76-4924-8429-8ce2f1c00966] Request is handled by new SQL query engine
[2021-01-14T14:56:18,929][ERROR][c.a.o.s.l.p.RestSQLQueryAction] [186590df9563.ant.amazon.com] Error happened during query handling
com.amazon.opendistroforelasticsearch.sql.exception.ExpressionEvaluationException: invalid to get longValue from value of type UNDEFINED
	at com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue.longValue(ExprValue.java:109) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.getLongValue(ExprValueUtils.java:164) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.expression.datetime.IntervalClause.second(IntervalClause.java:92) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.expression.datetime.IntervalClause.interval(IntervalClause.java:66) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL$3.valueOf(FunctionDSL.java:172) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL$3.valueOf(FunctionDSL.java:171) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL$2.valueOf(FunctionDSL.java:127) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.expression.NamedExpression.valueOf(NamedExpression.java:56) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.planner.physical.ProjectOperator.next(ProjectOperator.java:64) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.planner.physical.ProjectOperator.next(ProjectOperator.java:35) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.elasticsearch.executor.ElasticsearchExecutionEngine.lambda$execute$0(ElasticsearchExecutionEngine.java:51) [elasticsearch-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.elasticsearch.client.ElasticsearchNodeClient.lambda$withCurrentContext$5(ElasticsearchNodeClient.java:188) [elasticsearch-1.12.0.0.jar:?]
	at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:678) [elasticsearch-7.10.0.jar:7.10.0]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130) [?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630) [?:?]
	at java.lang.Thread.run(Thread.java:832) [?:?]

[dai-chen]

The first issue is because we only check literal and aggregate function and missed interval. Probably we can do it in the other way by finding identifier (qualified name) and ignore identifiers in aggregate function:

Sample code that needs test:

  private Optional<UnresolvedExpression> findNonAggregatedItemInSelect() {
    return querySpec.getSelectItems().stream()
                                     //.filter(this::isNonAggregatedExpression)
                                     //.filter(this::isNonLiteralFunction)
                                     .filter(this::isNonAggregateOrLiteralExpression)
                                     .findFirst();
  }

  private boolean isNonAggregateOrLiteralExpression(UnresolvedExpression expr) {
    if (expr instanceof AggregateFunction) {
      return false;
    }

    if (expr instanceof QualifiedName) {
      return true;
    }

    List<? extends Node> children = expr.getChild();
    return children.stream().anyMatch(child -> // false returned if empty children
        isNonAggregateOrLiteralExpression((UnresolvedExpression) child));
  }

[dai-chen]

The second issue is because we don't do null/missing check for interval:

sql/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/datetime/IntervalClause.java

Line 88 in 2aeb80b

return new ExprIntervalValue(Duration.ofNanos(getLongValue(value) * 1000));