Merge develop to master for ODFE 1.10.1.0 release (#733)

Merge develop to master for ODFE 1.10.1.0 release

.github/workflows/draft-release-notes-workflow.yml

@@ -16,6 +16,6 @@ jobs:
         with:
           config-name: draft-release-notes-config.yml
           tag: (None)
-          version: 1.10.0.0
+          version: 1.10.1.0
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/sql-workbench-release-workflow.yml

@@ -27,14 +27,14 @@ jobs:
         uses: actions/checkout@v1
         with:
           repository: opendistro-for-elasticsearch/kibana-oss
-          ref: 7.9.0
+          ref: 7.9.1
           token: ${{secrets.OD_ACCESS}}
           path: sql/kibana
 
       - name: Setup Node
         uses: actions/setup-node@v1
         with:
-          node-version: '10.21.0'
+          node-version: '10.22.0'
 
       - name: Move Workbench to Plugins Dir
         run: |

.github/workflows/sql-workbench-test-and-build-workflow.yml

@@ -15,12 +15,12 @@ jobs:
         uses: actions/checkout@v1
         with:
           repository: elastic/kibana
-          ref: v7.9.0
+          ref: v7.9.1
           path: sql/kibana
       - name: Setup Node
         uses: actions/setup-node@v1
         with:
-          node-version: '10.21.0'
+          node-version: '10.22.0'
       - name: Move Workbench to Plugins Dir
         run: |
           mkdir kibana/plugins

build.gradle

@@ -15,7 +15,7 @@
 
 buildscript {
     ext {
-        es_version = "7.9.0"
+        es_version = "7.9.1"
     }
 
     repositories {
@@ -43,7 +43,7 @@ repositories {
 }
 
 ext {
-    opendistroVersion = '1.10.0'
+    opendistroVersion = '1.10.1'
     isSnapshot = "true" == System.getProperty("build.snapshot", "true")
 }
 

core/build.gradle

@@ -49,9 +49,13 @@ jacocoTestCoverageVerification {
     violationRules {
         rule {
             limit {
+                counter = 'LINE'
+                minimum = 1.0
+            }
+            limit {
+                counter = 'BRANCH'
                 minimum = 1.0
             }
-
         }
     }
     afterEvaluate {

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/Analyzer.java

@@ -15,6 +15,8 @@
 
 package com.amazon.opendistroforelasticsearch.sql.analysis;
 
+import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRUCT;
+
 import com.amazon.opendistroforelasticsearch.sql.analysis.symbol.Namespace;
 import com.amazon.opendistroforelasticsearch.sql.analysis.symbol.Symbol;
 import com.amazon.opendistroforelasticsearch.sql.ast.AbstractNodeVisitor;
@@ -29,6 +31,7 @@
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Eval;
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter;
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project;
+import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN;
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation;
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename;
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort;
@@ -49,6 +52,7 @@
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalFilter;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalProject;
+import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRareTopN;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRelation;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRemove;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRename;
@@ -99,6 +103,11 @@ public LogicalPlan visitRelation(Relation node, AnalysisContext context) {
     TypeEnvironment curEnv = context.peek();
     Table table = storageEngine.getTable(node.getTableName());
     table.getFieldTypes().forEach((k, v) -> curEnv.define(new Symbol(Namespace.FIELD_NAME, k), v));
+
+    // Put index name or its alias in index namespace on type environment so qualifier
+    // can be removed when analyzing qualified name. The value (expr type) here doesn't matter.
+    curEnv.define(new Symbol(Namespace.INDEX_NAME, node.getTableNameOrAlias()), STRUCT);
+
     return new LogicalRelation(node.getTableName());
   }
 
@@ -166,6 +175,39 @@ public LogicalPlan visitAggregation(Aggregation node, AnalysisContext context) {
     return new LogicalAggregation(child, aggregators, groupBys);
   }
 
+  /**
+   * Build {@link LogicalRareTopN}.
+   */
+  @Override
+  public LogicalPlan visitRareTopN(RareTopN node, AnalysisContext context) {
+    final LogicalPlan child = node.getChild().get(0).accept(this, context);
+
+    ImmutableList.Builder<Expression> groupbyBuilder = new ImmutableList.Builder<>();
+    for (UnresolvedExpression expr : node.getGroupExprList()) {
+      groupbyBuilder.add(expressionAnalyzer.analyze(expr, context));
+    }
+    ImmutableList<Expression> groupBys = groupbyBuilder.build();
+
+    ImmutableList.Builder<Expression> fieldsBuilder = new ImmutableList.Builder<>();
+    for (Field f : node.getFields()) {
+      fieldsBuilder.add(expressionAnalyzer.analyze(f, context));
+    }
+    ImmutableList<Expression> fields = fieldsBuilder.build();
+
+    // new context
+    context.push();
+    TypeEnvironment newEnv = context.peek();
+    groupBys.forEach(group -> newEnv.define(new Symbol(Namespace.FIELD_NAME,
+        group.toString()), group.type()));
+    fields.forEach(field -> newEnv.define(new Symbol(Namespace.FIELD_NAME,
+        field.toString()), field.type()));
+
+    List<Argument> options = node.getNoOfResults();
+    Integer noOfResults = (Integer) options.get(0).getValue().getValue();
+
+    return new LogicalRareTopN(child, node.getCommandType(), noOfResults, fields, groupBys);
+  }
+
   /**
    * Build {@link LogicalProject} or {@link LogicalRemove} from {@link Field}.
    *

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzer.java

@@ -19,12 +19,13 @@
 import com.amazon.opendistroforelasticsearch.sql.analysis.symbol.Symbol;
 import com.amazon.opendistroforelasticsearch.sql.ast.AbstractNodeVisitor;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.AggregateFunction;
-import com.amazon.opendistroforelasticsearch.sql.ast.expression.Alias;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.And;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Compare;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.EqualTo;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Function;
+import com.amazon.opendistroforelasticsearch.sql.ast.expression.Interval;
+import com.amazon.opendistroforelasticsearch.sql.ast.expression.IntervalUnit;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Literal;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Not;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Or;
@@ -86,6 +87,13 @@ public Expression visitLiteral(Literal node, AnalysisContext context) {
         .literal(ExprValueUtils.fromObjectValue(node.getValue(), node.getType().getCoreType()));
   }
 
+  @Override
+  public Expression visitInterval(Interval node, AnalysisContext context) {
+    Expression value = node.getValue().accept(this, context);
+    Expression unit = DSL.literal(node.getUnit().name());
+    return dsl.interval(value, unit);
+  }
+
   @Override
   public Expression visitAnd(And node, AnalysisContext context) {
     Expression left = node.getLeft().accept(this, context);
@@ -155,13 +163,8 @@ public Expression visitField(Field node, AnalysisContext context) {
 
   @Override
   public Expression visitQualifiedName(QualifiedName node, AnalysisContext context) {
-    // Name with qualifier (index.field, index_alias.field, object/nested.inner_field
-    //  text.keyword) is not supported for now
-    if (node.getParts().size() > 1) {
-      throw new SyntaxCheckException(String.format(
-          "Qualified name [%s] is not supported yet", node));
-    }
-    return visitIdentifier(node.toString(), context);
+    QualifierAnalyzer qualifierAnalyzer = new QualifierAnalyzer(context);
+    return visitIdentifier(qualifierAnalyzer.unqualified(node), context);
   }
 
   private Expression visitIdentifier(String ident, AnalysisContext context) {

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/QualifierAnalyzer.java

@@ -0,0 +1,72 @@
+/*
+ *    Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License").
+ *    You may not use this file except in compliance with the License.
+ *    A copy of the License is located at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    or in the "license" file accompanying this file. This file is distributed
+ *    on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *    express or implied. See the License for the specific language governing
+ *    permissions and limitations under the License.
+ *
+ */
+
+package com.amazon.opendistroforelasticsearch.sql.analysis;
+
+import com.amazon.opendistroforelasticsearch.sql.analysis.symbol.Namespace;
+import com.amazon.opendistroforelasticsearch.sql.analysis.symbol.Symbol;
+import com.amazon.opendistroforelasticsearch.sql.ast.expression.QualifiedName;
+import com.amazon.opendistroforelasticsearch.sql.common.antlr.SyntaxCheckException;
+import com.amazon.opendistroforelasticsearch.sql.exception.SemanticCheckException;
+import java.util.Arrays;
+import java.util.Optional;
+import lombok.RequiredArgsConstructor;
+
+/**
+ * Analyzer that analyzes qualifier(s) in a full field name.
+ */
+@RequiredArgsConstructor
+public class QualifierAnalyzer {
+
+  private final AnalysisContext context;
+
+  public String unqualified(String... parts) {
+    return unqualified(QualifiedName.of(Arrays.asList(parts)));
+  }
+
+  /**
+   * Get unqualified name if its qualifier symbol found is in index namespace
+   * on type environment. Unqualified name means name with qualifier removed.
+   * For example, unqualified name of "accounts.age" or "acc.age" is "age".
+   *
+   * @return  unqualified name if criteria met above, otherwise original name
+   */
+  public String unqualified(QualifiedName fullName) {
+    return isQualifierIndexOrAlias(fullName) ? fullName.rest().toString() : fullName.toString();
+  }
+
+  private boolean isQualifierIndexOrAlias(QualifiedName fullName) {
+    Optional<String> qualifier = fullName.first();
+    if (qualifier.isPresent()) {
+      resolveQualifierSymbol(fullName, qualifier.get());
+      return true;
+    }
+    return false;
+  }
+
+  private void resolveQualifierSymbol(QualifiedName fullName, String qualifier) {
+    try {
+      context.peek().resolve(new Symbol(Namespace.INDEX_NAME, qualifier));
+    } catch (SemanticCheckException e) {
+      // Throw syntax check intentionally to indicate fall back to old engine.
+      // Need change to semantic check exception in future.
+      throw new SyntaxCheckException(String.format(
+          "The qualifier [%s] of qualified name [%s] must be an index name or its alias",
+              qualifier, fullName));
+    }
+  }
+
+}

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/SelectExpressionAnalyzer.java

@@ -18,13 +18,17 @@
 package com.amazon.opendistroforelasticsearch.sql.analysis;
 
 import com.amazon.opendistroforelasticsearch.sql.analysis.symbol.Namespace;
+import com.amazon.opendistroforelasticsearch.sql.analysis.symbol.Symbol;
 import com.amazon.opendistroforelasticsearch.sql.ast.AbstractNodeVisitor;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Alias;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.AllFields;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field;
+import com.amazon.opendistroforelasticsearch.sql.ast.expression.QualifiedName;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression;
 import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType;
+import com.amazon.opendistroforelasticsearch.sql.exception.SemanticCheckException;
 import com.amazon.opendistroforelasticsearch.sql.expression.DSL;
+import com.amazon.opendistroforelasticsearch.sql.expression.Expression;
 import com.amazon.opendistroforelasticsearch.sql.expression.NamedExpression;
 import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression;
 import com.google.common.collect.ImmutableList;
@@ -63,11 +67,26 @@ public List<NamedExpression> visitField(Field node, AnalysisContext context) {
 
   @Override
   public List<NamedExpression> visitAlias(Alias node, AnalysisContext context) {
-    return Collections.singletonList(DSL.named(node.getName(),
-        node.getDelegated().accept(expressionAnalyzer, context),
+    Expression expr = referenceIfSymbolDefined(node.getDelegated(), context);
+    return Collections.singletonList(DSL.named(
+        unqualifiedNameIfFieldOnly(node, context),
+        expr,
         node.getAlias()));
   }
 
+  private Expression referenceIfSymbolDefined(UnresolvedExpression expr,
+                                              AnalysisContext context) {
+    try {
+      // Since resolved aggregator.toString() is used as symbol name, unresolved expression
+      // needs to be analyzed too to get toString() name for consistency
+      String symbolName = expressionAnalyzer.analyze(expr, context).toString();
+      ExprType type = context.peek().resolve(new Symbol(Namespace.FIELD_NAME, symbolName));
+      return DSL.ref(symbolName, type);
+    } catch (SemanticCheckException e) {
+      return expr.accept(expressionAnalyzer, context);
+    }
+  }
+
   @Override
   public List<NamedExpression> visitAllFields(AllFields node,
                                               AnalysisContext context) {
@@ -76,4 +95,23 @@ public List<NamedExpression> visitAllFields(AllFields node,
     return lookupAllFields.entrySet().stream().map(entry -> DSL.named(entry.getKey(),
         new ReferenceExpression(entry.getKey(), entry.getValue()))).collect(Collectors.toList());
   }
+
+  /**
+   * Get unqualified name if select item is just a field. For example, suppose an index
+   * named "accounts", return "age" for "SELECT accounts.age". But do nothing for expression
+   * in "SELECT ABS(accounts.age)".
+   * Note that an assumption is made implicitly that original name field in Alias must be
+   * the same as the values in QualifiedName. This is true because AST builder does this.
+   * Otherwise, what unqualified() returns will override Alias's name as NamedExpression's name
+   * even though the QualifiedName doesn't have qualifier.
+   */
+  private String unqualifiedNameIfFieldOnly(Alias node, AnalysisContext context) {
+    UnresolvedExpression selectItem = node.getDelegated();
+    if (selectItem instanceof QualifiedName) {
+      QualifierAnalyzer qualifierAnalyzer = new QualifierAnalyzer(context);
+      return qualifierAnalyzer.unqualified((QualifiedName) selectItem);
+    }
+    return node.getName();
+  }
+
 }

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/symbol/Namespace.java

@@ -20,6 +20,7 @@
  */
 public enum Namespace {
 
+  INDEX_NAME("Index"),
   FIELD_NAME("Field"),
   FUNCTION_NAME("Function");
 

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java

@@ -26,6 +26,7 @@
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Function;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.In;
+import com.amazon.opendistroforelasticsearch.sql.ast.expression.Interval;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Let;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Literal;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Map;
@@ -39,6 +40,7 @@
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Eval;
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter;
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project;
+import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN;
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation;
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename;
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort;
@@ -177,6 +179,10 @@ public T visitDedupe(Dedupe node, C context) {
     return visitChildren(node, context);
   }
 
+  public T visitRareTopN(RareTopN node, C context) {
+    return visitChildren(node, context);
+  }
+
   public T visitValues(Values node, C context) {
     return visitChildren(node, context);
   }
@@ -188,4 +194,8 @@ public T visitAlias(Alias node, C context) {
   public T visitAllFields(AllFields node, C context) {
     return visitChildren(node, context);
   }
+
+  public T visitInterval(Interval node, C context) {
+    return visitChildren(node, context);
+  }
 }