You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2018/12/02 00:54:53 UTC

[01/15] calcite git commit: [CALCITE-2714] Make BasicSqlType immutable, and now SqlTypeFactory.createWithNullability can reuse existing type if possible (Ruben Quesada Lopez)

Repository: calcite
Updated Branches:
  refs/heads/master 439ca73b8 -> 18caf38f8


[CALCITE-2714] Make BasicSqlType immutable, and now SqlTypeFactory.createWithNullability can reuse existing type if possible (Ruben Quesada Lopez)

Close apache/calcite#947


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/08aefb09
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/08aefb09
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/08aefb09

Branch: refs/heads/master
Commit: 08aefb09c596c3a3292b68b695b1038fd4f64b92
Parents: 439ca73
Author: rubenada <ru...@gmail.com>
Authored: Thu Nov 29 11:08:03 2018 +0100
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Nov 30 17:53:31 2018 -0800

----------------------------------------------------------------------
 .../calcite/sql/type/AbstractSqlType.java       |  3 +-
 .../apache/calcite/sql/type/BasicSqlType.java   | 94 ++++++++++----------
 .../calcite/sql/type/SqlTypeFactoryImpl.java    |  5 +-
 3 files changed, 53 insertions(+), 49 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/08aefb09/core/src/main/java/org/apache/calcite/sql/type/AbstractSqlType.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/type/AbstractSqlType.java b/core/src/main/java/org/apache/calcite/sql/type/AbstractSqlType.java
index 4baa04d..2e316f5 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/AbstractSqlType.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/AbstractSqlType.java
@@ -24,6 +24,7 @@ import org.apache.calcite.rel.type.RelDataTypePrecedenceList;
 
 import java.io.Serializable;
 import java.util.List;
+import java.util.Objects;
 
 /**
  * Abstract base class for SQL implementations of {@link RelDataType}.
@@ -50,7 +51,7 @@ public abstract class AbstractSqlType
       boolean isNullable,
       List<? extends RelDataTypeField> fields) {
     super(fields);
-    this.typeName = typeName;
+    this.typeName = Objects.requireNonNull(typeName);
     this.isNullable = isNullable || (typeName == SqlTypeName.NULL);
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/08aefb09/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java b/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java
index 1c0ac32..72e4777 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java
@@ -23,10 +23,13 @@ import org.apache.calcite.util.SerializableCharset;
 import com.google.common.base.Preconditions;
 
 import java.nio.charset.Charset;
+import java.util.Objects;
 
 /**
  * BasicSqlType represents a standard atomic SQL type (excluding interval
  * types).
+ *
+ * <p>Instances of this class are immutable.
  */
 public class BasicSqlType extends AbstractSqlType {
   //~ Static fields/initializers ---------------------------------------------
@@ -36,8 +39,8 @@ public class BasicSqlType extends AbstractSqlType {
   private final int precision;
   private final int scale;
   private final RelDataTypeSystem typeSystem;
-  private SqlCollation collation;
-  private SerializableCharset wrappedCharset;
+  private final SqlCollation collation;
+  private final SerializableCharset wrappedCharset;
 
   //~ Constructors -----------------------------------------------------------
 
@@ -45,39 +48,51 @@ public class BasicSqlType extends AbstractSqlType {
    * Constructs a type with no parameters. This should only be called from a
    * factory method.
    *
+   * @param typeSystem Type system
    * @param typeName Type name
    */
   public BasicSqlType(RelDataTypeSystem typeSystem, SqlTypeName typeName) {
     this(typeSystem, typeName, false, PRECISION_NOT_SPECIFIED,
-        SCALE_NOT_SPECIFIED);
-    assert typeName.allowsPrecScale(false, false)
-        : "typeName.allowsPrecScale(false,false), typeName=" + typeName.name();
-    computeDigest();
+        SCALE_NOT_SPECIFIED, null, null);
+    checkPrecScale(typeName, false, false);
+  }
+
+  /** Throws if {@code typeName} does not allow the given combination of
+   * precision and scale. */
+  protected static void checkPrecScale(SqlTypeName typeName,
+      boolean precisionSpecified, boolean scaleSpecified) {
+    if (!typeName.allowsPrecScale(precisionSpecified, scaleSpecified)) {
+      throw new AssertionError("typeName.allowsPrecScale("
+          + precisionSpecified + ", " + scaleSpecified + "): " + typeName);
+    }
   }
 
   /**
    * Constructs a type with precision/length but no scale.
    *
+   * @param typeSystem Type system
    * @param typeName Type name
+   * @param precision Precision (called length for some types)
    */
   public BasicSqlType(RelDataTypeSystem typeSystem, SqlTypeName typeName,
       int precision) {
-    this(typeSystem, typeName, false, precision, SCALE_NOT_SPECIFIED);
-    assert typeName.allowsPrecScale(true, false)
-        : "typeName.allowsPrecScale(true, false)";
-    computeDigest();
+    this(typeSystem, typeName, false, precision, SCALE_NOT_SPECIFIED, null,
+        null);
+    checkPrecScale(typeName, true, false);
   }
 
   /**
    * Constructs a type with precision/length and scale.
    *
+   * @param typeSystem Type system
    * @param typeName Type name
+   * @param precision Precision (called length for some types)
+   * @param scale Scale
    */
   public BasicSqlType(RelDataTypeSystem typeSystem, SqlTypeName typeName,
       int precision, int scale) {
-    this(typeSystem, typeName, false, precision, scale);
-    assert typeName.allowsPrecScale(true, true);
-    computeDigest();
+    this(typeSystem, typeName, false, precision, scale, null, null);
+    checkPrecScale(typeName, true, true);
   }
 
   /** Internal constructor. */
@@ -86,61 +101,52 @@ public class BasicSqlType extends AbstractSqlType {
       SqlTypeName typeName,
       boolean nullable,
       int precision,
-      int scale) {
+      int scale,
+      SqlCollation collation,
+      SerializableCharset wrappedCharset) {
     super(typeName, nullable, null);
-    this.typeSystem = typeSystem;
+    this.typeSystem = Objects.requireNonNull(typeSystem);
     this.precision = precision;
     this.scale = scale;
+    this.collation = collation;
+    this.wrappedCharset = wrappedCharset;
+    computeDigest();
   }
 
   //~ Methods ----------------------------------------------------------------
 
   /**
-   * Constructs a type with nullablity
+   * Constructs a type with nullablity.
    */
   BasicSqlType createWithNullability(boolean nullable) {
-    BasicSqlType ret;
-    try {
-      ret = (BasicSqlType) this.clone();
-    } catch (CloneNotSupportedException e) {
-      throw new AssertionError(e);
+    if (nullable == this.isNullable) {
+      return this;
     }
-    ret.isNullable = nullable;
-    ret.computeDigest();
-    return ret;
+    return new BasicSqlType(this.typeSystem, this.typeName, nullable,
+        this.precision, this.scale, this.collation, this.wrappedCharset);
   }
 
   /**
    * Constructs a type with charset and collation.
    *
-   * <p>This must be a character tyoe.
+   * <p>This must be a character type.
    */
-  BasicSqlType createWithCharsetAndCollation(
-      Charset charset,
+  BasicSqlType createWithCharsetAndCollation(Charset charset,
       SqlCollation collation) {
     Preconditions.checkArgument(SqlTypeUtil.inCharFamily(this));
-    BasicSqlType ret;
-    try {
-      ret = (BasicSqlType) this.clone();
-    } catch (CloneNotSupportedException e) {
-      throw new AssertionError(e);
-    }
-    ret.wrappedCharset = SerializableCharset.forCharset(charset);
-    ret.collation = collation;
-    ret.computeDigest();
-    return ret;
+    return new BasicSqlType(this.typeSystem, this.typeName, this.isNullable,
+        this.precision, this.scale, collation,
+        SerializableCharset.forCharset(charset));
   }
 
-  // implement RelDataType
-  public int getPrecision() {
+  @Override public int getPrecision() {
     if (precision == PRECISION_NOT_SPECIFIED) {
       return typeSystem.getDefaultPrecision(typeName);
     }
     return precision;
   }
 
-  // implement RelDataType
-  public int getScale() {
+  @Override public int getScale() {
     if (scale == SCALE_NOT_SPECIFIED) {
       switch (typeName) {
       case TINYINT:
@@ -156,13 +162,11 @@ public class BasicSqlType extends AbstractSqlType {
     return scale;
   }
 
-  // implement RelDataType
-  public Charset getCharset() {
+  @Override public Charset getCharset() {
     return wrappedCharset == null ? null : wrappedCharset.getCharset();
   }
 
-  // implement RelDataType
-  public SqlCollation getCollation() {
+  @Override public SqlCollation getCollation() {
     return collation;
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/08aefb09/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
index 91556ca..b1ee8dc 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
@@ -192,10 +192,9 @@ public class SqlTypeFactoryImpl extends RelDataTypeFactoryImpl {
   @Override public RelDataType createTypeWithNullability(
       final RelDataType type,
       final boolean nullable) {
-    RelDataType newType;
+    final RelDataType newType;
     if (type instanceof BasicSqlType) {
-      BasicSqlType sqlType = (BasicSqlType) type;
-      newType = sqlType.createWithNullability(nullable);
+      newType = ((BasicSqlType) type).createWithNullability(nullable);
     } else if (type instanceof MapSqlType) {
       newType = copyMapType(type, nullable);
     } else if (type instanceof ArraySqlType) {


[04/15] calcite git commit: [CALCITE-2632] Ensure that RexNode and its sub-classes implement hashCode and equals methods (Zoltan Haindrich)

Posted by jh...@apache.org.
http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
index 4f5b8d9..c218e7c 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -1846,29 +1846,22 @@ from (select 2+deptno d2, 3+deptno d3 from emp) e
         <Resource name="plan">
             <![CDATA[
 LogicalProject(D2=[$0], D3=[$1])
-  LogicalProject(D2=[$0], D3=[$1], D1=[CAST($2):INTEGER], D30=[$3], $f2=[CAST($4):BOOLEAN])
+  LogicalProject(D2=[$0], D3=[$1], D1=[CAST($2):INTEGER], D6=[$3], $f2=[CAST($4):BOOLEAN])
     LogicalJoin(condition=[AND(=($0, $2), =($1, $3))], joinType=[inner])
       LogicalProject(D2=[+(2, $7)], D3=[+(3, $7)])
         LogicalTableScan(table=[[CATALOG, SALES, EMP]])
       LogicalAggregate(group=[{0, 1}], agg#0=[MIN($2)])
-        LogicalProject(D1=[$0], D3=[$2], $f0=[true])
+        LogicalProject(D1=[$0], D6=[$2], $f0=[true])
           LogicalFilter(condition=[IS NOT NULL($1)])
-            LogicalProject(D1=[$0], $f0=[$3], D3=[$2])
+            LogicalProject(D1=[$0], $f0=[$3], D6=[$2])
               LogicalJoin(condition=[=($0, $1)], joinType=[left])
                 LogicalProject(D1=[+($0, 1)])
                   LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
                 LogicalAggregate(group=[{0, 1}], agg#0=[MIN($2)])
-                  LogicalProject(D1=[$3], D3=[$4], $f0=[true])
-                    LogicalJoin(condition=[AND(=($0, $3), =($1, $3), =($2, $4))], joinType=[inner])
+                  LogicalProject(D4=[$0], D6=[$2], $f0=[true])
+                    LogicalFilter(condition=[=($1, $0)])
                       LogicalProject(D4=[+($0, 4)], D5=[+($0, 5)], D6=[+($0, 6)])
                         LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
-                      LogicalJoin(condition=[true], joinType=[inner])
-                        LogicalAggregate(group=[{0}])
-                          LogicalProject(D1=[+($0, 1)])
-                            LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
-                        LogicalAggregate(group=[{0}])
-                          LogicalProject(D3=[+(3, $7)])
-                            LogicalTableScan(table=[[CATALOG, SALES, EMP]])
 ]]>
         </Resource>
     </TestCase>
@@ -4778,29 +4771,22 @@ from (select 2+deptno d2, 3+deptno d3 from emp) e
         <Resource name="plan">
             <![CDATA[
 LogicalProject(D2=[$0], D3=[$1])
-  LogicalProject(D2=[$0], D3=[$1], D1=[CAST($2):INTEGER], D30=[$3], $f2=[CAST($4):BOOLEAN])
+  LogicalProject(D2=[$0], D3=[$1], D1=[CAST($2):INTEGER], D6=[$3], $f2=[CAST($4):BOOLEAN])
     LogicalJoin(condition=[AND(=($0, $2), =($1, $3))], joinType=[inner])
       LogicalProject(D2=[+(2, $7)], D3=[+(3, $7)])
         LogicalTableScan(table=[[CATALOG, SALES, EMP]])
       LogicalAggregate(group=[{0, 1}], agg#0=[MIN($2)])
-        LogicalProject(D1=[$0], D3=[$2], $f0=[true])
+        LogicalProject(D1=[$0], D6=[$2], $f0=[true])
           LogicalFilter(condition=[IS NOT NULL($1)])
-            LogicalProject(D1=[$0], $f0=[$3], D3=[$2])
+            LogicalProject(D1=[$0], $f0=[$3], D6=[$2])
               LogicalJoin(condition=[=($0, $1)], joinType=[left])
                 LogicalProject(D1=[+($0, 1)])
                   LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
                 LogicalAggregate(group=[{0, 1}], agg#0=[MIN($2)])
-                  LogicalProject(D1=[$3], D3=[$4], $f0=[true])
-                    LogicalJoin(condition=[AND(=($0, $3), =($1, $3), =($2, $4))], joinType=[inner])
+                  LogicalProject(D4=[$0], D6=[$2], $f0=[true])
+                    LogicalFilter(condition=[=($1, $0)])
                       LogicalProject(D4=[+($0, 4)], D5=[+($0, 5)], D6=[+($0, 6)])
                         LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
-                      LogicalJoin(condition=[true], joinType=[inner])
-                        LogicalAggregate(group=[{0}])
-                          LogicalProject(D1=[+($0, 1)])
-                            LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
-                        LogicalAggregate(group=[{0}])
-                          LogicalProject(D3=[+(3, $7)])
-                            LogicalTableScan(table=[[CATALOG, SALES, EMP]])
 ]]>
         </Resource>
     </TestCase>

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java
index b442ddd..311f9bc 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java
@@ -149,8 +149,8 @@ class ElasticsearchRules {
           return stripQuotes(strings.get(0)) + "[" + ((RexLiteral) op1).getValue2() + "]";
         }
       }
-      throw new IllegalArgumentException("Translation of " + call.toString()
-        + " is not supported by ElasticsearchProject");
+      throw new IllegalArgumentException("Translation of " + call
+          + " is not supported by ElasticsearchProject");
     }
 
     List<String> visitList(List<RexNode> list) {


[02/15] calcite git commit: [CALCITE-2689] In ElasticSearch adapter, allow grouping on non-textual fields like date and number

Posted by jh...@apache.org.
[CALCITE-2689] In ElasticSearch adapter, allow grouping on non-textual fields like date and number

Consider field type when populating `missing` (value) in Elasticsearch
terms aggregations.

Close apache/calcite#946


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/ed3da62d
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/ed3da62d
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/ed3da62d

Branch: refs/heads/master
Commit: ed3da62d75ead6c00bb73470c3436e51f6f77197
Parents: 08aefb0
Author: Andrei Sereda <25...@users.noreply.github.com>
Authored: Tue Nov 20 00:10:47 2018 -0500
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Nov 30 17:55:09 2018 -0800

----------------------------------------------------------------------
 elasticsearch/pom.xml                           |   5 +
 .../elasticsearch/ElasticsearchJson.java        |  67 +++++--
 .../elasticsearch/ElasticsearchMapping.java     | 188 +++++++++++++++++++
 .../elasticsearch/ElasticsearchTable.java       |  13 +-
 .../elasticsearch/ElasticsearchTransport.java   |  17 ++
 .../adapter/elasticsearch/Scrolling.java        |   1 -
 .../adapter/elasticsearch/AggregationTest.java  |  77 ++++++--
 .../adapter/elasticsearch/BooleanLogicTest.java |   7 +-
 .../adapter/elasticsearch/ScrollingTest.java    |   1 -
 9 files changed, 338 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/ed3da62d/elasticsearch/pom.xml
----------------------------------------------------------------------
diff --git a/elasticsearch/pom.xml b/elasticsearch/pom.xml
index ec9a0e0..86248ba 100644
--- a/elasticsearch/pom.xml
+++ b/elasticsearch/pom.xml
@@ -68,6 +68,11 @@ limitations under the License.
       <artifactId>jackson-annotations</artifactId>
     </dependency>
     <dependency>
+      <groupId>com.google.code.findbugs</groupId>
+      <artifactId>jsr305</artifactId>
+      <optional>true</optional>
+    </dependency>
+    <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
       <scope>test</scope>

http://git-wip-us.apache.org/repos/asf/calcite/blob/ed3da62d/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java
index 8a6b011..e389ecf 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java
@@ -26,14 +26,15 @@ import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
 import com.fasterxml.jackson.databind.node.ArrayNode;
-import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 
 import java.io.IOException;
 import java.time.Duration;
+import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Deque;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
@@ -50,7 +51,7 @@ import java.util.stream.StreamSupport;
 import static java.util.Collections.unmodifiableMap;
 
 /**
- * Internal objects (and deserializers) used to parse elastic search results
+ * Internal objects (and deserializers) used to parse Elasticsearch results
  * (which are in JSON format).
  *
  * <p>Since we're using basic row-level rest client http response has to be
@@ -58,13 +59,6 @@ import static java.util.Collections.unmodifiableMap;
  */
 final class ElasticsearchJson {
 
-  /**
-   * Used as special aggregation key for missing values (documents which are missing a field).
-   * Buckets with that value are then converted to {@code null}s in flat tabular format.
-   * @see <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-metrics-sum-aggregation.html">Missing Value</a>
-   */
-  static final JsonNode MISSING_VALUE = JsonNodeFactory.instance.textNode("__MISSING__");
-
   private ElasticsearchJson() {}
 
   /**
@@ -87,6 +81,50 @@ final class ElasticsearchJson {
   }
 
   /**
+   * Visits Elasticsearch
+   * <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping.html">mapping
+   * properties</a> and calls consumer for each {@code field / type} pair.
+   * Nested fields are represented as {@code foo.bar.qux}.
+   */
+  static void visitMappingProperties(ObjectNode mapping,
+      BiConsumer<String, String> consumer) {
+    Objects.requireNonNull(mapping, "mapping");
+    Objects.requireNonNull(consumer, "consumer");
+    visitMappingProperties(new ArrayDeque<>(), mapping, consumer);
+  }
+
+  private static void visitMappingProperties(Deque<String> path,
+      ObjectNode mapping, BiConsumer<String, String> consumer) {
+    Objects.requireNonNull(mapping, "mapping");
+    if (mapping.isMissingNode()) {
+      return;
+    }
+
+    if (mapping.has("properties")) {
+      // recurse
+      visitMappingProperties(path, (ObjectNode) mapping.get("properties"), consumer);
+      return;
+    }
+
+    if (mapping.has("type")) {
+      // this is leaf (register field / type mapping)
+      consumer.accept(String.join(".", path), mapping.get("type").asText());
+      return;
+    }
+
+    // otherwise continue visiting mapping(s)
+    Iterable<Map.Entry<String, JsonNode>> iter = mapping::fields;
+    for (Map.Entry<String, JsonNode> entry : iter) {
+      final String name = entry.getKey();
+      final ObjectNode node = (ObjectNode) entry.getValue();
+      path.add(name);
+      visitMappingProperties(path, node, consumer);
+      path.removeLast();
+    }
+  }
+
+
+  /**
    * Identifies a calcite row (as in relational algebra)
    */
   private static class RowKey {
@@ -601,19 +639,24 @@ final class ElasticsearchJson {
      * Determines if current key is a missing field key. Missing key is returned when document
      * does not have pivoting attribute (example {@code GROUP BY _MAP['a.b.missing']}). It helps
      * grouping documents which don't have a field. In relational algebra this
-     * would be {@code null}.
+     * would normally be {@code null}.
+     *
+     * <p>Please note that missing value is different for each type.
      *
      * @param key current {@code key} (usually string) as returned by ES
      * @return {@code true} if this value
-     * @see #MISSING_VALUE
      */
     private static boolean isMissingBucket(JsonNode key) {
-      return MISSING_VALUE.equals(key);
+      return ElasticsearchMapping.Datatype.isMissingValue(key);
     }
 
     private static Bucket parseBucket(JsonParser parser, String name, ObjectNode node)
         throws JsonProcessingException  {
 
+      if (!node.has("key")) {
+        throw new IllegalArgumentException("No 'key' attribute for " + node);
+      }
+
       final JsonNode keyNode = node.get("key");
       final Object key;
       if (isMissingBucket(keyNode) || keyNode.isNull()) {

http://git-wip-us.apache.org/repos/asf/calcite/blob/ed3da62d/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchMapping.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchMapping.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchMapping.java
new file mode 100644
index 0000000..93a8049
--- /dev/null
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchMapping.java
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License 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 org.apache.calcite.adapter.elasticsearch;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.JsonNodeFactory;
+import com.google.common.collect.ImmutableMap;
+
+import java.time.LocalDate;
+import java.time.ZoneOffset;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import javax.annotation.Nullable;
+
+/**
+ * Stores Elasticsearch
+ * <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping.html">
+ * mapping</a> information for particular index/type. This information is
+ * extracted from {@code /$index/$type/_mapping} endpoint.
+ *
+ * <p>Instances of this class are immutable.
+ */
+class ElasticsearchMapping {
+
+  private final String index;
+
+  private final String type;
+
+  private final Map<String, Datatype> mapping;
+
+  ElasticsearchMapping(final String index, final String type,
+      final Map<String, String> mapping) {
+    this.index = Objects.requireNonNull(index, "index");
+    this.type = Objects.requireNonNull(type, "type");
+    Objects.requireNonNull(mapping, "mapping");
+
+    final Map<String, Datatype> transformed = mapping.entrySet().stream()
+        .collect(Collectors.toMap(Map.Entry::getKey, e -> new Datatype(e.getValue())));
+    this.mapping = ImmutableMap.copyOf(transformed);
+  }
+
+  /**
+   * Returns ES schema for each field. Mapping is represented as field name
+   * {@code foo.bar.qux} and type ({@code keyword}, {@code boolean},
+   * {@code long}).
+   *
+   * @return immutable mapping between field and ES type
+   *
+   * @see <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-types.html">Mapping Types</a>
+   */
+  Map<String, Datatype> mapping() {
+    return this.mapping;
+  }
+
+  /**
+   * Used as special aggregation key for missing values (documents that are
+   * missing a field).
+   *
+   * <p>Buckets with that value are then converted to {@code null}s in flat
+   * tabular format.
+   *
+   * @see <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-metrics-sum-aggregation.html">Missing Value</a>
+   */
+  Optional<JsonNode> missingValueFor(String fieldName) {
+    if (!mapping().containsKey(fieldName)) {
+      final String message = String.format(Locale.ROOT,
+          "Field %s not defined for %s/%s", fieldName, index, type);
+      throw new IllegalArgumentException(message);
+    }
+
+    return mapping().get(fieldName).missingValue();
+  }
+
+  String index() {
+    return this.index;
+  }
+
+  String type() {
+    return this.type;
+  }
+
+  /**
+   * Represents elastic data-type, like {@code long}, {@code keyword},
+   * {@code date} etc.
+   *
+   * @see <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-types.html">Mapping Types</a>
+   */
+  static class Datatype {
+    private static final JsonNodeFactory FACTORY = JsonNodeFactory.instance;
+
+    // pre-cache missing values
+    private static final Set<JsonNode> MISSING_VALUES =
+        Stream.of("string", // for ES2
+            "text", "keyword",
+            "date", "long", "integer", "double", "float")
+            .map(Datatype::missingValueForType)
+            .collect(Collectors.toSet());
+
+    private final String name;
+    private final JsonNode missingValue;
+
+    private Datatype(final String name) {
+      this.name = Objects.requireNonNull(name, "name");
+      this.missingValue = missingValueForType(name);
+    }
+
+    /**
+     * Mapping between ES type and json value that represents
+     * {@code missing value} during aggregations. This value can't be
+     * {@code null} and should match type or the field (for ES long type it
+     * also has to be json integer, for date it has to match date format or be
+     * integer (millis epoch) etc.
+     *
+     * <p>It is used for terms aggregations to represent SQL {@code null}.
+     *
+     * @param name name of the type ({@code long}, {@code keyword} ...)
+     *
+     * @return json that will be used in elastic search terms aggregation for
+     * missing value
+     *
+     * @see <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-terms-aggregation.html#_missing_value_13">Missing Value</a>
+     */
+    private static @Nullable JsonNode missingValueForType(String name) {
+      switch (name) {
+      case "string": // for ES2
+      case "text":
+      case "keyword":
+        return FACTORY.textNode("__MISSING__");
+      case "long":
+        return FACTORY.numberNode(Long.MIN_VALUE);
+      case "integer":
+        return FACTORY.numberNode(Integer.MIN_VALUE);
+      case "short":
+        return FACTORY.numberNode(Short.MIN_VALUE);
+      case "double":
+        return FACTORY.numberNode(Double.MIN_VALUE);
+      case "float":
+        return FACTORY.numberNode(Float.MIN_VALUE);
+      case "date":
+        // sentinel for missing dates: 9999-12-31
+        final long millisEpoch = LocalDate.of(9999, 12, 31)
+            .atStartOfDay().toInstant(ZoneOffset.UTC).toEpochMilli();
+        // by default elastic returns dates as longs
+        return FACTORY.numberNode(millisEpoch);
+      }
+
+      // this is unknown type
+      return null;
+    }
+
+    /**
+     * Name of the type: {@code text}, {@code integer}, {@code float} etc.
+     */
+    String name() {
+      return this.name;
+    }
+
+    Optional<JsonNode> missingValue() {
+      return Optional.ofNullable(missingValue);
+    }
+
+    static boolean isMissingValue(JsonNode node) {
+      return MISSING_VALUES.contains(node);
+    }
+  }
+
+}
+
+// End ElasticsearchMapping.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/ed3da62d/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTable.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTable.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTable.java
index f9565ec..b009fff 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTable.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTable.java
@@ -219,15 +219,20 @@ public class ElasticsearchTable extends AbstractQueryableTable implements Transl
       final ObjectNode section = parent.with(aggName);
       final ObjectNode terms = section.with("terms");
       terms.put("field", name);
-      terms.set("missing", ElasticsearchJson.MISSING_VALUE); // expose missing terms
+
+      transport.mapping.missingValueFor(name).ifPresent(m -> {
+        // expose missing terms. each type has a different missing value
+        terms.set("missing", m);
+      });
 
       if (fetch != null) {
         terms.put("size", fetch);
       }
 
-      sort.stream().filter(e -> e.getKey().equals(name)).findAny().ifPresent(s -> {
-        terms.with("order").put("_key", s.getValue().isDescending() ? "desc" : "asc");
-      });
+      sort.stream().filter(e -> e.getKey().equals(name)).findAny()
+          .ifPresent(s ->
+              terms.with("order")
+                  .put("_key", s.getValue().isDescending() ? "desc" : "asc"));
 
       parent = section.with(AGGREGATIONS);
     }

http://git-wip-us.apache.org/repos/asf/calcite/blob/ed3da62d/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTransport.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTransport.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTransport.java
index 12173d8..0c9c06a 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTransport.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTransport.java
@@ -36,6 +36,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.fasterxml.jackson.databind.node.TextNode;
+import com.google.common.collect.ImmutableMap;
 
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.RestClient;
@@ -72,6 +73,8 @@ final class ElasticsearchTransport {
 
   final ElasticsearchVersion version;
 
+  final ElasticsearchMapping mapping;
+
   /**
    * Default batch size
    * @see <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-scroll.html">Scrolling API</a>
@@ -89,6 +92,7 @@ final class ElasticsearchTransport {
     this.typeName = Objects.requireNonNull(typeName, "typeName");
     this.fetchSize = fetchSize;
     this.version = version(); // cache version
+    this.mapping = fetchAndCreateMapping(); // cache mapping
   }
 
   RestClient restClient() {
@@ -112,6 +116,19 @@ final class ElasticsearchTransport {
         .apply(request);
   }
 
+  /**
+   * Build index mapping returning new instance of {@link ElasticsearchMapping}.
+   */
+  private ElasticsearchMapping fetchAndCreateMapping() {
+    final String uri = String.format(Locale.ROOT, "/%s/%s/_mapping", indexName, typeName);
+    final ObjectNode root = rawHttp(ObjectNode.class).apply(new HttpGet(uri));
+    ObjectNode properties = (ObjectNode) root.elements().next().get("mappings").elements().next();
+
+    ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
+    ElasticsearchJson.visitMappingProperties(properties, builder::put);
+    return new ElasticsearchMapping(indexName, typeName, builder.build());
+  }
+
   ObjectMapper mapper() {
     return mapper;
   }

http://git-wip-us.apache.org/repos/asf/calcite/blob/ed3da62d/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/Scrolling.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/Scrolling.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/Scrolling.java
index f947ae5..85850d8 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/Scrolling.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/Scrolling.java
@@ -14,7 +14,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.apache.calcite.adapter.elasticsearch;
 
 import com.fasterxml.jackson.databind.node.ObjectNode;

http://git-wip-us.apache.org/repos/asf/calcite/blob/ed3da62d/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/AggregationTest.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/AggregationTest.java b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/AggregationTest.java
index d06cc7d..0586e88 100644
--- a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/AggregationTest.java
+++ b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/AggregationTest.java
@@ -23,6 +23,8 @@ import org.apache.calcite.schema.impl.ViewTableMacro;
 import org.apache.calcite.test.CalciteAssert;
 import org.apache.calcite.test.ElasticsearchChecker;
 
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.collect.ImmutableMap;
 
@@ -41,7 +43,7 @@ import java.util.Locale;
 import java.util.Map;
 
 /**
- * Testing Elastic Search aggregation transformations.
+ * Testing Elasticsearch aggregation transformations.
  */
 public class AggregationTest {
 
@@ -53,20 +55,29 @@ public class AggregationTest {
   @BeforeClass
   public static void setupInstance() throws Exception {
 
-    final Map<String, String> mappings = ImmutableMap.of("cat1", "keyword",
-        "cat2", "keyword", "cat3", "keyword",
-        "val1", "long", "val2", "long");
+    final Map<String, String> mappings = ImmutableMap.<String, String>builder()
+        .put("cat1", "keyword")
+        .put("cat2", "keyword")
+        .put("cat3", "keyword")
+        .put("cat4", "date")
+        .put("cat5", "integer")
+        .put("val1", "long")
+        .put("val2", "long")
+        .build();
 
     NODE.createIndex(NAME, mappings);
 
-    String doc1 = "{'cat1': 'a', 'cat2': 'g', 'val1': 1 }".replace('\'', '"');
-    String doc2 = "{'cat2': 'g', 'cat3': 'y', 'val2': 5 }".replace('\'', '"');
-    String doc3 = "{'cat1': 'b', 'cat2':'h', 'cat3': 'z', 'val1': 7, 'val2': '42'}"
-        .replace('\'', '"');
+    String doc1 = "{cat1:'a', cat2:'g', val1:1, cat4:'2018-01-01', cat5:1}";
+    String doc2 = "{cat2:'g', cat3:'y', val2:5, cat4:'2019-12-12'}";
+    String doc3 = "{cat1:'b', cat2:'h', cat3:'z', cat5:2, val1:7, val2:42}";
 
-    List<ObjectNode> docs = new ArrayList<>();
+    final ObjectMapper mapper = new ObjectMapper()
+        .enable(JsonParser.Feature.ALLOW_UNQUOTED_FIELD_NAMES) // user-friendly settings to
+        .enable(JsonParser.Feature.ALLOW_SINGLE_QUOTES); // avoid too much quoting
+
+    final List<ObjectNode> docs = new ArrayList<>();
     for (String text: Arrays.asList(doc1, doc2, doc3)) {
-      docs.add((ObjectNode) NODE.mapper().readTree(text));
+      docs.add((ObjectNode) mapper.readTree(text));
     }
 
     NODE.insertBulk(NAME, docs);
@@ -85,6 +96,8 @@ public class AggregationTest {
             "select _MAP['cat1'] AS \"cat1\", "
                 + " _MAP['cat2']  AS \"cat2\", "
                 +  " _MAP['cat3'] AS \"cat3\", "
+                +  " _MAP['cat4'] AS \"cat4\", "
+                +  " _MAP['cat5'] AS \"cat5\", "
                 +  " _MAP['val1'] AS \"val1\", "
                 +  " _MAP['val2'] AS \"val2\" "
                 +  " from \"elastic\".\"%s\"", NAME);
@@ -118,7 +131,7 @@ public class AggregationTest {
   }
 
   @Test
-  public void all() throws Exception {
+  public void all() {
     CalciteAssert.that()
         .with(newConnectionFactory())
         .query("select count(*), sum(val1), sum(val2) from view")
@@ -141,7 +154,7 @@ public class AggregationTest {
   }
 
   @Test
-  public void cat1() throws Exception {
+  public void cat1() {
     CalciteAssert.that()
         .with(newConnectionFactory())
         .query("select cat1, sum(val1), sum(val2) from view group by cat1")
@@ -173,7 +186,7 @@ public class AggregationTest {
   }
 
   @Test
-  public void cat2() throws Exception {
+  public void cat2() {
     CalciteAssert.that()
         .with(newConnectionFactory())
         .query("select cat2, min(val1), max(val1), min(val2), max(val2) from view group by cat2")
@@ -194,7 +207,7 @@ public class AggregationTest {
   }
 
   @Test
-  public void cat1Cat2() throws Exception {
+  public void cat1Cat2() {
     CalciteAssert.that()
         .with(newConnectionFactory())
         .query("select cat1, cat2, sum(val1), sum(val2) from view group by cat1, cat2")
@@ -211,7 +224,7 @@ public class AggregationTest {
   }
 
   @Test
-  public void cat1Cat3() throws Exception {
+  public void cat1Cat3() {
     CalciteAssert.that()
         .with(newConnectionFactory())
         .query("select cat1, cat3, sum(val1), sum(val2) from view group by cat1, cat3")
@@ -224,7 +237,7 @@ public class AggregationTest {
    * Testing {@link org.apache.calcite.sql.SqlKind#ANY_VALUE} aggregate function
    */
   @Test
-  public void anyValue() throws Exception {
+  public void anyValue() {
     CalciteAssert.that()
         .with(newConnectionFactory())
         .query("select cat1, any_value(cat2) from view group by cat1")
@@ -247,7 +260,7 @@ public class AggregationTest {
   }
 
   @Test
-  public void cat1Cat2Cat3() throws Exception {
+  public void cat1Cat2Cat3() {
     CalciteAssert.that()
             .with(newConnectionFactory())
             .query("select cat1, cat2, cat3, count(*), sum(val1), sum(val2) from view "
@@ -256,6 +269,36 @@ public class AggregationTest {
                     "cat1=b; cat2=h; cat3=z; EXPR$3=1; EXPR$4=7.0; EXPR$5=42.0",
                     "cat1=null; cat2=g; cat3=y; EXPR$3=1; EXPR$4=0.0; EXPR$5=5.0");
   }
+
+  /**
+   * Group by
+   * <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/date.html">
+   * date</a> data type.
+   */
+  @Test
+  public void dateCat() {
+    CalciteAssert.that()
+        .with(newConnectionFactory())
+        .query("select cat4, sum(val1) from view group by cat4")
+        .returnsUnordered("cat4=1514764800000; EXPR$1=1.0",
+            "cat4=1576108800000; EXPR$1=0.0",
+            "cat4=null; EXPR$1=7.0");
+  }
+
+  /**
+   * Group by
+   * <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/number.html">
+   * number</a> data type.
+   */
+  @Test
+  public void integerCat() {
+    CalciteAssert.that()
+        .with(newConnectionFactory())
+        .query("select cat5, sum(val1) from view group by cat5")
+        .returnsUnordered("cat5=1; EXPR$1=1.0",
+            "cat5=null; EXPR$1=0.0",
+            "cat5=2; EXPR$1=7.0");
+  }
 }
 
 // End AggregationTest.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/ed3da62d/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/BooleanLogicTest.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/BooleanLogicTest.java b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/BooleanLogicTest.java
index c870234..c3a233a 100644
--- a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/BooleanLogicTest.java
+++ b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/BooleanLogicTest.java
@@ -54,7 +54,7 @@ public class BooleanLogicTest {
   @BeforeClass
   public static void setupInstance() throws Exception {
 
-    final Map<String, String> mapping = ImmutableMap.of("A", "keyword", "b", "keyword",
+    final Map<String, String> mapping = ImmutableMap.of("a", "keyword", "b", "keyword",
         "c", "keyword", "int", "long");
 
     NODE.createIndex(NAME, mapping);
@@ -80,7 +80,8 @@ public class BooleanLogicTest {
                 +  " from \"elastic\".\"%s\"", NAME);
 
         ViewTableMacro macro = ViewTable.viewMacro(root, viewSql,
-                Collections.singletonList("elastic"), Arrays.asList("elastic", "view"), false);
+            Collections.singletonList("elastic"),
+            Arrays.asList("elastic", "view"), false);
         root.add("VIEW", macro);
 
         return connection;
@@ -131,7 +132,7 @@ public class BooleanLogicTest {
     assertSingle("select * from view where c = 'c' and (a in ('a', 'b') or num in (41, 42))");
     assertSingle("select * from view where (a = 'a' or b = 'b') or (num = 42 and c = 'c')");
     assertSingle("select * from view where a = 'a' and (b = '0' or (b = 'b' and "
-            +  "(c = '0' or (c = 'c' and num = 42))))");
+        +  "(c = '0' or (c = 'c' and num = 42))))");
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/calcite/blob/ed3da62d/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ScrollingTest.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ScrollingTest.java b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ScrollingTest.java
index 5beec12..b792e20 100644
--- a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ScrollingTest.java
+++ b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ScrollingTest.java
@@ -14,7 +14,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.apache.calcite.adapter.elasticsearch;
 
 import org.apache.calcite.jdbc.CalciteConnection;


[13/15] calcite git commit: [CALCITE-2619] Reduce string literal creation cost by deferring and caching charset conversion (Ted Xu)

Posted by jh...@apache.org.
[CALCITE-2619] Reduce string literal creation cost by deferring and caching charset conversion (Ted Xu)

Close apache/calcite#911


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/36bd5fb2
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/36bd5fb2
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/36bd5fb2

Branch: refs/heads/master
Commit: 36bd5fb2e6db955a581de5f55b00a089c4bc1389
Parents: 96605a8
Author: Ted Xu <fr...@gmail.com>
Authored: Sat Nov 10 14:46:24 2018 +0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Sat Dec 1 14:42:49 2018 -0800

----------------------------------------------------------------------
 .../java/org/apache/calcite/rex/RexBuilder.java |  17 ++
 .../java/org/apache/calcite/sql/SqlUtil.java    |  65 +++++++-
 .../java/org/apache/calcite/util/NlsString.java | 155 +++++++++++++------
 .../org/apache/calcite/rex/RexBuilderTest.java  |  50 ++++++
 .../calcite/sql/parser/SqlParserTest.java       |   1 +
 5 files changed, 234 insertions(+), 54 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/36bd5fb2/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
index 5741a11..d346ec7 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
@@ -28,6 +28,7 @@ import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.rel.type.RelDataTypeField;
 import org.apache.calcite.runtime.FlatLists;
 import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlCollation;
 import org.apache.calcite.sql.SqlIntervalQualifier;
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlOperator;
@@ -1034,6 +1035,22 @@ public class RexBuilder {
   }
 
   /**
+   * Creates a character string literal with type CHAR.
+   *
+   * @param value       String value in bytes
+   * @param charsetName SQL-level charset name
+   * @param collation   Sql collation
+   * @return String     literal
+   */
+  protected RexLiteral makePreciseStringLiteral(ByteString value,
+      String charsetName, SqlCollation collation) {
+    return makeLiteral(
+        new NlsString(value, charsetName, collation),
+        typeFactory.createSqlType(SqlTypeName.CHAR),
+        SqlTypeName.CHAR);
+  }
+
+  /**
    * Ensures expression is interpreted as a specified type. The returned
    * expression may be wrapped with a cast.
    *

http://git-wip-us.apache.org/repos/asf/calcite/blob/36bd5fb2/core/src/main/java/org/apache/calcite/sql/SqlUtil.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlUtil.java b/core/src/main/java/org/apache/calcite/sql/SqlUtil.java
index 52ba9a4..461b7ed 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlUtil.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlUtil.java
@@ -16,6 +16,7 @@
  */
 package org.apache.calcite.sql;
 
+import org.apache.calcite.avatica.util.ByteString;
 import org.apache.calcite.linq4j.Ord;
 import org.apache.calcite.linq4j.function.Functions;
 import org.apache.calcite.rel.type.RelDataType;
@@ -38,11 +39,14 @@ import org.apache.calcite.util.Pair;
 import org.apache.calcite.util.Util;
 
 import com.google.common.base.Predicates;
+import com.google.common.base.Utf8;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterators;
 import com.google.common.collect.Lists;
 
 import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.nio.charset.UnsupportedCharsetException;
 import java.sql.DatabaseMetaData;
 import java.sql.SQLException;
 import java.text.MessageFormat;
@@ -836,18 +840,63 @@ public abstract class SqlUtil {
    * @return Java-level name, or null if SQL-level name is unknown
    */
   public static String translateCharacterSetName(String name) {
-    if (name.equals("LATIN1")) {
+    switch (name) {
+    case "BIG5":
+      return "Big5";
+    case "LATIN1":
       return "ISO-8859-1";
-    } else if (name.equals("UTF16")) {
-      return ConversionUtil.NATIVE_UTF16_CHARSET_NAME;
-    } else if (name.equals(ConversionUtil.NATIVE_UTF16_CHARSET_NAME)) {
-      // no translation needed
+    case "GB2312":
+    case "GBK":
       return name;
-    } else if (name.equals("ISO-8859-1")) {
-      // no translation needed
+    case "UTF8":
+      return "UTF-8";
+    case "UTF16":
+      return ConversionUtil.NATIVE_UTF16_CHARSET_NAME;
+    case "UTF-16BE":
+    case "UTF-16LE":
+    case "ISO-8859-1":
+    case "UTF-8":
       return name;
+    default:
+      return null;
+    }
+  }
+
+  /**
+   * Returns the Java-level {@link Charset} based on given SQL-level name.
+   *
+   * @param charsetName Sql charset name, must not be null.
+   * @return charset, or default charset if charsetName is null.
+   * @throws UnsupportedCharsetException If no support for the named charset
+   *     is available in this instance of the Java virtual machine
+   */
+  public static Charset getCharset(String charsetName) {
+    assert charsetName != null;
+    charsetName = charsetName.toUpperCase(Locale.ROOT);
+    String javaCharsetName = translateCharacterSetName(charsetName);
+    if (javaCharsetName == null) {
+      throw new UnsupportedCharsetException(charsetName);
+    }
+    return Charset.forName(javaCharsetName);
+  }
+
+  /**
+   * Validate if value can be decoded by given charset.
+   *
+   * @param value nls string in byte array
+   * @param charset charset
+   * @throws RuntimeException If the given value cannot be represented in the
+   *     given charset
+   */
+  public static void validateCharset(ByteString value, Charset charset) {
+    if (charset == StandardCharsets.UTF_8) {
+      final byte[] bytes = value.getBytes();
+      if (!Utf8.isWellFormed(bytes)) {
+        //CHECKSTYLE: IGNORE 1
+        final String string = new String(bytes, charset);
+        throw RESOURCE.charsetEncoding(string, charset.name()).ex();
+      }
     }
-    return null;
   }
 
   /** If a node is "AS", returns the underlying expression; otherwise returns

http://git-wip-us.apache.org/repos/asf/calcite/blob/36bd5fb2/core/src/main/java/org/apache/calcite/util/NlsString.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/util/NlsString.java b/core/src/main/java/org/apache/calcite/util/NlsString.java
index fbb9386..fd41811 100644
--- a/core/src/main/java/org/apache/calcite/util/NlsString.java
+++ b/core/src/main/java/org/apache/calcite/util/NlsString.java
@@ -16,19 +16,25 @@
  */
 package org.apache.calcite.util;
 
+import org.apache.calcite.avatica.util.ByteString;
 import org.apache.calcite.runtime.SqlFunctions;
 import org.apache.calcite.sql.SqlCollation;
 import org.apache.calcite.sql.SqlUtil;
 
-import java.nio.CharBuffer;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+
+import java.nio.ByteBuffer;
 import java.nio.charset.CharacterCodingException;
 import java.nio.charset.Charset;
-import java.nio.charset.CharsetEncoder;
+import java.nio.charset.CharsetDecoder;
 import java.nio.charset.IllegalCharsetNameException;
 import java.nio.charset.UnsupportedCharsetException;
 import java.util.List;
 import java.util.Locale;
 import java.util.Objects;
+import javax.annotation.Nonnull;
 
 import static org.apache.calcite.util.Static.RESOURCE;
 
@@ -39,8 +45,31 @@ import static org.apache.calcite.util.Static.RESOURCE;
 public class NlsString implements Comparable<NlsString>, Cloneable {
   //~ Instance fields --------------------------------------------------------
 
+  private static final LoadingCache<Pair<ByteString, Charset>, String>
+      DECODE_MAP =
+      CacheBuilder.newBuilder()
+          .softValues()
+          .build(
+              new CacheLoader<Pair<ByteString, Charset>, String>() {
+                public String load(@Nonnull Pair<ByteString, Charset> key) {
+                  final Charset charset = key.right;
+                  final CharsetDecoder decoder = charset.newDecoder();
+                  final byte[] bytes = key.left.getBytes();
+                  final ByteBuffer buffer = ByteBuffer.wrap(bytes);
+                  try {
+                    return decoder.decode(buffer).toString();
+                  } catch (CharacterCodingException ex) {
+                    throw RESOURCE.charsetEncoding(
+                        //CHECKSTYLE: IGNORE 1
+                        new String(bytes, Charset.defaultCharset()),
+                        charset.name()).ex();
+                  }
+                }
+              });
+
+  private final String stringValue;
+  private final ByteString bytesValue;
   private final String charsetName;
-  private final String value;
   private final Charset charset;
   private final SqlCollation collation;
 
@@ -49,43 +78,71 @@ public class NlsString implements Comparable<NlsString>, Cloneable {
   /**
    * Creates a string in a specified character set.
    *
-   * @param value       String constant, must not be null
-   * @param charsetName Name of the character set, may be null
+   * @param bytesValue  Byte array constant, must not be null
+   * @param charsetName Name of the character set, must not be null
    * @param collation   Collation, may be null
+   *
    * @throws IllegalCharsetNameException If the given charset name is illegal
    * @throws UnsupportedCharsetException If no support for the named charset
    *     is available in this instance of the Java virtual machine
    * @throws RuntimeException If the given value cannot be represented in the
    *     given charset
    */
-  public NlsString(
-      String value,
-      String charsetName,
+  public NlsString(ByteString bytesValue, String charsetName,
       SqlCollation collation) {
-    assert value != null;
-    if (null != charsetName) {
-      charsetName = charsetName.toUpperCase(Locale.ROOT);
-      this.charsetName = charsetName;
-      String javaCharsetName =
-          SqlUtil.translateCharacterSetName(charsetName);
-      if (javaCharsetName == null) {
-        throw new UnsupportedCharsetException(charsetName);
-      }
-      this.charset = Charset.forName(javaCharsetName);
-      CharsetEncoder encoder = charset.newEncoder();
-
-      // dry run to see if encoding hits any problems
-      try {
-        encoder.encode(CharBuffer.wrap(value));
-      } catch (CharacterCodingException ex) {
-        throw RESOURCE.charsetEncoding(value, javaCharsetName).ex();
-      }
+    this(null, Objects.requireNonNull(bytesValue),
+        Objects.requireNonNull(charsetName), collation);
+  }
+
+  /**
+   * Easy constructor for Java string.
+   *
+   * @param stringValue String constant, must not be null
+   * @param charsetName Name of the character set, may be null
+   * @param collation Collation, may be null
+   *
+   * @throws IllegalCharsetNameException If the given charset name is illegal
+   * @throws UnsupportedCharsetException If no support for the named charset
+   *     is available in this instance of the Java virtual machine
+   * @throws RuntimeException If the given value cannot be represented in the
+   *     given charset
+   */
+  public NlsString(String stringValue, String charsetName,
+      SqlCollation collation) {
+    this(Objects.requireNonNull(stringValue), null, charsetName, collation);
+  }
+
+  /** Internal constructor; other constructors must call it. */
+  private NlsString(String stringValue, ByteString bytesValue,
+      String charsetName, SqlCollation collation) {
+    if (charsetName != null) {
+      this.charsetName = charsetName.toUpperCase(Locale.ROOT);
+      this.charset = SqlUtil.getCharset(charsetName);
     } else {
       this.charsetName = null;
       this.charset = null;
     }
+    if ((stringValue != null) == (bytesValue != null)) {
+      throw new IllegalArgumentException("Specify stringValue or bytesValue");
+    }
+    if (bytesValue != null) {
+      if (charsetName == null) {
+        throw new IllegalArgumentException("Bytes value requires charset");
+      }
+      SqlUtil.validateCharset(bytesValue, charset);
+    } else {
+      // Java string can be malformed if LATIN1 is required.
+      if (this.charsetName != null
+          && (this.charsetName.equals("LATIN1")
+          || this.charsetName.equals("ISO-8859-1"))) {
+        if (!charset.newEncoder().canEncode(stringValue)) {
+          throw RESOURCE.charsetEncoding(stringValue, charset.name()).ex();
+        }
+      }
+    }
     this.collation = collation;
-    this.value = value;
+    this.stringValue = stringValue;
+    this.bytesValue = bytesValue;
   }
 
   //~ Methods ----------------------------------------------------------------
@@ -99,25 +156,22 @@ public class NlsString implements Comparable<NlsString>, Cloneable {
   }
 
   public int hashCode() {
-    return Objects.hash(value, charsetName, collation);
+    return Objects.hash(stringValue, bytesValue, charsetName, collation);
   }
 
   public boolean equals(Object obj) {
-    if (!(obj instanceof NlsString)) {
-      return false;
-    }
-    NlsString that = (NlsString) obj;
-    return Objects.equals(value, that.value)
-        && Objects.equals(charsetName, that.charsetName)
-        && Objects.equals(collation, that.collation);
+    return this == obj
+        || obj instanceof NlsString
+        && Objects.equals(stringValue, ((NlsString) obj).stringValue)
+        && Objects.equals(bytesValue, ((NlsString) obj).bytesValue)
+        && Objects.equals(charsetName, ((NlsString) obj).charsetName)
+        && Objects.equals(collation, ((NlsString) obj).collation);
   }
 
-  // implement Comparable
-  public int compareTo(NlsString other) {
+  @Override public int compareTo(NlsString other) {
     // TODO jvs 18-Jan-2006:  Actual collation support.  This just uses
     // the default collation.
-
-    return value.compareTo(other.value);
+    return getValue().compareTo(other.getValue());
   }
 
   public String getCharsetName() {
@@ -133,7 +187,11 @@ public class NlsString implements Comparable<NlsString>, Cloneable {
   }
 
   public String getValue() {
-    return value;
+    if (stringValue == null) {
+      assert bytesValue != null;
+      return DECODE_MAP.getUnchecked(Pair.of(bytesValue, charset));
+    }
+    return stringValue;
   }
 
   /**
@@ -141,8 +199,8 @@ public class NlsString implements Comparable<NlsString>, Cloneable {
    * right.
    */
   public NlsString rtrim() {
-    String trimmed = SqlFunctions.rtrim(value);
-    if (!trimmed.equals(value)) {
+    String trimmed = SqlFunctions.rtrim(getValue());
+    if (!trimmed.equals(getValue())) {
       return new NlsString(trimmed, charsetName, collation);
     }
     return this;
@@ -165,7 +223,7 @@ public class NlsString implements Comparable<NlsString>, Cloneable {
       ret.append(charsetName);
     }
     ret.append("'");
-    ret.append(Util.replace(value, "'", "''"));
+    ret.append(Util.replace(getValue(), "'", "''"));
     ret.append("'");
 
     // NOTE jvs 3-Feb-2005:  see FRG-78 for why this should go away
@@ -200,12 +258,12 @@ public class NlsString implements Comparable<NlsString>, Cloneable {
     }
     String charSetName = args.get(0).charsetName;
     SqlCollation collation = args.get(0).collation;
-    int length = args.get(0).value.length();
+    int length = args.get(0).getValue().length();
 
     // sum string lengths and validate
     for (int i = 1; i < args.size(); i++) {
       final NlsString arg = args.get(i);
-      length += arg.value.length();
+      length += arg.getValue().length();
       if (!((arg.charsetName == null)
           || arg.charsetName.equals(charSetName))) {
         throw new IllegalArgumentException("mismatched charsets");
@@ -218,7 +276,7 @@ public class NlsString implements Comparable<NlsString>, Cloneable {
 
     StringBuilder sb = new StringBuilder(length);
     for (NlsString arg : args) {
-      sb.append(arg.value);
+      sb.append(arg.getValue());
     }
     return new NlsString(
         sb.toString(),
@@ -231,6 +289,11 @@ public class NlsString implements Comparable<NlsString>, Cloneable {
   public NlsString copy(String value) {
     return new NlsString(value, charsetName, collation);
   }
+
+  /** Returns the value as a {@link ByteString}. */
+  public ByteString getValueBytes() {
+    return bytesValue;
+  }
 }
 
 // End NlsString.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/36bd5fb2/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java b/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
index f959a76..1445e95 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
@@ -16,12 +16,15 @@
  */
 package org.apache.calcite.rex;
 
+import org.apache.calcite.avatica.util.ByteString;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.rel.type.RelDataTypeSystem;
+import org.apache.calcite.sql.SqlCollation;
 import org.apache.calcite.sql.type.SqlTypeFactoryImpl;
 import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.util.DateString;
+import org.apache.calcite.util.NlsString;
 import org.apache.calcite.util.TimeString;
 import org.apache.calcite.util.TimestampString;
 import org.apache.calcite.util.TimestampWithTimeZoneString;
@@ -29,6 +32,7 @@ import org.apache.calcite.util.Util;
 
 import org.junit.Test;
 
+import java.nio.charset.StandardCharsets;
 import java.util.Calendar;
 import java.util.TimeZone;
 
@@ -489,6 +493,52 @@ public class RexBuilderTest {
     }
   }
 
+  /**
+   * Test string literal encoding.
+   */
+  @Test public void testStringLiteral() {
+    final RelDataTypeFactory typeFactory =
+        new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+    final RelDataType varchar =
+        typeFactory.createSqlType(SqlTypeName.VARCHAR);
+    final RexBuilder builder = new RexBuilder(typeFactory);
+
+    final NlsString latin1 = new NlsString("foobar", "LATIN1", SqlCollation.IMPLICIT);
+    final NlsString utf8 = new NlsString("foobar", "UTF8", SqlCollation.IMPLICIT);
+
+    RexNode literal = builder.makePreciseStringLiteral("foobar");
+    assertEquals("'foobar'", literal.toString());
+    literal = builder.makePreciseStringLiteral(
+        new ByteString(new byte[] { 'f', 'o', 'o', 'b', 'a', 'r'}),
+        "UTF8",
+        SqlCollation.IMPLICIT);
+    assertEquals("_UTF8'foobar'", literal.toString());
+    literal = builder.makePreciseStringLiteral(
+        new ByteString("\u82f1\u56fd".getBytes(StandardCharsets.UTF_8)),
+        "UTF8",
+        SqlCollation.IMPLICIT);
+    assertEquals("_UTF8'\u82f1\u56fd'", literal.toString());
+    // Test again to check decode cache.
+    literal = builder.makePreciseStringLiteral(
+        new ByteString("\u82f1".getBytes(StandardCharsets.UTF_8)),
+        "UTF8",
+        SqlCollation.IMPLICIT);
+    assertEquals("_UTF8'\u82f1'", literal.toString());
+    try {
+      literal = builder.makePreciseStringLiteral(
+          new ByteString("\u82f1\u56fd".getBytes(StandardCharsets.UTF_8)),
+          "GB2312",
+          SqlCollation.IMPLICIT);
+      fail("expected exception, got " + literal);
+    } catch (RuntimeException e) {
+      assertThat(e.getMessage(), containsString("Failed to encode"));
+    }
+    literal = builder.makeLiteral(latin1, varchar, false);
+    assertEquals("_LATIN1'foobar'", literal.toString());
+    literal = builder.makeLiteral(utf8, varchar, false);
+    assertEquals("_UTF8'foobar'", literal.toString());
+  }
+
 }
 
 // End RexBuilderTest.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/36bd5fb2/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
index a7a1233..af49b2f 100644
--- a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
@@ -3727,6 +3727,7 @@ public class SqlParserTest {
     checkExp(
         "_iso-8859-1'bye' \n\n--\n-- this is a comment\n' bye'",
         "_ISO-8859-1'bye'\n' bye'");
+    checkExp("_utf8'hi'", "_UTF8'hi'");
 
     // newline in string literal
     checkExp("'foo\rbar'", "'foo\rbar'");


[05/15] calcite git commit: [CALCITE-2632] Ensure that RexNode and its sub-classes implement hashCode and equals methods (Zoltan Haindrich)

Posted by jh...@apache.org.
[CALCITE-2632] Ensure that RexNode and its sub-classes implement hashCode and equals methods (Zoltan Haindrich)

Previously there was no default hashCode/equals implementations. To
ensure that they are working properly is crucial while working with
basic collections like sets/maps.

Fix ups (Julian Hyde):

* Use Objects.equals(x, y) in preference to x.equals(y) only when x or y
 may be null; and use x == y when objects are primitive.

* When implementing Object.equals, use the pattern

  return this == obj
    || obj instanceof Type
    && a.equals(((Type) obj).a) ...

  whenever possible.

* In the many cases where RexNode.toString() is used as a key in
  collections, use the raw RexNode instead.

Close apache/calcite#943


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/847e76cd
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/847e76cd
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/847e76cd

Branch: refs/heads/master
Commit: 847e76cde2894ea7540749f68669ce2d910c2fa9
Parents: ce47088
Author: Zoltan Haindrich <ki...@rxd.hu>
Authored: Mon Nov 26 15:58:50 2018 +0100
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Nov 30 20:29:35 2018 -0800

----------------------------------------------------------------------
 .../calcite/plan/RexImplicationChecker.java     |   4 +-
 .../calcite/plan/SubstitutionVisitor.java       |   8 +-
 .../java/org/apache/calcite/rel/core/Match.java |   2 +-
 .../org/apache/calcite/rel/core/Window.java     |  13 ++
 .../calcite/rel/metadata/RelMdPredicates.java   | 137 +++++++++----------
 .../apache/calcite/rel/metadata/RelMdUtil.java  |  39 +-----
 .../calcite/rel/mutable/MutableFilter.java      |   5 +-
 .../apache/calcite/rel/mutable/MutableJoin.java |   6 +-
 .../calcite/rel/mutable/MutableSemiJoin.java    |   6 +-
 .../rel/rules/AbstractMaterializedViewRule.java |  97 ++++++-------
 .../calcite/rel/rules/DateRangeRules.java       |  21 ++-
 .../rel/rules/JoinPushExpressionsRule.java      |   2 +-
 .../apache/calcite/rel/rules/PushProjector.java |  39 ++----
 .../calcite/rel/rules/ReduceDecimalsRule.java   |   8 +-
 .../rel/rules/ReduceExpressionsRule.java        |   2 +-
 .../calcite/rel/rules/SubQueryRemoveRule.java   |   2 +-
 .../org/apache/calcite/rex/LogicVisitor.java    |   2 +-
 .../java/org/apache/calcite/rex/RexCall.java    |  22 ++-
 .../apache/calcite/rex/RexCorrelVariable.java   |  12 ++
 .../org/apache/calcite/rex/RexDynamicParam.java |  14 ++
 .../java/org/apache/calcite/rex/RexNode.java    |  13 ++
 .../java/org/apache/calcite/rex/RexOver.java    |   6 +-
 .../apache/calcite/rex/RexProgramBuilder.java   |   4 +-
 .../org/apache/calcite/rex/RexRangeRef.java     |  13 ++
 .../org/apache/calcite/rex/RexSimplify.java     |  85 ++++++------
 .../org/apache/calcite/rex/RexSubQuery.java     |   7 +-
 .../java/org/apache/calcite/rex/RexUtil.java    |  46 +++----
 .../org/apache/calcite/rex/RexVariable.java     |  10 +-
 .../calcite/sql2rel/SqlToRelConverter.java      |  16 +--
 .../org/apache/calcite/test/RelOptRulesTest.xml |   9 +-
 .../calcite/test/SqlToRelConverterTest.xml      |  34 ++---
 .../elasticsearch/ElasticsearchRules.java       |   4 +-
 32 files changed, 336 insertions(+), 352 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/plan/RexImplicationChecker.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/plan/RexImplicationChecker.java b/core/src/main/java/org/apache/calcite/plan/RexImplicationChecker.java
index f2b32f2..d769b7e 100644
--- a/core/src/main/java/org/apache/calcite/plan/RexImplicationChecker.java
+++ b/core/src/main/java/org/apache/calcite/plan/RexImplicationChecker.java
@@ -168,7 +168,7 @@ public class RexImplicationChecker {
     }
 
     // E.g. "x is null" implies "x is null".
-    if (RexUtil.eq(first, second)) {
+    if (first.equals(second)) {
       return true;
     }
 
@@ -184,7 +184,7 @@ public class RexImplicationChecker {
       final RexNode operand = ((RexCall) second).getOperands().get(0);
       final Strong strong = new Strong() {
         @Override public boolean isNull(RexNode node) {
-          return RexUtil.eq(node, operand)
+          return node.equals(operand)
               || super.isNull(node);
         }
       };

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
index 4212fae..b30763c 100644
--- a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
+++ b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
@@ -1362,13 +1362,13 @@ public class SubstitutionVisitor {
   /** Builds a shuttle that stores a list of expressions, and can map incoming
    * expressions to references to them. */
   protected static RexShuttle getRexShuttle(MutableProject target) {
-    final Map<String, Integer> map = new HashMap<>();
+    final Map<RexNode, Integer> map = new HashMap<>();
     for (RexNode e : target.projects) {
-      map.put(e.toString(), map.size());
+      map.put(e, map.size());
     }
     return new RexShuttle() {
       @Override public RexNode visitInputRef(RexInputRef ref) {
-        final Integer integer = map.get(ref.getName());
+        final Integer integer = map.get(ref);
         if (integer != null) {
           return new RexInputRef(integer, ref.getType());
         }
@@ -1376,7 +1376,7 @@ public class SubstitutionVisitor {
       }
 
       @Override public RexNode visitCall(RexCall call) {
-        final Integer integer = map.get(call.toString());
+        final Integer integer = map.get(call);
         if (integer != null) {
           return new RexInputRef(integer, call.getType());
         }

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/core/Match.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/core/Match.java b/core/src/main/java/org/apache/calcite/rel/core/Match.java
index d2c170e..742fa81 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/Match.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/Match.java
@@ -277,7 +277,7 @@ public abstract class Match extends SingleRel {
           }
           boolean update = true;
           for (RexMRAggCall rex : set) {
-            if (rex.toString().equals(aggCall.toString())) {
+            if (rex.equals(aggCall)) {
               update = false;
               break;
             }

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/core/Window.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/core/Window.java b/core/src/main/java/org/apache/calcite/rel/core/Window.java
index 186f7d3..013e7cc 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/Window.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/Window.java
@@ -47,6 +47,7 @@ import com.google.common.collect.ImmutableList;
 
 import java.util.AbstractList;
 import java.util.List;
+import java.util.Objects;
 
 /**
  * A relational expression representing a set of window aggregates.
@@ -361,6 +362,18 @@ public abstract class Window extends SingleRel {
       this.distinct = distinct;
     }
 
+    /** {@inheritDoc}
+     *
+     * <p>Override {@link RexCall}, defining equality based on identity.
+     */
+    @Override public boolean equals(Object obj) {
+      return this == obj;
+    }
+
+    @Override public int hashCode() {
+      return Objects.hash(digest, ordinal, distinct);
+    }
+
     @Override public RexCall clone(RelDataType type, List<RexNode> operands) {
       throw new UnsupportedOperationException();
     }

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
index 985464c..a7993cb 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
@@ -59,9 +59,9 @@ import org.apache.calcite.util.mapping.Mappings;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Collections;
 import java.util.HashMap;
@@ -69,7 +69,6 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Set;
 import java.util.SortedMap;
@@ -358,58 +357,56 @@ public class RelMdPredicates
    * Infers predicates for a Union.
    */
   public RelOptPredicateList getPredicates(Union union, RelMetadataQuery mq) {
-    RexBuilder rexBuilder = union.getCluster().getRexBuilder();
+    final RexBuilder rexBuilder = union.getCluster().getRexBuilder();
 
-    Map<String, RexNode> finalPreds = new HashMap<>();
-    List<RexNode> finalResidualPreds = new ArrayList<>();
-    for (int i = 0; i < union.getInputs().size(); i++) {
-      RelNode input = union.getInputs().get(i);
-      RelOptPredicateList info = mq.getPulledUpPredicates(input);
+    Set<RexNode> finalPredicates = new HashSet<>();
+    final List<RexNode> finalResidualPredicates = new ArrayList<>();
+    for (Ord<RelNode> input : Ord.zip(union.getInputs())) {
+      RelOptPredicateList info = mq.getPulledUpPredicates(input.e);
       if (info.pulledUpPredicates.isEmpty()) {
         return RelOptPredicateList.EMPTY;
       }
-      Map<String, RexNode> preds = new HashMap<>();
-      List<RexNode> residualPreds = new ArrayList<>();
+      final Set<RexNode> predicates = new HashSet<>();
+      final List<RexNode> residualPredicates = new ArrayList<>();
       for (RexNode pred : info.pulledUpPredicates) {
-        final String predDigest = pred.toString();
-        if (i == 0) {
-          preds.put(predDigest, pred);
+        if (input.i == 0) {
+          predicates.add(pred);
           continue;
         }
-        if (finalPreds.containsKey(predDigest)) {
-          preds.put(predDigest, pred);
+        if (finalPredicates.contains(pred)) {
+          predicates.add(pred);
         } else {
-          residualPreds.add(pred);
+          residualPredicates.add(pred);
         }
       }
-      // Add new residual preds
-      finalResidualPreds.add(RexUtil.composeConjunction(rexBuilder, residualPreds));
+      // Add new residual predicates
+      finalResidualPredicates.add(RexUtil.composeConjunction(rexBuilder, residualPredicates));
       // Add those that are not part of the final set to residual
-      for (Entry<String, RexNode> e : finalPreds.entrySet()) {
-        if (!preds.containsKey(e.getKey())) {
+      for (RexNode e : finalPredicates) {
+        if (!predicates.contains(e)) {
           // This node was in previous union inputs, but it is not in this one
-          for (int j = 0; j < i; j++) {
-            finalResidualPreds.set(j,
+          for (int j = 0; j < input.i; j++) {
+            finalResidualPredicates.set(j,
                 RexUtil.composeConjunction(rexBuilder,
-                    Lists.newArrayList(finalResidualPreds.get(j), e.getValue())));
+                    Arrays.asList(finalResidualPredicates.get(j), e)));
           }
         }
       }
-      // Final preds
-      finalPreds = preds;
+      // Final predicates
+      finalPredicates = predicates;
     }
 
-    List<RexNode> preds = new ArrayList<>(finalPreds.values());
+    final List<RexNode> predicates = new ArrayList<>(finalPredicates);
     final RelOptCluster cluster = union.getCluster();
     final RexExecutor executor =
         Util.first(cluster.getPlanner().getExecutor(), RexUtil.EXECUTOR);
-    final RelOptPredicateList predicates = RelOptPredicateList.EMPTY;
-    RexNode disjPred = new RexSimplify(rexBuilder, predicates, executor)
-        .simplifyOrs(finalResidualPreds);
-    if (!disjPred.isAlwaysTrue()) {
-      preds.add(disjPred);
+    RexNode disjunctivePredicate =
+        new RexSimplify(rexBuilder, RelOptPredicateList.EMPTY, executor)
+            .simplifyOrs(finalResidualPredicates);
+    if (!disjunctivePredicate.isAlwaysTrue()) {
+      predicates.add(disjunctivePredicate);
     }
-    return RelOptPredicateList.of(rexBuilder, preds);
+    return RelOptPredicateList.of(rexBuilder, predicates);
   }
 
   /**
@@ -479,20 +476,20 @@ public class RelMdPredicates
     final ImmutableBitSet rightFieldsBitSet;
     final ImmutableBitSet allFieldsBitSet;
     SortedMap<Integer, BitSet> equivalence;
-    final Map<String, ImmutableBitSet> exprFields;
-    final Set<String> allExprDigests;
-    final Set<String> equalityPredicates;
+    final Map<RexNode, ImmutableBitSet> exprFields;
+    final Set<RexNode> allExprs;
+    final Set<RexNode> equalityPredicates;
     final RexNode leftChildPredicates;
     final RexNode rightChildPredicates;
     final RexSimplify simplify;
 
     JoinConditionBasedPredicateInference(Join joinRel,
-        RexNode lPreds, RexNode rPreds, RexSimplify simplify) {
-      this(joinRel, joinRel instanceof SemiJoin, lPreds, rPreds, simplify);
+        RexNode leftPredicates, RexNode rightPredicates, RexSimplify simplify) {
+      this(joinRel, joinRel instanceof SemiJoin, leftPredicates, rightPredicates, simplify);
     }
 
     private JoinConditionBasedPredicateInference(Join joinRel, boolean isSemiJoin,
-        RexNode lPreds, RexNode rPreds, RexSimplify simplify) {
+        RexNode leftPredicates, RexNode rightPredicates, RexSimplify simplify) {
       super();
       this.joinRel = joinRel;
       this.isSemiJoin = isSemiJoin;
@@ -508,35 +505,35 @@ public class RelMdPredicates
           nSysFields + nFieldsLeft + nFieldsRight);
 
       exprFields = new HashMap<>();
-      allExprDigests = new HashSet<>();
+      allExprs = new HashSet<>();
 
-      if (lPreds == null) {
+      if (leftPredicates == null) {
         leftChildPredicates = null;
       } else {
         Mappings.TargetMapping leftMapping = Mappings.createShiftMapping(
             nSysFields + nFieldsLeft, nSysFields, 0, nFieldsLeft);
-        leftChildPredicates = lPreds.accept(
+        leftChildPredicates = leftPredicates.accept(
             new RexPermuteInputsShuttle(leftMapping, joinRel.getInput(0)));
 
-        allExprDigests.add(leftChildPredicates.toString());
+        allExprs.add(leftChildPredicates);
         for (RexNode r : RelOptUtil.conjunctions(leftChildPredicates)) {
-          exprFields.put(r.toString(), RelOptUtil.InputFinder.bits(r));
-          allExprDigests.add(r.toString());
+          exprFields.put(r, RelOptUtil.InputFinder.bits(r));
+          allExprs.add(r);
         }
       }
-      if (rPreds == null) {
+      if (rightPredicates == null) {
         rightChildPredicates = null;
       } else {
         Mappings.TargetMapping rightMapping = Mappings.createShiftMapping(
             nSysFields + nFieldsLeft + nFieldsRight,
             nSysFields + nFieldsLeft, 0, nFieldsRight);
-        rightChildPredicates = rPreds.accept(
+        rightChildPredicates = rightPredicates.accept(
             new RexPermuteInputsShuttle(rightMapping, joinRel.getInput(1)));
 
-        allExprDigests.add(rightChildPredicates.toString());
+        allExprs.add(rightChildPredicates);
         for (RexNode r : RelOptUtil.conjunctions(rightChildPredicates)) {
-          exprFields.put(r.toString(), RelOptUtil.InputFinder.bits(r));
-          allExprDigests.add(r.toString());
+          exprFields.put(r, RelOptUtil.InputFinder.bits(r));
+          allExprs.add(r);
         }
       }
 
@@ -555,8 +552,7 @@ public class RelMdPredicates
               compose(rexBuilder, ImmutableList.of(joinRel.getCondition())));
 
       final EquivalenceFinder eF = new EquivalenceFinder();
-      new ArrayList<>(
-          Lists.transform(exprs, input -> input.accept(eF)));
+      exprs.forEach(input -> input.accept(eF));
 
       equivalence = BitSets.closure(equivalence);
     }
@@ -577,12 +573,12 @@ public class RelMdPredicates
     public RelOptPredicateList inferPredicates(
         boolean includeEqualityInference) {
       final List<RexNode> inferredPredicates = new ArrayList<>();
-      final Set<String> allExprDigests = new HashSet<>(this.allExprDigests);
+      final Set<RexNode> allExprs = new HashSet<>(this.allExprs);
       final JoinRelType joinType = joinRel.getJoinType();
       switch (joinType) {
       case INNER:
       case LEFT:
-        infer(leftChildPredicates, allExprDigests, inferredPredicates,
+        infer(leftChildPredicates, allExprs, inferredPredicates,
             includeEqualityInference,
             joinType == JoinRelType.LEFT ? rightFieldsBitSet
                 : allFieldsBitSet);
@@ -591,7 +587,7 @@ public class RelMdPredicates
       switch (joinType) {
       case INNER:
       case RIGHT:
-        infer(rightChildPredicates, allExprDigests, inferredPredicates,
+        infer(rightChildPredicates, allExprs, inferredPredicates,
             includeEqualityInference,
             joinType == JoinRelType.RIGHT ? leftFieldsBitSet
                 : allFieldsBitSet);
@@ -659,12 +655,12 @@ public class RelMdPredicates
       return rightChildPredicates;
     }
 
-    private void infer(RexNode predicates, Set<String> allExprsDigests,
+    private void infer(RexNode predicates, Set<RexNode> allExprs,
         List<RexNode> inferredPredicates, boolean includeEqualityInference,
         ImmutableBitSet inferringFields) {
       for (RexNode r : RelOptUtil.conjunctions(predicates)) {
         if (!includeEqualityInference
-            && equalityPredicates.contains(r.toString())) {
+            && equalityPredicates.contains(r)) {
           continue;
         }
         for (Mapping m : mappings(r)) {
@@ -676,33 +672,31 @@ public class RelMdPredicates
           // some duplicates in in result pulledUpPredicates
           RexNode simplifiedTarget =
               simplify.simplifyFilterPredicates(RelOptUtil.conjunctions(tr));
-          if (checkTarget(inferringFields, allExprsDigests, tr)
-              && checkTarget(inferringFields, allExprsDigests, simplifiedTarget)) {
+          if (checkTarget(inferringFields, allExprs, tr)
+              && checkTarget(inferringFields, allExprs, simplifiedTarget)) {
             inferredPredicates.add(simplifiedTarget);
-            allExprsDigests.add(simplifiedTarget.toString());
+            allExprs.add(simplifiedTarget);
           }
         }
       }
     }
 
     Iterable<Mapping> mappings(final RexNode predicate) {
-      return () -> {
-        ImmutableBitSet fields = exprFields.get(predicate.toString());
-        if (fields.cardinality() == 0) {
-          return Collections.emptyIterator();
-        }
-        return new ExprsItr(fields);
-      };
+      final ImmutableBitSet fields = exprFields.get(predicate);
+      if (fields.cardinality() == 0) {
+        return Collections.emptyList();
+      }
+      return () -> new ExprsItr(fields);
     }
 
     private boolean checkTarget(ImmutableBitSet inferringFields,
-        Set<String> allExprsDigests, RexNode tr) {
+        Set<RexNode> allExprs, RexNode tr) {
       return inferringFields.contains(RelOptUtil.InputFinder.bits(tr))
-          && !allExprsDigests.contains(tr.toString())
+          && !allExprs.contains(tr)
           && !isAlwaysTrue(tr);
     }
 
-    private void equivalent(int p1, int p2) {
+    private void markAsEquivalent(int p1, int p2) {
       BitSet b = equivalence.get(p1);
       b.set(p2);
 
@@ -728,9 +722,8 @@ public class RelMdPredicates
           int lPos = pos(call.getOperands().get(0));
           int rPos = pos(call.getOperands().get(1));
           if (lPos != -1 && rPos != -1) {
-            JoinConditionBasedPredicateInference.this.equivalent(lPos, rPos);
-            JoinConditionBasedPredicateInference.this.equalityPredicates
-                .add(call.toString());
+            markAsEquivalent(lPos, rPos);
+            equalityPredicates.add(call);
           }
         }
         return null;

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java
index 6229f4e..73cae72 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java
@@ -49,7 +49,7 @@ import com.google.common.collect.ImmutableList;
 
 import java.math.BigDecimal;
 import java.util.ArrayList;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -433,20 +433,9 @@ public class RelMdUtil {
       RexBuilder rexBuilder,
       RexNode pred1,
       RexNode pred2) {
-    final List<RexNode> unionList = new ArrayList<>();
-    final Set<String> strings = new HashSet<>();
-
-    for (RexNode rex : RelOptUtil.conjunctions(pred1)) {
-      if (strings.add(rex.toString())) {
-        unionList.add(rex);
-      }
-    }
-    for (RexNode rex2 : RelOptUtil.conjunctions(pred2)) {
-      if (strings.add(rex2.toString())) {
-        unionList.add(rex2);
-      }
-    }
-
+    final Set<RexNode> unionList = new LinkedHashSet<>();
+    unionList.addAll(RelOptUtil.conjunctions(pred1));
+    unionList.addAll(RelOptUtil.conjunctions(pred2));
     return RexUtil.composeConjunction(rexBuilder, unionList, true);
   }
 
@@ -463,23 +452,9 @@ public class RelMdUtil {
       RexBuilder rexBuilder,
       RexNode pred1,
       RexNode pred2) {
-    final List<RexNode> list1 = RelOptUtil.conjunctions(pred1);
-    final List<RexNode> list2 = RelOptUtil.conjunctions(pred2);
-    final List<RexNode> minusList = new ArrayList<>();
-
-    for (RexNode rex1 : list1) {
-      boolean add = true;
-      for (RexNode rex2 : list2) {
-        if (rex2.toString().compareTo(rex1.toString()) == 0) {
-          add = false;
-          break;
-        }
-      }
-      if (add) {
-        minusList.add(rex1);
-      }
-    }
-
+    final List<RexNode> minusList =
+        new ArrayList<>(RelOptUtil.conjunctions(pred1));
+    minusList.removeAll(RelOptUtil.conjunctions(pred2));
     return RexUtil.composeConjunction(rexBuilder, minusList, true);
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/mutable/MutableFilter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/mutable/MutableFilter.java b/core/src/main/java/org/apache/calcite/rel/mutable/MutableFilter.java
index 787487f..ef9f8d7 100644
--- a/core/src/main/java/org/apache/calcite/rel/mutable/MutableFilter.java
+++ b/core/src/main/java/org/apache/calcite/rel/mutable/MutableFilter.java
@@ -43,13 +43,12 @@ public class MutableFilter extends MutableSingleRel {
   @Override public boolean equals(Object obj) {
     return obj == this
         || obj instanceof MutableFilter
-        && condition.toString().equals(
-            ((MutableFilter) obj).condition.toString())
+        && condition.equals(((MutableFilter) obj).condition)
         && input.equals(((MutableFilter) obj).input);
   }
 
   @Override public int hashCode() {
-    return Objects.hash(input, condition.toString());
+    return Objects.hash(input, condition);
   }
 
   @Override public StringBuilder digest(StringBuilder buf) {

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/mutable/MutableJoin.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/mutable/MutableJoin.java b/core/src/main/java/org/apache/calcite/rel/mutable/MutableJoin.java
index 09d5a1c..0a49c8c 100644
--- a/core/src/main/java/org/apache/calcite/rel/mutable/MutableJoin.java
+++ b/core/src/main/java/org/apache/calcite/rel/mutable/MutableJoin.java
@@ -66,8 +66,7 @@ public class MutableJoin extends MutableBiRel {
     return obj == this
         || obj instanceof MutableJoin
         && joinType == ((MutableJoin) obj).joinType
-        && condition.toString().equals(
-            ((MutableJoin) obj).condition.toString())
+        && condition.equals(((MutableJoin) obj).condition)
         && Objects.equals(variablesSet,
             ((MutableJoin) obj).variablesSet)
         && left.equals(((MutableJoin) obj).left)
@@ -75,8 +74,7 @@ public class MutableJoin extends MutableBiRel {
   }
 
   @Override public int hashCode() {
-    return Objects.hash(left, right,
-        condition.toString(), joinType, variablesSet);
+    return Objects.hash(left, right, condition, joinType, variablesSet);
   }
 
   @Override public StringBuilder digest(StringBuilder buf) {

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/mutable/MutableSemiJoin.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/mutable/MutableSemiJoin.java b/core/src/main/java/org/apache/calcite/rel/mutable/MutableSemiJoin.java
index c2e81b7..5806696 100644
--- a/core/src/main/java/org/apache/calcite/rel/mutable/MutableSemiJoin.java
+++ b/core/src/main/java/org/apache/calcite/rel/mutable/MutableSemiJoin.java
@@ -61,8 +61,7 @@ public class MutableSemiJoin extends MutableBiRel {
   @Override public boolean equals(Object obj) {
     return obj == this
         || obj instanceof MutableSemiJoin
-        && condition.toString().equals(
-            ((MutableSemiJoin) obj).condition.toString())
+        && condition.equals(((MutableSemiJoin) obj).condition)
         && leftKeys.equals(((MutableSemiJoin) obj).leftKeys)
         && rightKeys.equals(((MutableSemiJoin) obj).rightKeys)
         && left.equals(((MutableSemiJoin) obj).left)
@@ -70,8 +69,7 @@ public class MutableSemiJoin extends MutableBiRel {
   }
 
   @Override public int hashCode() {
-    return Objects.hash(left, right,
-        condition.toString(), leftKeys, rightKeys);
+    return Objects.hash(left, right, condition, leftKeys, rightKeys);
   }
 
   @Override public StringBuilder digest(StringBuilder buf) {

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
index 89e444a..7eedc93 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
@@ -1359,7 +1359,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
       }
 
       // Generate result rewriting
-      List<String> additionalViewExprs = new ArrayList<>();
+      final List<RexNode> additionalViewExprs = new ArrayList<>();
       Mapping rewritingMapping = null;
       RelNode result = relBuilder.push(input).build();
       // We create view expressions that will be used in a Project on top of the
@@ -1389,7 +1389,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
             RexNode targetNode = rollupNodes.get(
                 targetIdx - viewInputFieldCount - viewInputDifferenceViewFieldCount);
             // We need to rollup this expression
-            final Multimap<String, Integer> exprsLineage = ArrayListMultimap.create();
+            final Multimap<RexNode, Integer> exprsLineage = ArrayListMultimap.create();
             final ImmutableBitSet refs = RelOptUtil.InputFinder.bits(targetNode);
             for (int childTargetIdx : refs) {
               added = false;
@@ -1398,10 +1398,10 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
                 if (!n.isA(SqlKind.INPUT_REF)) {
                   continue;
                 }
-                int ref = ((RexInputRef) n).getIndex();
+                final int ref = ((RexInputRef) n).getIndex();
                 if (ref == childTargetIdx) {
                   exprsLineage.put(
-                      new RexInputRef(childTargetIdx, targetNode.getType()).toString(), k);
+                      new RexInputRef(ref, targetNode.getType()), k);
                   added = true;
                 }
               }
@@ -1414,7 +1414,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
             groupSetB.set(inputViewExprs.size());
             rewritingMapping.set(inputViewExprs.size(), i);
             additionalViewExprs.add(
-                new RexInputRef(targetIdx, targetNode.getType()).toString());
+                new RexInputRef(targetIdx, targetNode.getType()));
             // We need to create the rollup expression
             inputViewExprs.add(
                 shuttleReferences(rexBuilder, targetNode, exprsLineage));
@@ -1536,12 +1536,12 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
         topRowType = queryAggregate.getRowType();
       }
       // Available in view.
-      final Multimap<String, Integer> viewExprs = ArrayListMultimap.create();
+      final Multimap<RexNode, Integer> viewExprs = ArrayListMultimap.create();
       int numberViewExprs = 0;
       for (RexNode viewExpr : topViewProject.getChildExps()) {
-        viewExprs.put(viewExpr.toString(), numberViewExprs++);
+        viewExprs.put(viewExpr, numberViewExprs++);
       }
-      for (String additionalViewExpr : additionalViewExprs) {
+      for (RexNode additionalViewExpr : additionalViewExprs) {
         viewExprs.put(additionalViewExpr, numberViewExprs++);
       }
       final List<RexNode> rewrittenExprs = new ArrayList<>(topExprs.size());
@@ -1587,7 +1587,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
       Multimap<Integer, Integer> m = ArrayListMultimap.create();
       Map<RexTableInputRef, Set<RexTableInputRef>> equivalenceClassesMap =
           sourceEC.getEquivalenceClassesMap();
-      Multimap<String, Integer> exprsLineage = ArrayListMultimap.create();
+      Multimap<RexNode, Integer> exprsLineage = ArrayListMultimap.create();
       List<RexNode> timestampExprs = new ArrayList<>();
       for (int i = 0; i < target.getRowType().getFieldCount(); i++) {
         Set<RexNode> s = mq.getExpressionLineage(target, rexBuilder.makeInputRef(target, i));
@@ -1604,7 +1604,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
             simplified,
             tableMapping.inverse(),
             equivalenceClassesMap);
-        exprsLineage.put(expr.toString(), i);
+        exprsLineage.put(expr, i);
         SqlTypeName sqlTypeName = expr.getType().getSqlTypeName();
         if (sqlTypeName == SqlTypeName.TIMESTAMP
             || sqlTypeName == SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE) {
@@ -1633,7 +1633,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
             // can try to find a match
             final RexNode simplified =
                 simplify.simplifyUnknownAsFalse(ceilExpr);
-            exprsLineage.put(simplified.toString(),
+            exprsLineage.put(simplified,
                 target.getRowType().getFieldCount() + additionalExprs.size() - 1);
           }
           // FLOOR
@@ -1651,7 +1651,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
             // can try to find a match
             final RexNode simplified =
                 simplify.simplifyUnknownAsFalse(floorExpr);
-            exprsLineage.put(simplified.toString(),
+            exprsLineage.put(simplified,
                 target.getRowType().getFieldCount() + additionalExprs.size() - 1);
           }
         }
@@ -1670,7 +1670,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
         final RexNode simplified = simplify.simplifyUnknownAsFalse(e);
         RexNode targetExpr = RexUtil.swapColumnReferences(rexBuilder,
             simplified, equivalenceClassesMap);
-        Collection<Integer> c = exprsLineage.get(targetExpr.toString());
+        final Collection<Integer> c = exprsLineage.get(targetExpr);
         if (!c.isEmpty()) {
           for (Integer j : c) {
             m.put(i, j);
@@ -2010,8 +2010,8 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
                 parentTRef.getTable().getRowType().getFieldList().get(uniqueKeyPos).getType());
             if (!foreignKeyColumnType.isNullable()
                 && sourceEC.getEquivalenceClassesMap().containsKey(uniqueKeyColumnRef)
-                && sourceEC.getEquivalenceClassesMap().get(uniqueKeyColumnRef).contains(
-                    foreignKeyColumnRef)) {
+                && sourceEC.getEquivalenceClassesMap().get(uniqueKeyColumnRef)
+                    .contains(foreignKeyColumnRef)) {
               equiColumns.put(foreignKeyColumnRef, uniqueKeyColumnRef);
             } else {
               canBeRewritten = false;
@@ -2310,8 +2310,8 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
       BiMap<RelTableRef, RelTableRef> tableMapping,
       EquivalenceClasses ec,
       List<RexNode> nodeExprs) {
-    Map<String, Integer> exprsLineage = new HashMap<>();
-    Map<String, Integer> exprsLineageLosslessCasts = new HashMap<>();
+    final Map<RexNode, Integer> exprsLineage = new HashMap<>();
+    final Map<RexNode, Integer> exprsLineageLosslessCasts = new HashMap<>();
     for (int i = 0; i < nodeExprs.size(); i++) {
       final Set<RexNode> s = mq.getExpressionLineage(node, nodeExprs.get(i));
       if (s == null) {
@@ -2325,12 +2325,12 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
       // mapping, then we take first element from the corresponding equivalence class
       final RexNode e = RexUtil.swapTableColumnReferences(rexBuilder,
           s.iterator().next(), tableMapping, ec.getEquivalenceClassesMap());
-      exprsLineage.put(e.toString(), i);
+      exprsLineage.put(e, i);
       if (RexUtil.isLosslessCast(e)) {
-        exprsLineageLosslessCasts.put(((RexCall) e).getOperands().get(0).toString(), i);
+        exprsLineageLosslessCasts.put(((RexCall) e).getOperands().get(0), i);
       }
     }
-    return NodeLineage.of(exprsLineage, exprsLineageLosslessCasts);
+    return new NodeLineage(exprsLineage, exprsLineageLosslessCasts);
   }
 
   /**
@@ -2344,8 +2344,8 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
       BiMap<RelTableRef, RelTableRef> tableMapping,
       EquivalenceClasses ec,
       List<RexNode> nodeExprs) {
-    Map<String, Integer> exprsLineage = new HashMap<>();
-    Map<String, Integer> exprsLineageLosslessCasts = new HashMap<>();
+    final Map<RexNode, Integer> exprsLineage = new HashMap<>();
+    final Map<RexNode, Integer> exprsLineageLosslessCasts = new HashMap<>();
     for (int i = 0; i < nodeExprs.size(); i++) {
       final Set<RexNode> s = mq.getExpressionLineage(node, nodeExprs.get(i));
       if (s == null) {
@@ -2354,17 +2354,17 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
       }
       // We only support project - filter - join, thus it should map to
       // a single expression
-      assert s.size() == 1;
+      final RexNode node2 = Iterables.getOnlyElement(s);
       // Rewrite expr. First we take first element from the corresponding equivalence class,
       // then we swap the table references following the table mapping
-      final RexNode e = RexUtil.swapColumnTableReferences(
-          rexBuilder, s.iterator().next(), ec.getEquivalenceClassesMap(), tableMapping);
-      exprsLineage.put(e.toString(), i);
+      final RexNode e = RexUtil.swapColumnTableReferences(rexBuilder, node2,
+          ec.getEquivalenceClassesMap(), tableMapping);
+      exprsLineage.put(e, i);
       if (RexUtil.isLosslessCast(e)) {
-        exprsLineageLosslessCasts.put(((RexCall) e).getOperands().get(0).toString(), i);
+        exprsLineageLosslessCasts.put(((RexCall) e).getOperands().get(0), i);
       }
     }
-    return NodeLineage.of(exprsLineage, exprsLineageLosslessCasts);
+    return new NodeLineage(exprsLineage, exprsLineageLosslessCasts);
   }
 
   /**
@@ -2392,12 +2392,12 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
           }
 
           private RexNode replace(RexNode e) {
-            Integer pos = nodeLineage.exprsLineage.get(e.toString());
+            Integer pos = nodeLineage.exprsLineage.get(e);
             if (pos != null) {
               // Found it
               return rexBuilder.makeInputRef(node, pos);
             }
-            pos = nodeLineage.exprsLineageLosslessCasts.get(e.toString());
+            pos = nodeLineage.exprsLineageLosslessCasts.get(e);
             if (pos != null) {
               // Found it
               return rexBuilder.makeCast(
@@ -2436,29 +2436,29 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
   }
 
   /**
-   * Replaces all the possible subexpressions by input references
+   * Replaces all the possible sub-expressions by input references
    * to the input node.
    */
   private static RexNode shuttleReferences(final RexBuilder rexBuilder,
-      final RexNode expr, final Multimap<String, Integer> exprsLineage) {
+      final RexNode expr, final Multimap<RexNode, Integer> exprsLineage) {
     return shuttleReferences(rexBuilder, expr,
         exprsLineage, null, null);
   }
 
   /**
-   * Replaces all the possible subexpressions by input references
+   * Replaces all the possible sub-expressions by input references
    * to the input node. If available, it uses the rewriting mapping
    * to change the position to reference. Takes the reference type
    * from the input node.
    */
   private static RexNode shuttleReferences(final RexBuilder rexBuilder,
-      final RexNode expr, final Multimap<String, Integer> exprsLineage,
+      final RexNode expr, final Multimap<RexNode, Integer> exprsLineage,
       final RelNode node, final Mapping rewritingMapping) {
     try {
       RexShuttle visitor =
           new RexShuttle() {
             @Override public RexNode visitTableInputRef(RexTableInputRef ref) {
-              Collection<Integer> c = exprsLineage.get(ref.toString());
+              Collection<Integer> c = exprsLineage.get(ref);
               if (c.isEmpty()) {
                 // Cannot map expression
                 throw Util.FoundOne.NULL;
@@ -2478,7 +2478,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
             }
 
             @Override public RexNode visitInputRef(RexInputRef inputRef) {
-              Collection<Integer> c = exprsLineage.get(inputRef.toString());
+              Collection<Integer> c = exprsLineage.get(inputRef);
               if (c.isEmpty()) {
                 // Cannot map expression
                 throw Util.FoundOne.NULL;
@@ -2498,7 +2498,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
             }
 
             @Override public RexNode visitCall(final RexCall call) {
-              Collection<Integer> c = exprsLineage.get(call.toString());
+              Collection<Integer> c = exprsLineage.get(call);
               if (c.isEmpty()) {
                 // Cannot map expression
                 return super.visitCall(call);
@@ -2612,28 +2612,21 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule {
     }
   }
 
-  /**
-   * Class to encapsulate expression lineage details
-   */
+  /** Expression lineage details. */
   private static class NodeLineage {
-    private final Map<String, Integer> exprsLineage;
-    private final Map<String, Integer> exprsLineageLosslessCasts;
+    private final Map<RexNode, Integer> exprsLineage;
+    private final Map<RexNode, Integer> exprsLineageLosslessCasts;
 
-    private NodeLineage(Map<String, Integer> exprsLineage,
-        Map<String, Integer> exprsLineageLosslessCasts) {
-      this.exprsLineage = Collections.unmodifiableMap(exprsLineage);
-      this.exprsLineageLosslessCasts = Collections.unmodifiableMap(exprsLineageLosslessCasts);
-    }
-
-    protected static NodeLineage of(
-        Map<String, Integer> exprsLineage, Map<String, Integer> exprsLineageLosslessCasts) {
-      return new NodeLineage(exprsLineage, exprsLineageLosslessCasts);
+    private NodeLineage(Map<RexNode, Integer> exprsLineage,
+        Map<RexNode, Integer> exprsLineageLosslessCasts) {
+      this.exprsLineage = ImmutableMap.copyOf(exprsLineage);
+      this.exprsLineageLosslessCasts =
+          ImmutableMap.copyOf(exprsLineageLosslessCasts);
     }
   }
 
   /** Edge for graph */
   private static class Edge extends DefaultEdge {
-
     final Multimap<RexTableInputRef, RexTableInputRef> equiColumns =
         ArrayListMultimap.create();
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/rules/DateRangeRules.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/DateRangeRules.java b/core/src/main/java/org/apache/calcite/rel/rules/DateRangeRules.java
index e9a968e..8d2337f 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/DateRangeRules.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/DateRangeRules.java
@@ -158,7 +158,7 @@ public abstract class DateRangeRules {
       timeUnits = ImmutableSortedSet.<TimeUnitRange>naturalOrder()
           .addAll(timeUnits).add(TimeUnitRange.YEAR).build();
     }
-    final Map<String, RangeSet<Calendar>> operandRanges = new HashMap<>();
+    final Map<RexNode, RangeSet<Calendar>> operandRanges = new HashMap<>();
     for (TimeUnitRange timeUnit : timeUnits) {
       e = e.accept(
           new ExtractShuttle(rexBuilder, timeUnit, operandRanges, timeUnits,
@@ -183,7 +183,7 @@ public abstract class DateRangeRules {
           .unwrap(CalciteConnectionConfig.class).timeZone();
       final RexNode condition =
           replaceTimeUnits(rexBuilder, filter.getCondition(), timeZone);
-      if (RexUtil.eq(condition, filter.getCondition())) {
+      if (condition.equals(filter.getCondition())) {
         return;
       }
       final RelBuilder relBuilder =
@@ -238,14 +238,14 @@ public abstract class DateRangeRules {
   static class ExtractShuttle extends RexShuttle {
     private final RexBuilder rexBuilder;
     private final TimeUnitRange timeUnit;
-    private final Map<String, RangeSet<Calendar>> operandRanges;
+    private final Map<RexNode, RangeSet<Calendar>> operandRanges;
     private final Deque<RexCall> calls = new ArrayDeque<>();
     private final ImmutableSortedSet<TimeUnitRange> timeUnitRanges;
     private final String timeZone;
 
     @VisibleForTesting
     ExtractShuttle(RexBuilder rexBuilder, TimeUnitRange timeUnit,
-        Map<String, RangeSet<Calendar>> operandRanges,
+        Map<RexNode, RangeSet<Calendar>> operandRanges,
         ImmutableSortedSet<TimeUnitRange> timeUnitRanges, String timeZone) {
       this.rexBuilder = Objects.requireNonNull(rexBuilder);
       this.timeUnit = Objects.requireNonNull(timeUnit);
@@ -332,8 +332,7 @@ public abstract class DateRangeRules {
       if (timeUnit == TimeUnitRange.YEAR) {
         return true;
       }
-      final RangeSet<Calendar> calendarRangeSet =
-          operandRanges.get(operand.toString());
+      final RangeSet<Calendar> calendarRangeSet = operandRanges.get(operand);
       if (calendarRangeSet == null || calendarRangeSet.isEmpty()) {
         return false;
       }
@@ -361,7 +360,7 @@ public abstract class DateRangeRules {
           //noinspection unchecked
           return (List<RexNode>) exprs;
         }
-        final Map<String, RangeSet<Calendar>> save =
+        final Map<RexNode, RangeSet<Calendar>> save =
             ImmutableMap.copyOf(operandRanges);
         final ImmutableList.Builder<RexNode> clonedOperands =
             ImmutableList.builder();
@@ -400,7 +399,7 @@ public abstract class DateRangeRules {
 
     RexNode compareExtract(SqlKind comparison, RexNode operand,
         RexLiteral literal) {
-      RangeSet<Calendar> rangeSet = operandRanges.get(operand.toString());
+      RangeSet<Calendar> rangeSet = operandRanges.get(operand);
       if (rangeSet == null) {
         rangeSet = ImmutableRangeSet.<Calendar>of().complement();
       }
@@ -439,7 +438,7 @@ public abstract class DateRangeRules {
       }
       // Intersect old range set with new.
       s2.removeAll(rangeSet.complement());
-      operandRanges.put(operand.toString(), ImmutableRangeSet.copyOf(s2));
+      operandRanges.put(operand, ImmutableRangeSet.copyOf(s2));
       final List<RexNode> nodes = new ArrayList<>();
       for (Range<Calendar> r : s2.asRanges()) {
         nodes.add(toRex(operand, r));
@@ -569,7 +568,7 @@ public abstract class DateRangeRules {
 
     private RexNode compareFloorCeil(SqlKind comparison, RexNode operand,
         RexLiteral timeLiteral, TimeUnitRange timeUnit, boolean floor) {
-      RangeSet<Calendar> rangeSet = operandRanges.get(operand.toString());
+      RangeSet<Calendar> rangeSet = operandRanges.get(operand);
       if (rangeSet == null) {
         rangeSet = ImmutableRangeSet.<Calendar>of().complement();
       }
@@ -581,7 +580,7 @@ public abstract class DateRangeRules {
       s2.add(range);
       // Intersect old range set with new.
       s2.removeAll(rangeSet.complement());
-      operandRanges.put(operand.toString(), ImmutableRangeSet.copyOf(s2));
+      operandRanges.put(operand, ImmutableRangeSet.copyOf(s2));
       if (range.isEmpty()) {
         return rexBuilder.makeLiteral(false);
       }

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/rules/JoinPushExpressionsRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/JoinPushExpressionsRule.java b/core/src/main/java/org/apache/calcite/rel/rules/JoinPushExpressionsRule.java
index 4fb33e5..923809f 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/JoinPushExpressionsRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/JoinPushExpressionsRule.java
@@ -61,7 +61,7 @@ public class JoinPushExpressionsRule extends RelOptRule {
     // If the join is the same, we bail out
     if (newJoin instanceof Join) {
       final RexNode newCondition = ((Join) newJoin).getCondition();
-      if (join.getCondition().toString().equals(newCondition.toString())) {
+      if (join.getCondition().equals(newCondition)) {
         return;
       }
     }

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java b/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java
index e0d6b71..e34a4a8 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java
@@ -700,11 +700,15 @@ public class PushProjector {
         final ImmutableBitSet exprArgs = RelOptUtil.InputFinder.bits(call);
         if (exprArgs.cardinality() > 0) {
           if (leftFields.contains(exprArgs) && isStrong(exprArgs, call)) {
-            addExpr(preserveLeft, call);
+            if (!preserveLeft.contains(call)) {
+              preserveLeft.add(call);
+            }
             return true;
           } else if (rightFields.contains(exprArgs) && isStrong(exprArgs, call)) {
             assert preserveRight != null;
-            addExpr(preserveRight, call);
+            if (!preserveRight.contains(call)) {
+              preserveRight.add(call);
+            }
             return true;
           }
         }
@@ -721,22 +725,6 @@ public class PushProjector {
       return null;
     }
 
-    /**
-     * Adds an expression to a list if the same expression isn't already in
-     * the list. Expressions are identical if their digests are the same.
-     *
-     * @param exprList current list of expressions
-     * @param newExpr  new expression to be added
-     */
-    private void addExpr(List<RexNode> exprList, RexNode newExpr) {
-      String newExprString = newExpr.toString();
-      for (RexNode expr : exprList) {
-        if (newExprString.compareTo(expr.toString()) == 0) {
-          return;
-        }
-      }
-      exprList.add(newExpr);
-    }
   }
 
   /**
@@ -804,13 +792,13 @@ public class PushProjector {
         int adjust1,
         List<RexNode> rexList2,
         int adjust2) {
-      int match = findExprInList(rex, rexList1);
+      int match = rexList1.indexOf(rex);
       if (match >= 0) {
         return match + adjust1;
       }
 
       if (rexList2 != null) {
-        match = findExprInList(rex, rexList2);
+        match = rexList2.indexOf(rex);
         if (match >= 0) {
           return match + adjust2;
         }
@@ -818,17 +806,6 @@ public class PushProjector {
 
       return -1;
     }
-
-    private int findExprInList(RexNode rex, List<RexNode> rexList) {
-      int match = 0;
-      for (RexNode rexElement : rexList) {
-        if (rexElement.toString().compareTo(rex.toString()) == 0) {
-          return match;
-        }
-        match++;
-      }
-      return -1;
-    }
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/rules/ReduceDecimalsRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ReduceDecimalsRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ReduceDecimalsRule.java
index 1242d51..c8dd509 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/ReduceDecimalsRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/ReduceDecimalsRule.java
@@ -122,8 +122,8 @@ public class ReduceDecimalsRule extends RelOptRule {
    * longs.
    */
   public class DecimalShuttle extends RexShuttle {
-    private final Map<Pair<String, String>, RexNode> irreducible;
-    private final Map<Pair<String, String>, RexNode> results;
+    private final Map<Pair<RexNode, String>, RexNode> irreducible;
+    private final Map<Pair<RexNode, String>, RexNode> results;
     private final ExpanderMap expanderMap;
 
     public DecimalShuttle(RexBuilder rexBuilder) {
@@ -173,7 +173,7 @@ public class ReduceDecimalsRule extends RelOptRule {
      * Registers node so it will not be computed again
      */
     private void register(RexNode node, RexNode reducedNode) {
-      Pair<String, String> key = RexUtil.makeKey(node);
+      Pair<RexNode, String> key = RexUtil.makeKey(node);
       if (node == reducedNode) {
         irreducible.put(key, reducedNode);
       } else {
@@ -185,7 +185,7 @@ public class ReduceDecimalsRule extends RelOptRule {
      * Lookup registered node
      */
     private RexNode lookup(RexNode node) {
-      Pair<String, String> key = RexUtil.makeKey(node);
+      Pair<RexNode, String> key = RexUtil.makeKey(node);
       if (irreducible.get(key) != null) {
         return node;
       }

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
index 741d083..50823f4 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
@@ -547,7 +547,7 @@ public abstract class ReduceExpressionsRule extends RelOptRule {
     boolean simplified = false;
     for (int i = 0; i < expList.size(); i++) {
       RexNode expr2 = simplifier.apply(expList.get(i));
-      if (!expr2.toString().equals(expList.get(i).toString())) {
+      if (!expr2.equals(expList.get(i))) {
         expList.set(i, expr2);
         simplified = true;
       }

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java b/core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java
index b993184..9eebeb3 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java
@@ -599,7 +599,7 @@ public abstract class SubQueryRemoveRule extends RelOptRule {
     }
 
     @Override public RexNode visitSubQuery(RexSubQuery subQuery) {
-      return RexUtil.eq(subQuery, this.subQuery) ? replacement : subQuery;
+      return subQuery.equals(this.subQuery) ? replacement : subQuery;
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/LogicVisitor.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/LogicVisitor.java b/core/src/main/java/org/apache/calcite/rex/LogicVisitor.java
index f632e94..7d4f494 100644
--- a/core/src/main/java/org/apache/calcite/rex/LogicVisitor.java
+++ b/core/src/main/java/org/apache/calcite/rex/LogicVisitor.java
@@ -115,7 +115,7 @@ public class LogicVisitor implements RexBiVisitor<Logic, Logic> {
   }
 
   private Logic end(RexNode node, Logic arg) {
-    if (RexUtil.eq(node, seek)) {
+    if (node.equals(seek)) {
       logicCollection.add(arg);
     }
     return arg;

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/RexCall.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexCall.java b/core/src/main/java/org/apache/calcite/rex/RexCall.java
index d2c4ac2..26b095d 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexCall.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexCall.java
@@ -26,6 +26,7 @@ import com.google.common.collect.ImmutableList;
 
 import java.util.List;
 import java.util.Objects;
+import javax.annotation.Nonnull;
 
 /**
  * An expression formed by a call to an operator with zero or more expressions
@@ -65,8 +66,8 @@ public class RexCall extends RexNode {
 
   //~ Methods ----------------------------------------------------------------
 
-  protected String computeDigest(boolean withType) {
-    StringBuilder sb = new StringBuilder(op.getName());
+  protected @Nonnull String computeDigest(boolean withType) {
+    final StringBuilder sb = new StringBuilder(op.getName());
     if ((operands.size() == 0)
         && (op.getSyntax() == SqlSyntax.FUNCTION_ID)) {
       // Don't print params for empty arg list. For example, we want
@@ -77,8 +78,7 @@ public class RexCall extends RexNode {
         if (i > 0) {
           sb.append(", ");
         }
-        RexNode operand = operands.get(i);
-        sb.append(operand.toString());
+        sb.append(operands.get(i));
       }
       sb.append(")");
     }
@@ -92,13 +92,13 @@ public class RexCall extends RexNode {
     return sb.toString();
   }
 
-  public String toString() {
+  @Override public final @Nonnull String toString() {
     // This data race is intentional
     String localDigest = digest;
     if (localDigest == null) {
       localDigest = computeDigest(
           isA(SqlKind.CAST) || isA(SqlKind.NEW_SPECIFICATION));
-      digest = localDigest;
+      digest = Objects.requireNonNull(localDigest);
     }
     return localDigest;
   }
@@ -173,6 +173,16 @@ public class RexCall extends RexNode {
   public RexCall clone(RelDataType type, List<RexNode> operands) {
     return new RexCall(type, op, operands);
   }
+
+  @Override public boolean equals(Object obj) {
+    return obj == this
+        || obj instanceof RexCall
+        && toString().equals(obj.toString());
+  }
+
+  @Override public int hashCode() {
+    return toString().hashCode();
+  }
 }
 
 // End RexCall.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/RexCorrelVariable.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexCorrelVariable.java b/core/src/main/java/org/apache/calcite/rex/RexCorrelVariable.java
index 750a97b..a16b30d 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexCorrelVariable.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexCorrelVariable.java
@@ -54,6 +54,18 @@ public class RexCorrelVariable extends RexVariable {
   @Override public SqlKind getKind() {
     return SqlKind.CORREL_VARIABLE;
   }
+
+  @Override public boolean equals(Object obj) {
+    return this == obj
+        || obj instanceof RexCorrelVariable
+        && digest.equals(((RexCorrelVariable) obj).digest)
+        && type.equals(((RexCorrelVariable) obj).type)
+        && id.equals(((RexCorrelVariable) obj).id);
+  }
+
+  @Override public int hashCode() {
+    return Objects.hash(digest, type, id);
+  }
 }
 
 // End RexCorrelVariable.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/RexDynamicParam.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexDynamicParam.java b/core/src/main/java/org/apache/calcite/rex/RexDynamicParam.java
index 6bbd692..7e92d4a 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexDynamicParam.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexDynamicParam.java
@@ -19,6 +19,8 @@ package org.apache.calcite.rex;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.sql.SqlKind;
 
+import java.util.Objects;
+
 /**
  * Dynamic parameter reference in a row-expression.
  */
@@ -59,6 +61,18 @@ public class RexDynamicParam extends RexVariable {
   public <R, P> R accept(RexBiVisitor<R, P> visitor, P arg) {
     return visitor.visitDynamicParam(this, arg);
   }
+
+  @Override public boolean equals(Object obj) {
+    return this == obj
+        || obj instanceof RexDynamicParam
+        && digest.equals(((RexDynamicParam) obj).digest)
+        && type.equals(((RexDynamicParam) obj).type)
+        && index == ((RexDynamicParam) obj).index;
+  }
+
+  @Override public int hashCode() {
+    return Objects.hash(digest, type, index);
+  }
 }
 
 // End RexDynamicParam.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/RexNode.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexNode.java b/core/src/main/java/org/apache/calcite/rex/RexNode.java
index 95df460..ca382c0 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexNode.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexNode.java
@@ -96,6 +96,19 @@ public abstract class RexNode {
    * {@link RexBiVisitor#visitInputRef(RexInputRef, Object)} visitXxx} method.
    */
   public abstract <R, P> R accept(RexBiVisitor<R, P> visitor, P arg);
+
+  /** {@inheritDoc}
+   *
+   * <p>Every node must implement {@link #equals} based on its content
+   */
+  @Override public abstract boolean equals(Object obj);
+
+  /** {@inheritDoc}
+   *
+   * <p>Every node must implement {@link #hashCode} consistent with
+   * {@link #equals}
+   */
+  @Override public abstract int hashCode();
 }
 
 // End RexNode.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/RexOver.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexOver.java b/core/src/main/java/org/apache/calcite/rex/RexOver.java
index fe7f7d9..7737ccb 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexOver.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexOver.java
@@ -26,6 +26,7 @@ import com.google.common.base.Preconditions;
 
 import java.util.List;
 import java.util.Objects;
+import javax.annotation.Nonnull;
 
 /**
  * Call to an aggregate function over a window.
@@ -88,7 +89,7 @@ public class RexOver extends RexCall {
     return distinct;
   }
 
-  @Override protected String computeDigest(boolean withType) {
+  @Override protected @Nonnull String computeDigest(boolean withType) {
     final StringBuilder sb = new StringBuilder(op.getName());
     sb.append("(");
     if (distinct) {
@@ -98,8 +99,7 @@ public class RexOver extends RexCall {
       if (i > 0) {
         sb.append(", ");
       }
-      RexNode operand = operands.get(i);
-      sb.append(operand.toString());
+      sb.append(operands.get(i));
     }
     sb.append(")");
     if (withType) {

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/RexProgramBuilder.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexProgramBuilder.java b/core/src/main/java/org/apache/calcite/rex/RexProgramBuilder.java
index f871b9d..9feb711 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexProgramBuilder.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexProgramBuilder.java
@@ -43,7 +43,7 @@ public class RexProgramBuilder {
   private final RexBuilder rexBuilder;
   private final RelDataType inputRowType;
   private final List<RexNode> exprList = new ArrayList<>();
-  private final Map<Pair<String, String>, RexLocalRef> exprMap =
+  private final Map<Pair<RexNode, String>, RexLocalRef> exprMap =
       new HashMap<>();
   private final List<RexLocalRef> localRefList = new ArrayList<>();
   private final List<RexLocalRef> projectRefList = new ArrayList<>();
@@ -329,7 +329,7 @@ public class RexProgramBuilder {
     expr = simplify.simplifyPreservingType(expr);
 
     RexLocalRef ref;
-    final Pair<String, String> key;
+    final Pair<RexNode, String> key;
     if (expr instanceof RexLocalRef) {
       key = null;
       ref = (RexLocalRef) expr;

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/RexRangeRef.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexRangeRef.java b/core/src/main/java/org/apache/calcite/rex/RexRangeRef.java
index 04aa277..97d5e46 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexRangeRef.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexRangeRef.java
@@ -18,6 +18,8 @@ package org.apache.calcite.rex;
 
 import org.apache.calcite.rel.type.RelDataType;
 
+import java.util.Objects;
+
 /**
  * Reference to a range of columns.
  *
@@ -74,6 +76,17 @@ public class RexRangeRef extends RexNode {
   public <R, P> R accept(RexBiVisitor<R, P> visitor, P arg) {
     return visitor.visitRangeRef(this, arg);
   }
+
+  @Override public boolean equals(Object obj) {
+    return this == obj
+        || obj instanceof RexRangeRef
+        && type.equals(((RexRangeRef) obj).type)
+        && offset == ((RexRangeRef) obj).offset;
+  }
+
+  @Override public int hashCode() {
+    return Objects.hash(type, offset);
+  }
 }
 
 // End RexRangeRef.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
index a8e95e9..f78a90c 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
@@ -300,7 +300,7 @@ public class RexSimplify {
     // Simplify "x <op> x"
     final RexNode o0 = operands.get(0);
     final RexNode o1 = operands.get(1);
-    if (RexUtil.eq(o0, o1)
+    if (o0.equals(o1)
         && (unknownAs == FALSE
             || (!o0.getType().isNullable()
                 && !o1.getType().isNullable()))) {
@@ -514,7 +514,7 @@ public class RexSimplify {
 
     for (RexNode p : predicates.pulledUpPredicates) {
       IsPredicate pred = IsPredicate.of(p);
-      if (pred == null || !a.toString().equals(pred.ref.toString())) {
+      if (pred == null || !a.equals(pred.ref)) {
         continue;
       }
       if (kind == pred.kind) {
@@ -649,12 +649,12 @@ public class RexSimplify {
   }
 
   private RexNode simplifyCoalesce(RexCall call) {
-    final Set<String> digests = new HashSet<>();
+    final Set<RexNode> operandSet = new HashSet<>();
     final List<RexNode> operands = new ArrayList<>();
     for (RexNode operand : call.getOperands()) {
       operand = simplify(operand, UNKNOWN);
       if (!RexUtil.isNull(operand)
-          && digests.add(operand.toString())) {
+          && operandSet.add(operand)) {
         operands.add(operand);
       }
       if (!operand.getType().isNullable()) {
@@ -699,7 +699,7 @@ public class RexSimplify {
 
       // create new branch
       if (lastBranch != null) {
-        if (lastBranch.value.toString().equals(newValue.toString())
+        if (lastBranch.value.equals(newValue)
             && isSafeExpression(newCond)) {
           // in this case, last branch and new branch have the same conclusion,
           // hence we create a new composite condition and we do not add it to
@@ -809,7 +809,7 @@ public class RexSimplify {
     }
 
     @Override public String toString() {
-      return new StringBuilder(cond.toString()).append(" => ").append(value).toString();
+      return cond + " => " + value;
     }
 
     /** Given "CASE WHEN p1 THEN v1 ... ELSE e END"
@@ -1120,14 +1120,15 @@ public class RexSimplify {
       return simplify(terms.get(0), FALSE);
     }
     // Try to simplify the expression
-    final Multimap<String, Pair<String, RexNode>> equalityTerms = ArrayListMultimap.create();
-    final Map<String, Pair<Range<C>, List<RexNode>>> rangeTerms =
+    final Multimap<RexNode, Pair<RexNode, RexNode>> equalityTerms =
+        ArrayListMultimap.create();
+    final Map<RexNode, Pair<Range<C>, List<RexNode>>> rangeTerms =
         new HashMap<>();
-    final Map<String, String> equalityConstantTerms = new HashMap<>();
-    final Set<String> negatedTerms = new HashSet<>();
-    final Set<String> nullOperands = new HashSet<>();
+    final Map<RexNode, RexLiteral> equalityConstantTerms = new HashMap<>();
+    final Set<RexNode> negatedTerms = new HashSet<>();
+    final Set<RexNode> nullOperands = new HashSet<>();
     final Set<RexNode> notNullOperands = new LinkedHashSet<>();
-    final Set<String> comparedOperands = new HashSet<>();
+    final Set<RexNode> comparedOperands = new HashSet<>();
 
     // Add the predicates from the source to the range terms.
     for (RexNode predicate : predicates.pulledUpPredicates) {
@@ -1173,19 +1174,19 @@ public class RexSimplify {
       case LESS_THAN_OR_EQUAL:
       case GREATER_THAN_OR_EQUAL:
         RexCall call = (RexCall) term;
-        RexNode left = call.getOperands().get(0);
-        comparedOperands.add(left.toString());
+        final RexNode left = call.getOperands().get(0);
+        comparedOperands.add(left);
         // if it is a cast, we include the inner reference
         if (left.getKind() == SqlKind.CAST) {
           RexCall leftCast = (RexCall) left;
-          comparedOperands.add(leftCast.getOperands().get(0).toString());
+          comparedOperands.add(leftCast.getOperands().get(0));
         }
-        RexNode right = call.getOperands().get(1);
-        comparedOperands.add(right.toString());
+        final RexNode right = call.getOperands().get(1);
+        comparedOperands.add(right);
         // if it is a cast, we include the inner reference
         if (right.getKind() == SqlKind.CAST) {
           RexCall rightCast = (RexCall) right;
-          comparedOperands.add(rightCast.getOperands().get(0).toString());
+          comparedOperands.add(rightCast.getOperands().get(0));
         }
         final Comparison comparison = Comparison.of(term);
         // Check for comparison with null values
@@ -1198,15 +1199,15 @@ public class RexSimplify {
         // and hence it can be evaluated to FALSE
         if (term.getKind() == SqlKind.EQUALS) {
           if (comparison != null) {
-            final String literal = comparison.literal.toString();
-            final String prevLiteral =
-                equalityConstantTerms.put(comparison.ref.toString(), literal);
+            final RexLiteral literal = comparison.literal;
+            final RexLiteral prevLiteral =
+                equalityConstantTerms.put(comparison.ref, literal);
             if (prevLiteral != null && !literal.equals(prevLiteral)) {
               return rexBuilder.makeLiteral(false);
             }
           } else if (RexUtil.isReferenceOrAccess(left, true)
               && RexUtil.isReferenceOrAccess(right, true)) {
-            equalityTerms.put(left.toString(), Pair.of(right.toString(), term));
+            equalityTerms.put(left, Pair.of(right, term));
           }
         }
         // Assume the expression a > 5 is part of a Filter condition.
@@ -1216,10 +1217,10 @@ public class RexSimplify {
         // Observe that for creating the inverted term we invert the list of operands.
         RexNode negatedTerm = RexUtil.negate(rexBuilder, call);
         if (negatedTerm != null) {
-          negatedTerms.add(negatedTerm.toString());
+          negatedTerms.add(negatedTerm);
           RexNode invertNegatedTerm = RexUtil.invert(rexBuilder, (RexCall) negatedTerm);
           if (invertNegatedTerm != null) {
-            negatedTerms.add(invertNegatedTerm.toString());
+            negatedTerms.add(invertNegatedTerm);
           }
         }
         // Remove terms that are implied by predicates on the input,
@@ -1243,10 +1244,10 @@ public class RexSimplify {
         }
         break;
       case IN:
-        comparedOperands.add(((RexCall) term).operands.get(0).toString());
+        comparedOperands.add(((RexCall) term).operands.get(0));
         break;
       case BETWEEN:
-        comparedOperands.add(((RexCall) term).operands.get(1).toString());
+        comparedOperands.add(((RexCall) term).operands.get(1));
         break;
       case IS_NOT_NULL:
         notNullOperands.add(((RexCall) term).getOperands().get(0));
@@ -1254,7 +1255,7 @@ public class RexSimplify {
         --i;
         break;
       case IS_NULL:
-        nullOperands.add(((RexCall) term).getOperands().get(0).toString());
+        nullOperands.add(((RexCall) term).getOperands().get(0));
       }
     }
     // If one column should be null and is in a comparison predicate,
@@ -1266,14 +1267,14 @@ public class RexSimplify {
     // Check for equality of two refs wrt equality with constants
     // Example #1. x=5 AND y=5 AND x=y : x=5 AND y=5
     // Example #2. x=5 AND y=6 AND x=y - not satisfiable
-    for (String ref1 : equalityTerms.keySet()) {
-      final String literal1 = equalityConstantTerms.get(ref1);
+    for (RexNode ref1 : equalityTerms.keySet()) {
+      final RexLiteral literal1 = equalityConstantTerms.get(ref1);
       if (literal1 == null) {
         continue;
       }
-      Collection<Pair<String, RexNode>> references = equalityTerms.get(ref1);
-      for (Pair<String, RexNode> ref2 : references) {
-        final String literal2 = equalityConstantTerms.get(ref2.left);
+      Collection<Pair<RexNode, RexNode>> references = equalityTerms.get(ref1);
+      for (Pair<RexNode, RexNode> ref2 : references) {
+        final RexLiteral literal2 = equalityConstantTerms.get(ref2.left);
         if (literal2 == null) {
           continue;
         }
@@ -1291,7 +1292,7 @@ public class RexSimplify {
     //
     // Example. IS NOT NULL(x) AND x < 5  : x < 5
     for (RexNode operand : notNullOperands) {
-      if (!comparedOperands.contains(operand.toString())) {
+      if (!comparedOperands.contains(operand)) {
         terms.add(
             rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, operand));
       }
@@ -1303,12 +1304,12 @@ public class RexSimplify {
     // Example #1. x AND y AND z AND NOT (x AND y)  - not satisfiable
     // Example #2. x AND y AND NOT (x AND y)        - not satisfiable
     // Example #3. x AND y AND NOT (x AND y AND z)  - may be satisfiable
-    final Set<String> termsSet = new HashSet<>(RexUtil.strings(terms));
+    final Set<RexNode> termsSet = new HashSet<>(terms);
     for (RexNode notDisjunction : notTerms) {
       if (!RexUtil.isDeterministic(notDisjunction)) {
         continue;
       }
-      final List<String> terms2Set = RexUtil.strings(RelOptUtil.conjunctions(notDisjunction));
+      final List<RexNode> terms2Set = RelOptUtil.conjunctions(notDisjunction);
       if (termsSet.containsAll(terms2Set)) {
         return rexBuilder.makeLiteral(false);
       }
@@ -1320,7 +1321,7 @@ public class RexSimplify {
       terms.add(simplify(call, FALSE));
     }
     // The negated terms: only deterministic expressions
-    for (String negatedTerm : negatedTerms) {
+    for (RexNode negatedTerm : negatedTerms) {
       if (termsSet.contains(negatedTerm)) {
         return rexBuilder.makeLiteral(false);
       }
@@ -1673,11 +1674,11 @@ public class RexSimplify {
 
   private static <C extends Comparable<C>> RexNode processRange(
       RexBuilder rexBuilder, List<RexNode> terms,
-      Map<String, Pair<Range<C>, List<RexNode>>> rangeTerms, RexNode term,
+      Map<RexNode, Pair<Range<C>, List<RexNode>>> rangeTerms, RexNode term,
       RexNode ref, C v0, SqlKind comparison) {
-    Pair<Range<C>, List<RexNode>> p = rangeTerms.get(ref.toString());
+    Pair<Range<C>, List<RexNode>> p = rangeTerms.get(ref);
     if (p == null) {
-      rangeTerms.put(ref.toString(),
+      rangeTerms.put(ref,
           Pair.of(range(comparison, v0),
               (List<RexNode>) ImmutableList.of(term)));
     } else {
@@ -1691,7 +1692,7 @@ public class RexSimplify {
           // Range is empty, not satisfiable
           return rexBuilder.makeLiteral(false);
         }
-        rangeTerms.put(ref.toString(),
+        rangeTerms.put(ref,
             Pair.of(Range.singleton(v0),
                 (List<RexNode>) ImmutableList.of(term)));
         // remove
@@ -1862,7 +1863,7 @@ public class RexSimplify {
           }
         }
         newBounds.add(term);
-        rangeTerms.put(ref.toString(),
+        rangeTerms.put(ref,
             Pair.of(r, (List<RexNode>) newBounds.build()));
       } else if (removeLowerBound) {
         ImmutableList.Builder<RexNode> newBounds = ImmutableList.builder();
@@ -1874,7 +1875,7 @@ public class RexSimplify {
           }
         }
         newBounds.add(term);
-        rangeTerms.put(ref.toString(),
+        rangeTerms.put(ref,
             Pair.of(r, (List<RexNode>) newBounds.build()));
       }
     }

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/RexSubQuery.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexSubQuery.java b/core/src/main/java/org/apache/calcite/rex/RexSubQuery.java
index 05fd176..fceb58e 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexSubQuery.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexSubQuery.java
@@ -30,6 +30,7 @@ import org.apache.calcite.sql.type.SqlTypeName;
 import com.google.common.collect.ImmutableList;
 
 import java.util.List;
+import javax.annotation.Nonnull;
 
 /**
  * Scalar expression that represents an IN, EXISTS or scalar sub-query.
@@ -108,11 +109,11 @@ public class RexSubQuery extends RexCall {
     return visitor.visitSubQuery(this, arg);
   }
 
-  @Override protected String computeDigest(boolean withType) {
-    StringBuilder sb = new StringBuilder(op.getName());
+  @Override protected @Nonnull String computeDigest(boolean withType) {
+    final StringBuilder sb = new StringBuilder(op.getName());
     sb.append("(");
     for (RexNode operand : operands) {
-      sb.append(operand.toString());
+      sb.append(operand);
       sb.append(", ");
     }
     sb.append("{\n");

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/RexUtil.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexUtil.java b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
index 2eed8d2..56eabb6 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexUtil.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
@@ -968,8 +968,8 @@ public class RexUtil {
    * representation. For example, "10" integer and "10" bigint result in
    * different keys.
    */
-  public static Pair<String, String> makeKey(RexNode expr) {
-    return Pair.of(expr.toString(), expr.getType().getFullTypeString());
+  public static Pair<RexNode, String> makeKey(RexNode expr) {
+    return Pair.of(expr, expr.getType().getFullTypeString());
   }
 
   /**
@@ -1054,17 +1054,17 @@ public class RexUtil {
       return ImmutableList.of();
     }
     final ImmutableList.Builder<RexNode> builder = ImmutableList.builder();
-    final Set<String> digests = new HashSet<>(); // to eliminate duplicates
+    final Set<RexNode> set = new HashSet<>(); // to eliminate duplicates
     for (RexNode node : nodes) {
       if (node != null) {
-        addAnd(builder, digests, node);
+        addAnd(builder, set, node);
       }
     }
     return builder.build();
   }
 
   private static void addAnd(ImmutableList.Builder<RexNode> builder,
-      Set<String> digests, RexNode node) {
+      Set<RexNode> digests, RexNode node) {
     switch (node.getKind()) {
     case AND:
       for (RexNode operand : ((RexCall) node).getOperands()) {
@@ -1072,7 +1072,7 @@ public class RexUtil {
       }
       return;
     default:
-      if (!node.isAlwaysTrue() && digests.add(node.toString())) {
+      if (!node.isAlwaysTrue() && digests.add(node)) {
         builder.add(node);
       }
     }
@@ -1122,23 +1122,23 @@ public class RexUtil {
       return ImmutableList.of();
     }
     final ImmutableList.Builder<RexNode> builder = ImmutableList.builder();
-    final Set<String> digests = new HashSet<>(); // to eliminate duplicates
+    final Set<RexNode> set = new HashSet<>(); // to eliminate duplicates
     for (RexNode node : nodes) {
-      addOr(builder, digests, node);
+      addOr(builder, set, node);
     }
     return builder.build();
   }
 
   private static void addOr(ImmutableList.Builder<RexNode> builder,
-      Set<String> digests, RexNode node) {
+      Set<RexNode> set, RexNode node) {
     switch (node.getKind()) {
     case OR:
       for (RexNode operand : ((RexCall) node).getOperands()) {
-        addOr(builder, digests, operand);
+        addOr(builder, set, operand);
       }
       return;
     default:
-      if (!node.isAlwaysFalse() && digests.add(node.toString())) {
+      if (!node.isAlwaysFalse() && set.add(node)) {
         builder.add(node);
       }
     }
@@ -1637,7 +1637,7 @@ public class RexUtil {
     Iterator<RexNode> iterator = targets.iterator();
     while (iterator.hasNext()) {
       RexNode next = iterator.next();
-      if (eq(next, e)) {
+      if (next.equals(e)) {
         ++count;
         iterator.remove();
       }
@@ -1650,6 +1650,7 @@ public class RexUtil {
    * <p>This method considers structure, not semantics. 'x &lt; y' is not
    * equivalent to 'y &gt; x'.
    */
+  @Deprecated // use e1.equals(e2)
   public static boolean eq(RexNode e1, RexNode e2) {
     return e1 == e2 || e1.toString().equals(e2.toString());
   }
@@ -2071,7 +2072,7 @@ public class RexUtil {
    * Walks over expressions and builds a bank of common sub-expressions.
    */
   private static class ExpressionNormalizer extends RexVisitorImpl<RexNode> {
-    final Map<String, RexNode> mapDigestToExpr = new HashMap<>();
+    final Map<RexNode, RexNode> map = new HashMap<>();
     final boolean allowDups;
 
     protected ExpressionNormalizer(boolean allowDups) {
@@ -2080,8 +2081,7 @@ public class RexUtil {
     }
 
     protected RexNode register(RexNode expr) {
-      final String key = expr.toString();
-      final RexNode previous = mapDigestToExpr.put(key, expr);
+      final RexNode previous = map.put(expr, expr);
       if (!allowDups && (previous != null)) {
         throw new SubExprExistsException(expr);
       }
@@ -2089,7 +2089,7 @@ public class RexUtil {
     }
 
     protected RexNode lookup(RexNode expr) {
-      return mapDigestToExpr.get(expr.toString());
+      return map.get(expr);
     }
 
     public RexNode visitInputRef(RexInputRef inputRef) {
@@ -2327,7 +2327,7 @@ public class RexUtil {
         return and(pullList(operands));
       case OR:
         operands = flattenOr(((RexCall) rex).getOperands());
-        final Map<String, RexNode> factors = commonFactors(operands);
+        final Map<RexNode, RexNode> factors = commonFactors(operands);
         if (factors.isEmpty()) {
           return or(operands);
         }
@@ -2356,25 +2356,25 @@ public class RexUtil {
       return list;
     }
 
-    private Map<String, RexNode> commonFactors(List<RexNode> nodes) {
-      final Map<String, RexNode> map = new HashMap<>();
+    private Map<RexNode, RexNode> commonFactors(List<RexNode> nodes) {
+      final Map<RexNode, RexNode> map = new HashMap<>();
       int i = 0;
       for (RexNode node : nodes) {
         if (i++ == 0) {
           for (RexNode conjunction : RelOptUtil.conjunctions(node)) {
-            map.put(conjunction.toString(), conjunction);
+            map.put(conjunction, conjunction);
           }
         } else {
-          map.keySet().retainAll(strings(RelOptUtil.conjunctions(node)));
+          map.keySet().retainAll(RelOptUtil.conjunctions(node));
         }
       }
       return map;
     }
 
-    private RexNode removeFactor(Map<String, RexNode> factors, RexNode node) {
+    private RexNode removeFactor(Map<RexNode, RexNode> factors, RexNode node) {
       List<RexNode> list = new ArrayList<>();
       for (RexNode operand : RelOptUtil.conjunctions(node)) {
-        if (!factors.containsKey(operand.toString())) {
+        if (!factors.containsKey(operand)) {
           list.add(operand);
         }
       }

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/rex/RexVariable.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexVariable.java b/core/src/main/java/org/apache/calcite/rex/RexVariable.java
index 717a569..6a702c0 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexVariable.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexVariable.java
@@ -18,6 +18,8 @@ package org.apache.calcite.rex;
 
 import org.apache.calcite.rel.type.RelDataType;
 
+import java.util.Objects;
+
 /**
  * A row-expression which references a field.
  */
@@ -32,11 +34,9 @@ public abstract class RexVariable extends RexNode {
   protected RexVariable(
       String name,
       RelDataType type) {
-    assert type != null;
-    assert name != null;
-    this.name = name;
-    this.digest = name;
-    this.type = type;
+    this.name = Objects.requireNonNull(name);
+    this.digest = Objects.requireNonNull(name);
+    this.type = Objects.requireNonNull(type);
   }
 
   //~ Methods ----------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
index 9512312..2e58719 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -706,8 +706,8 @@ public class SqlToRelConverter {
       final List<Integer> origins = new ArrayList<>();
       int dupCount = 0;
       for (int i = 0; i < projectExprs.size(); i++) {
-        int x = findExpr(projectExprs.get(i), projectExprs, i);
-        if (x >= 0) {
+        int x = projectExprs.indexOf(projectExprs.get(i));
+        if (x >= 0 && x < i) {
           origins.add(x);
           ++dupCount;
         } else {
@@ -770,16 +770,6 @@ public class SqlToRelConverter {
         false);
   }
 
-  private int findExpr(RexNode seek, List<RexNode> exprs, int count) {
-    for (int i = 0; i < count; i++) {
-      RexNode expr = exprs.get(i);
-      if (expr.toString().equals(seek.toString())) {
-        return i;
-      }
-    }
-    return -1;
-  }
-
   /**
    * Converts a query's ORDER BY clause, if any.
    *
@@ -5095,7 +5085,7 @@ public class SqlToRelConverter {
     private int lookupOrCreateGroupExpr(RexNode expr) {
       int index = 0;
       for (RexNode convertedInputExpr : Pair.left(convertedInputExprs)) {
-        if (expr.toString().equals(convertedInputExpr.toString())) {
+        if (expr.equals(convertedInputExpr)) {
           return index;
         }
         ++index;

http://git-wip-us.apache.org/repos/asf/calcite/blob/847e76cd/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index ba38082..dd775ff 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -5245,9 +5245,9 @@ LogicalAggregate(group=[{0}], EXPR$1=[STDDEV_POP($1)], EXPR$2=[AVG($1)], EXPR$3=
         </Resource>
         <Resource name="planAfter">
             <![CDATA[
-LogicalProject(NAME=[$0], EXPR$1=[CAST(POWER(/(-($1, /(*($2, $2), $3)), $3), 0.5)):INTEGER NOT NULL], EXPR$2=[CAST(/($2, $3)):INTEGER NOT NULL], EXPR$3=[CAST(POWER(/(-($4, /(*($2, $2), $3)), CASE(=($3, 1), null, -($3, 1))), 0.5)):INTEGER NOT NULL], EXPR$4=[CAST(/(-($5, /(*($2, $2), $3)), $3)):INTEGER NOT NULL], EXPR$5=[CAST(/(-($6, /(*($2, $2), $3)), CASE(=($3, 1), null, -($3, 1)))):INTEGER NOT NULL])
-  LogicalAggregate(group=[{0}], agg#0=[$SUM0($2)], agg#1=[$SUM0($1)], agg#2=[COUNT()], agg#3=[$SUM0($3)], agg#4=[$SUM0($4)], agg#5=[$SUM0($5)])
-    LogicalProject(NAME=[$0], DEPTNO=[$1], $f2=[*($1, $1)], $f3=[*($1, $1)], $f4=[*($1, $1)], $f5=[*($1, $1)])
+LogicalProject(NAME=[$0], EXPR$1=[CAST(POWER(/(-($1, /(*($2, $2), $3)), $3), 0.5)):INTEGER NOT NULL], EXPR$2=[CAST(/($2, $3)):INTEGER NOT NULL], EXPR$3=[CAST(POWER(/(-($1, /(*($2, $2), $3)), CASE(=($3, 1), null, -($3, 1))), 0.5)):INTEGER NOT NULL], EXPR$4=[CAST(/(-($1, /(*($2, $2), $3)), $3)):INTEGER NOT NULL], EXPR$5=[CAST(/(-($1, /(*($2, $2), $3)), CASE(=($3, 1), null, -($3, 1)))):INTEGER NOT NULL])
+  LogicalAggregate(group=[{0}], agg#0=[$SUM0($2)], agg#1=[$SUM0($1)], agg#2=[COUNT()])
+    LogicalProject(NAME=[$0], DEPTNO=[$1], $f2=[*($1, $1)])
       LogicalProject(NAME=[$1], DEPTNO=[$0])
         LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
 ]]>
@@ -5775,8 +5775,7 @@ LogicalProject(DEPTNO=[$0], ENAME=[$1])
   LogicalProject(DEPTNO=[$9], ENAME=[$1])
     LogicalJoin(condition=[=($7, $9)], joinType=[inner])
       LogicalTableScan(table=[[CATALOG, SALES, EMP]])
-      LogicalFilter(condition=[=($0, 10)])
-        LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+      LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
 ]]>
         </Resource>
     </TestCase>


[11/15] calcite git commit: [CALCITE-2720] RelMetadataQuery.getTableOrigin throws IndexOutOfBoundsException if RelNode has no columns (Zoltan Haindrich)

Posted by jh...@apache.org.
[CALCITE-2720] RelMetadataQuery.getTableOrigin throws IndexOutOfBoundsException if RelNode has no columns (Zoltan Haindrich)

An example is an Aggregate with empty groupKey an no aggregate
functions. Occurs during LoptOptimizeJoinRule.

Close apache/calcite#949

An unrelated PR that we wish to close:
Close apache/calcite#949


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/e8093446
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/e8093446
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/e8093446

Branch: refs/heads/master
Commit: e80934465cee318fd306159489ee3e5c768dcb68
Parents: 7f8556e
Author: Zoltan Haindrich <ki...@rxd.hu>
Authored: Tue Nov 20 09:28:20 2018 +0100
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Nov 30 21:11:49 2018 -0800

----------------------------------------------------------------------
 core/src/main/java/org/apache/calcite/sql/SqlOperator.java   | 7 ++++++-
 core/src/main/java/org/apache/calcite/tools/RelBuilder.java  | 5 -----
 .../test/java/org/apache/calcite/test/RelBuilderTest.java    | 8 ++++----
 .../test/java/org/apache/calcite/test/RelMetadataTest.java   | 1 -
 .../test/java/org/apache/calcite/test/RexProgramTest.java    | 5 +++++
 5 files changed, 15 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/e8093446/core/src/main/java/org/apache/calcite/sql/SqlOperator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlOperator.java b/core/src/main/java/org/apache/calcite/sql/SqlOperator.java
index c4c092e..80f9c9f 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlOperator.java
@@ -467,7 +467,12 @@ public abstract class SqlOperator {
   public RelDataType inferReturnType(
       SqlOperatorBinding opBinding) {
     if (returnTypeInference != null) {
-      return returnTypeInference.inferReturnType(opBinding);
+      RelDataType returnType = returnTypeInference.inferReturnType(opBinding);
+      if (returnType == null) {
+        throw new IllegalArgumentException("Cannot infer return type for "
+            + opBinding.getOperator() + "; operand types: " + opBinding.collectOperandTypes());
+      }
+      return returnType;
     }
 
     // Derived type should have overridden this method, since it didn't

http://git-wip-us.apache.org/repos/asf/calcite/blob/e8093446/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index 9b7a6e9..0465286 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -556,11 +556,6 @@ public class RelBuilder {
   private RexNode call(SqlOperator operator, List<RexNode> operandList) {
     final RexBuilder builder = cluster.getRexBuilder();
     final RelDataType type = builder.deriveReturnType(operator, operandList);
-    if (type == null) {
-      throw new IllegalArgumentException("cannot derive type: " + operator
-          + "; operands: "
-          + Lists.transform(operandList, e -> e + ": " + e.getType()));
-    }
     return builder.makeCall(type, operator, operandList);
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/e8093446/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
index ff3f861..77a91f7 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -401,8 +401,8 @@ public class RelBuilderTest {
       fail("expected error, got " + call);
     } catch (IllegalArgumentException e) {
       assertThat(e.getMessage(),
-          is("cannot derive type: +; "
-              + "operands: [$1: VARCHAR(10), $3: SMALLINT]"));
+          is("Cannot infer return type for +; "
+              + "operand types: [VARCHAR(10), SMALLINT]"));
     }
   }
 
@@ -2057,7 +2057,7 @@ public class RelBuilderTest {
       builder.call(SqlStdOperatorTable.PLUS, Lists.newArrayList(arg0, arg1));
       fail("Invalid combination of parameter types");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage(), containsString("cannot derive type"));
+      assertThat(e.getMessage(), containsString("Cannot infer return type"));
     }
 
     // test for b) call(operator, RexNode...)
@@ -2065,7 +2065,7 @@ public class RelBuilderTest {
       builder.call(SqlStdOperatorTable.PLUS, arg0, arg1);
       fail("Invalid combination of parameter types");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage(), containsString("cannot derive type"));
+      assertThat(e.getMessage(), containsString("Cannot infer return type"));
     }
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/e8093446/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
index 4628875..0706782 100644
--- a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
@@ -87,7 +87,6 @@ import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.tools.FrameworkConfig;
 import org.apache.calcite.tools.Frameworks;
 import org.apache.calcite.tools.RelBuilder;
-import org.apache.calcite.tools.RelBuilder.AggCall;
 import org.apache.calcite.util.BuiltInMethod;
 import org.apache.calcite.util.ImmutableBitSet;
 import org.apache.calcite.util.ImmutableIntList;

http://git-wip-us.apache.org/repos/asf/calcite/blob/e8093446/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RexProgramTest.java b/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
index b52937f..829024b 100644
--- a/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
@@ -748,6 +748,11 @@ public class RexProgramTest extends RexProgramBuilderBase {
     checkSimplifyUnchanged(cast(cast(vVarchar(), tInt()), tVarchar()));
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void checkNoCommonReturnTypeException() {
+    coalesce(vVarchar(1), vInt(2));
+  }
+
   /** Unit test for {@link org.apache.calcite.rex.RexUtil#toCnf}. */
   @Test public void testCnf() {
     final RelDataType booleanType =


[03/15] calcite git commit: [CALCITE-2715] In JDBC adapter, do not generate character set in data types for MS SQL Server (Piotr Bojko)

Posted by jh...@apache.org.
[CALCITE-2715] In JDBC adapter, do not generate character set in data types for MS SQL Server (Piotr Bojko)

MS SQL Server does not support character set as part of data type.

Close apache/calcite#945


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/ce47088f
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/ce47088f
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/ce47088f

Branch: refs/heads/master
Commit: ce47088f118b796908b1de4b1440d21d08d2b6b7
Parents: ed3da62
Author: Piotr Bojko <pt...@gmail.com>
Authored: Wed Nov 28 12:12:54 2018 +0100
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Nov 30 17:57:00 2018 -0800

----------------------------------------------------------------------
 .../org/apache/calcite/sql/dialect/MssqlSqlDialect.java  |  4 ++++
 .../calcite/rel/rel2sql/RelToSqlConverterTest.java       | 11 +++++++++++
 2 files changed, 15 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/ce47088f/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java
index 6261481..644a39b 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java
@@ -81,6 +81,10 @@ public class MssqlSqlDialect extends SqlDialect {
     }
   }
 
+  @Override public boolean supportsCharSet() {
+    return false;
+  }
+
   /**
    * Unparses datetime floor for Microsoft SQL Server.
    * There is no TRUNC function, so simulate this using calls to CONVERT.

http://git-wip-us.apache.org/repos/asf/calcite/blob/ce47088f/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index 189c33f..93676c1 100644
--- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -495,6 +495,17 @@ public class RelToSqlConverterTest {
     sql(query).withHive().ok(expected);
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-2715">[CALCITE-2715]
+   * MS SQL Server does not support character set as part of data type</a>. */
+  @Test public void testMssqlCharacterSet() {
+    String query = "select \"hire_date\", cast(\"hire_date\" as varchar(10))\n"
+        + "from \"foodmart\".\"reserve_employee\"";
+    final String expected = "SELECT [hire_date], CAST([hire_date] AS VARCHAR(10))\n"
+        + "FROM [foodmart].[reserve_employee]";
+    sql(query).withMssql().ok(expected);
+  }
+
   /**
    * Tests that IN can be un-parsed.
    *


[07/15] calcite git commit: [CALCITE-2705] Site: Remove duplicate "selectivity" in list of metadata types (Alan Jin)

Posted by jh...@apache.org.
[CALCITE-2705] Site: Remove duplicate "selectivity" in list of metadata types (Alan Jin)

Close apache/calcite#940


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/849f1416
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/849f1416
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/849f1416

Branch: refs/heads/master
Commit: 849f1416d435acb1409be853f8cc62ff610eda9a
Parents: 453f171
Author: LantaoJin <ji...@gmail.com>
Authored: Mon Nov 26 20:06:44 2018 +0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Nov 30 20:29:36 2018 -0800

----------------------------------------------------------------------
 site/_docs/adapter.md | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/849f1416/site/_docs/adapter.md
----------------------------------------------------------------------
diff --git a/site/_docs/adapter.md b/site/_docs/adapter.md
index 20a7703..80fbd21 100644
--- a/site/_docs/adapter.md
+++ b/site/_docs/adapter.md
@@ -601,9 +601,8 @@ There are many built-in kinds of metadata, including
 [row count]({{ site.apiRoot }}/org/apache/calcite/rel/metadata/RelMdRowCount.html),
 [selectivity]({{ site.apiRoot }}/org/apache/calcite/rel/metadata/RelMdSelectivity.html),
 [size]({{ site.apiRoot }}/org/apache/calcite/rel/metadata/RelMdSize.html),
-[table references]({{ site.apiRoot }}/org/apache/calcite/rel/metadata/RelMdTableReferences.html),
-[unique keys]({{ site.apiRoot }}/org/apache/calcite/rel/metadata/RelMdUniqueKeys.html), and
-[selectivity]({{ site.apiRoot }}/org/apache/calcite/rel/metadata/RelMdSelectivity.html);
+[table references]({{ site.apiRoot }}/org/apache/calcite/rel/metadata/RelMdTableReferences.html), and
+[unique keys]({{ site.apiRoot }}/org/apache/calcite/rel/metadata/RelMdUniqueKeys.html);
 you can also define your own.
 
 You can then supply a *metadata provider* that computes that kind of metadata


[08/15] calcite git commit: [CALCITE-2637] In SQL parser, allow prefix '-' between BETWEEN and AND (Qi Yu)

Posted by jh...@apache.org.
[CALCITE-2637] In SQL parser, allow prefix '-' between BETWEEN and AND (Qi Yu)

For example, "WHERE deptno BETWEEN - deptno AND 5".

Fix Mimer URL; add a test (Julian Hyde)

Close apache/calcite#941


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/453f171d
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/453f171d
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/453f171d

Branch: refs/heads/master
Commit: 453f171d900ac8ac04bed95bbdd59c986ba4e726
Parents: 847e76c
Author: park.yq <pa...@alibaba-inc.com>
Authored: Mon Nov 26 21:12:45 2018 +0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Nov 30 20:29:36 2018 -0800

----------------------------------------------------------------------
 core/src/main/codegen/templates/Parser.jj       |  6 +++--
 .../calcite/sql/parser/SqlParserTest.java       | 24 +++++++++++++++++++-
 2 files changed, 27 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/453f171d/core/src/main/codegen/templates/Parser.jj
----------------------------------------------------------------------
diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj
index 6361391..247c8b0 100644
--- a/core/src/main/codegen/templates/Parser.jj
+++ b/core/src/main/codegen/templates/Parser.jj
@@ -3031,6 +3031,7 @@ List<Object> Expression2(ExprContext exprContext) :
 {
     final List<Object> list = new ArrayList();
     List<Object> list2;
+    final List<Object> list3 = new ArrayList();
     SqlNodeList nodeList;
     SqlNode e;
     SqlOperator op;
@@ -3106,9 +3107,10 @@ List<Object> Expression2(ExprContext exprContext) :
                         <ASYMMETRIC>
                     ]
                 )
-                e = Expression3(ExprContext.ACCEPT_SUB_QUERY) {
+                Expression2b(ExprContext.ACCEPT_SUB_QUERY, list3) {
                     list.add(new SqlParserUtil.ToTreeListItem(op, s.pos()));
-                    list.add(e);
+                    list.addAll(list3);
+                    list3.clear();
                 }
             |
                 {

http://git-wip-us.apache.org/repos/asf/calcite/blob/453f171d/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
index 6d5d1dd..535aa15 100644
--- a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
@@ -83,7 +83,7 @@ public class SqlParserTest {
    * the SQL:92, SQL:99, SQL:2003, SQL:2011, SQL:2014 standards and in Calcite.
    *
    * <p>The standard keywords are derived from
-   * <a href="http://developer.mimer.com/validator/sql-reserved-words.tml">Mimer</a>
+   * <a href="https://developer.mimer.com/wp-content/uploads/2018/05/Standard-SQL-Reserved-Words-Summary.pdf">Mimer</a>
    * and from the specification.
    *
    * <p>If a new <b>reserved</b> keyword is added to the parser, include it in
@@ -731,6 +731,28 @@ public class SqlParserTest {
             + "INNER JOIN `DEPT` AS `D` (`DEPTNO`, `DNAME`) ON (`EMP`.`DEPTNO` = `DEPT`.`DEPTNO`)");
   }
 
+  /** Test case that does not reproduce but is related to
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-2637">[CALCITE-2637]
+   * Prefix '-' operator failed between BETWEEN and AND</a>. */
+  @Test public void testBetweenAnd() {
+    final String sql = "select * from emp\n"
+        + "where deptno between - DEPTNO + 1 and 5";
+    final String expected = "SELECT *\n"
+        + "FROM `EMP`\n"
+        + "WHERE (`DEPTNO` BETWEEN ASYMMETRIC ((- `DEPTNO`) + 1) AND 5)";
+    sql(sql).ok(expected);
+  }
+
+  @Test public void testBetweenAnd2() {
+    final String sql = "select * from emp\n"
+        + "where deptno between - DEPTNO + 1 and - empno - 3";
+    final String expected = "SELECT *\n"
+        + "FROM `EMP`\n"
+        + "WHERE (`DEPTNO` BETWEEN ASYMMETRIC ((- `DEPTNO`) + 1)"
+        + " AND ((- `EMPNO`) - 3))";
+    sql(sql).ok(expected);
+  }
+
   @Ignore
   @Test public void testDerivedColumnListNoAs() {
     check("select * from emp e (empno, gender) where true", "foo");


[14/15] calcite git commit: [CALCITE-2701] Make generated Baz classes immutable

Posted by jh...@apache.org.
[CALCITE-2701] Make generated Baz classes immutable

Revert the workaround introduced in EnumerableRelImplementor due to JANINO-169.


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/fbf193b1
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/fbf193b1
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/fbf193b1

Branch: refs/heads/master
Commit: fbf193b19b382efee6ab6d31c98e796de0df0784
Parents: 36bd5fb
Author: Stamatis Zampetakis <za...@gmail.com>
Authored: Fri Nov 23 11:58:10 2018 +0100
Committer: Julian Hyde <jh...@apache.org>
Committed: Sat Dec 1 14:43:58 2018 -0800

----------------------------------------------------------------------
 .../adapter/enumerable/EnumerableRelImplementor.java | 15 +--------------
 .../test/java/org/apache/calcite/test/JdbcTest.java  |  5 +++--
 2 files changed, 4 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/fbf193b1/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelImplementor.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelImplementor.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelImplementor.java
index 1b91d7f..b902926 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelImplementor.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelImplementor.java
@@ -124,13 +124,6 @@ public class EnumerableRelImplementor extends JavaRelImplementor {
     final List<MemberDeclaration> memberDeclarations = new ArrayList<>();
     new TypeRegistrar(memberDeclarations).go(result);
 
-    // The following is a workaround to
-    // http://jira.codehaus.org/browse/JANINO-169. Otherwise we'd remove the
-    // member variable, rename the "root0" parameter as "root", and reference it
-    // directly from inner classes.
-    final ParameterExpression root0_ =
-        Expressions.parameter(Modifier.FINAL, DataContext.class, "root0");
-
     // This creates the following code
     // final Integer v1stashed = (Integer) root.get("v1stashed")
     // It is convenient for passing non-literal "compile-time" constants
@@ -145,20 +138,14 @@ public class EnumerableRelImplementor extends JavaRelImplementor {
 
     final BlockStatement block = Expressions.block(
         Iterables.concat(
-            ImmutableList.of(
-                Expressions.statement(
-                    Expressions.assign(DataContext.ROOT, root0_))),
             stashed,
             result.block.statements));
     memberDeclarations.add(
-        Expressions.fieldDecl(0, DataContext.ROOT, null));
-
-    memberDeclarations.add(
         Expressions.methodDecl(
             Modifier.PUBLIC,
             Enumerable.class,
             BuiltInMethod.BINDABLE_BIND.method.getName(),
-            Expressions.list(root0_),
+            Expressions.list(DataContext.ROOT),
             block));
     memberDeclarations.add(
         Expressions.methodDecl(Modifier.PUBLIC, Class.class,

http://git-wip-us.apache.org/repos/asf/calcite/blob/fbf193b1/core/src/test/java/org/apache/calcite/test/JdbcTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index f84f24f..3491b53 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -1381,8 +1381,9 @@ public class JdbcTest {
 
   /** Janino bug
    * <a href="https://jira.codehaus.org/browse/JANINO-169">[JANINO-169]</a>
-   * running queries against the JDBC adapter. As of janino-2.7.3 bug is
-   * open but we have a workaround in EnumerableRelImplementor. */
+   * running queries against the JDBC adapter. The bug is not present with
+   * janino-3.0.9 so the workaround in EnumerableRelImplementor was removed.
+   */
   @Test public void testJanino169() {
     CalciteAssert.that()
         .with(CalciteAssert.Config.JDBC_FOODMART)


[06/15] calcite git commit: [CALCITE-2542] In SQL parser, allow '. field' to follow expressions other than tables and columns (Rong Rong)

Posted by jh...@apache.org.
[CALCITE-2542] In SQL parser, allow '. field' to follow expressions other than tables and columns (Rong Rong)

Fix how SqlNode AtomicRowExpression + DOT operation is parsed, adding
in <DOT> parser for expression2b for parsing additional identifier
after AtomicRowExpression.

Close apache/calcite#933


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/45258404
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/45258404
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/45258404

Branch: refs/heads/master
Commit: 45258404b7844204266f8c356a8e41ef1f1e3683
Parents: 849f141
Author: Rong Rong <wa...@hotmail.com>
Authored: Mon Nov 19 16:42:10 2018 -0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Nov 30 20:29:36 2018 -0800

----------------------------------------------------------------------
 core/src/main/codegen/templates/Parser.jj       | 10 ++++++++
 .../apache/calcite/sql/fun/SqlDotOperator.java  | 18 +++++++++++---
 .../calcite/sql/parser/SqlParserTest.java       | 15 +++++++----
 .../calcite/test/SqlToRelConverterTest.java     | 10 ++++++++
 .../apache/calcite/test/SqlValidatorTest.java   | 26 ++++++++++++++++++--
 .../calcite/test/SqlToRelConverterTest.xml      | 22 +++++++++++++++++
 6 files changed, 90 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/main/codegen/templates/Parser.jj
----------------------------------------------------------------------
diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj
index 247c8b0..b137d6f 100644
--- a/core/src/main/codegen/templates/Parser.jj
+++ b/core/src/main/codegen/templates/Parser.jj
@@ -3000,6 +3000,7 @@ void Expression2b(ExprContext exprContext, List<Object> list) :
 {
     SqlNode e;
     SqlOperator op;
+    String p;
 }
 {
     (
@@ -3011,6 +3012,15 @@ void Expression2b(ExprContext exprContext, List<Object> list) :
     e = Expression3(exprContext) {
         list.add(e);
     }
+    (
+        <DOT>
+        p = Identifier() {
+            list.add(
+                new SqlParserUtil.ToTreeListItem(
+                    SqlStdOperatorTable.DOT, getPos()));
+            list.add(new SqlIdentifier(p, getPos()));
+        }
+    )*
 }
 
 /**

http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java
index e9b70c7..3739a40 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java
@@ -93,14 +93,22 @@ public class SqlDotOperator extends SqlSpecialOperator {
 
   @Override public RelDataType deriveType(SqlValidator validator,
       SqlValidatorScope scope, SqlCall call) {
-    RelDataType nodeType = validator.deriveType(scope, call.getOperandList().get(0));
+    final SqlNode operand = call.getOperandList().get(0);
+    final RelDataType nodeType =
+        validator.deriveType(scope, operand);
     assert nodeType != null;
+    if (!nodeType.isStruct()) {
+      throw SqlUtil.newContextException(operand.getParserPosition(),
+          Static.RESOURCE.incompatibleTypes());
+    }
 
-    final String fieldName = call.getOperandList().get(1).toString();
-    RelDataTypeField field =
+    final SqlNode fieldId = call.operand(1);
+    final String fieldName = fieldId.toString();
+    final RelDataTypeField field =
         nodeType.getField(fieldName, false, false);
     if (field == null) {
-      throw SqlUtil.newContextException(SqlParserPos.ZERO, Static.RESOURCE.unknownField(fieldName));
+      throw SqlUtil.newContextException(fieldId.getParserPosition(),
+          Static.RESOURCE.unknownField(fieldName));
     }
     RelDataType type = field.getType();
 
@@ -130,6 +138,8 @@ public class SqlDotOperator extends SqlSpecialOperator {
         callBinding.getValidator().deriveType(callBinding.getScope(), left);
     if (type.getSqlTypeName() != SqlTypeName.ROW) {
       return false;
+    } else if (type.getSqlIdentifier().isStar()) {
+      return false;
     }
     final RelDataType operandType = callBinding.getOperandType(0);
     final SqlSingleOperandTypeChecker checker = getChecker(operandType);

http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
index 535aa15..a7a1233 100644
--- a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
@@ -1094,6 +1094,11 @@ public class SqlParserTest {
     return false;
   }
 
+  @Test public void testRowWitDot() {
+    check("select (1,2).a from c.t", "SELECT ((ROW(1, 2)).`A`)\nFROM `C`.`T`");
+    check("select row(1,2).a from c.t", "SELECT ((ROW(1, 2)).`A`)\nFROM `C`.`T`");
+  }
+
   @Test public void testPeriod() {
     // We don't have a PERIOD constructor currently;
     // ROW constructor is sufficient for now.
@@ -1514,6 +1519,10 @@ public class SqlParserTest {
             + "FROM `EMP`");
   }
 
+  @Test public void testFunctionCallWithDot() {
+    checkExp("foo(a,b).c", "(`FOO`(`A`, `B`).`C`)");
+  }
+
   @Test public void testFunctionInFunction() {
     checkExp("ln(power(2,2))", "LN(POWER(2, 2))");
   }
@@ -2472,11 +2481,6 @@ public class SqlParserTest {
                 + "FROM `EMP`");
   }
 
-  @Test public void testTableStarColumnFails() {
-    sql("select emp.*^.^xx from emp")
-        .fails("(?s).*Encountered \".\" .*");
-  }
-
   @Test public void testNotExists() {
     check(
         "select * from dept where not not exists (select * from emp) and true",
@@ -6620,6 +6624,7 @@ public class SqlParserTest {
         "(?s)Encountered \"to year\" at line 1, column 19.\n"
             + "Was expecting one of:\n"
             + "    <EOF> \n"
+            + "    \"\\.\" \\.\\.\\.\n"
             + "    \"NOT\" \\.\\.\\..*");
     checkExpFails("interval '1-2' year ^to^ day", ANY);
     checkExpFails("interval '1-2' year ^to^ hour", ANY);

http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index 94707fe..77fa01c 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -78,6 +78,16 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
     sql(sql).convertsTo(plan);
   }
 
+  @Test public void testDotLiteralAfterNestedRow() {
+    final String sql = "select ((1,2),(3,4,5)).\"EXPR$1\".\"EXPR$2\" from emp";
+    sql(sql).ok();
+  }
+
+  @Test public void testDotLiteralAfterRow() {
+    final String sql = "select row(1,2).\"EXPR$1\" from emp";
+    sql(sql).ok();
+  }
+
   @Test public void testIntegerLiteral() {
     final String sql = "select 1 from emp";
     sql(sql).ok();

http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index ab2d444..c59f3a1 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -1259,6 +1259,26 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
         "INTEGER NOT NULL");
   }
 
+  @Test public void testRowWitValidDot() {
+    checkColumnType("select ((1,2),(3,4,5)).\"EXPR$1\".\"EXPR$2\"\n from dept",
+        "INTEGER NOT NULL");
+    checkColumnType("select row(1,2).\"EXPR$1\" from dept",
+        "INTEGER NOT NULL");
+    checkColumnType("select t.a.\"EXPR$1\" from (select row(1,2) as a from (values (1))) as t",
+        "INTEGER NOT NULL");
+  }
+
+  @Test public void testRowWithInvalidDotOperation() {
+    final String sql = "select t.^s.\"EXPR$1\"^ from (\n"
+        + "  select 1 AS s from (values (1))) as t";
+    checkExpFails(sql,
+        "(?s).*Column 'S\\.EXPR\\$1' not found in table 'T'.*");
+    checkExpFails("select ^array[1, 2, 3]^.\"EXPR$1\" from dept",
+        "(?s).*Incompatible types.*");
+    checkExpFails("select ^'mystr'^.\"EXPR$1\" from dept",
+        "(?s).*Incompatible types.*");
+  }
+
   @Test public void testMultiset() {
     checkExpType("multiset[1]", "INTEGER NOT NULL MULTISET NOT NULL");
     checkExpType(
@@ -4818,8 +4838,10 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
 
   @Test public void testStarDotIdFails() {
     // Fails in parser
+    sql("select emp.^*^.\"EXPR$1\" from emp")
+        .fails("(?s).*Unknown field '\\*'");
     sql("select emp.^*^.foo from emp")
-        .fails("(?s).*Encountered \".\" at .*");
+        .fails("(?s).*Unknown field '\\*'");
     // Parser does not allow star dot identifier.
     sql("select ^*^.foo from emp")
         .fails("(?s).*Encountered \".\" at .*");
@@ -7966,7 +7988,7 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
   }
 
   @Test public void testArrayOfRecordType() {
-    sql("SELECT name, dept_nested.employees[1].ne as ne from dept_nested")
+    sql("SELECT name, dept_nested.employees[1].^ne^ as ne from dept_nested")
         .fails("Unknown field 'NE'");
     sql("SELECT name, dept_nested.employees[1].ename as ename from dept_nested")
         .type("RecordType(VARCHAR(10) NOT NULL NAME, VARCHAR(10) ENAME) NOT NULL");

http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
index c218e7c..9128946 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -717,6 +717,28 @@ LogicalProject(EXPR$0=[CHAR_LENGTH('foo')])
             <![CDATA[values (character_length('foo'))]]>
         </Resource>
     </TestCase>
+    <TestCase name="testDotLiteralAfterNestedRow">
+        <Resource name="sql">
+            <![CDATA[select ((1,2),(3,4,5))."EXPR$1"."EXPR$2" from emp]]>
+        </Resource>
+        <Resource name="plan">
+            <![CDATA[
+LogicalProject(EXPR$0=[ROW(ROW(1, 2), ROW(3, 4, 5)).EXPR$1.EXPR$2])
+  LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+    </TestCase>
+    <TestCase name="testDotLiteralAfterRow">
+        <Resource name="sql">
+            <![CDATA[select row(1,2)."EXPR$1" from emp]]>
+        </Resource>
+        <Resource name="plan">
+            <![CDATA[
+LogicalProject(EXPR$0=[ROW(1, 2).EXPR$1])
+  LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+    </TestCase>
     <TestCase name="testDynamicNestedColumn">
         <Resource name="sql">
             <![CDATA[select t3.fake_q1['fake_col2'] as fake2


[09/15] calcite git commit: [CALCITE-2717] Use Interner instead of LoadingCache to cache traits to allow GC (Haisheng Yuan)

Posted by jh...@apache.org.
[CALCITE-2717] Use Interner instead of LoadingCache to cache traits to allow GC (Haisheng Yuan)

Even though canonicalMap's value is soft referenced, key is strong referenced,
key and value are referencing the same object. So traits in the cache will
never be garbage-collected, which may cause OOM if we have tons of different
traits. This issue has been fixed by caching traits using Interner instead of
LoadingCache. In addition, canonizeComposite is removed, since canonize can do
the same work.

Close apache/calcite#951


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/de9a7164
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/de9a7164
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/de9a7164

Branch: refs/heads/master
Commit: de9a7164b2d3e5a1637254fd0d54c7a4c886166e
Parents: 4525840
Author: Haisheng Yuan <h....@alibaba-inc.com>
Authored: Fri Nov 30 14:25:20 2018 -0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Nov 30 20:29:37 2018 -0800

----------------------------------------------------------------------
 .../apache/calcite/plan/RelCompositeTrait.java  | 24 ++-------
 .../org/apache/calcite/plan/RelTraitDef.java    | 57 +++++---------------
 2 files changed, 15 insertions(+), 66 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/de9a7164/core/src/main/java/org/apache/calcite/plan/RelCompositeTrait.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/plan/RelCompositeTrait.java b/core/src/main/java/org/apache/calcite/plan/RelCompositeTrait.java
index 892088e..e3555b8 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelCompositeTrait.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelCompositeTrait.java
@@ -67,7 +67,7 @@ class RelCompositeTrait<T extends RelMultipleTrait> implements RelTrait {
       }
       compositeTrait = new RelCompositeTrait<>(def, (T[]) traits);
     }
-    return def.canonizeComposite(compositeTrait);
+    return def.canonize(compositeTrait);
   }
 
   public RelTraitDef getTraitDef() {
@@ -105,30 +105,12 @@ class RelCompositeTrait<T extends RelMultipleTrait> implements RelTrait {
     return ImmutableList.copyOf(traits);
   }
 
-  RelCompositeTrait<T> canonize(RelTraitDef<T> traitDef) {
-    T[] newTraits = null;
-    for (int i = 0; i < traits.length; i++) {
-      final T trait = traits[i];
-      final T trait2 = traitDef.canonize(trait);
-      if (trait2 != trait) {
-        if (newTraits == null) {
-          newTraits = traits.clone();
-        }
-        newTraits[i] = trait2;
-      }
-    }
-    if (newTraits == null) {
-      return this;
-    }
-    assert false;
-    // TODO: cache duplicate composites
-    return new RelCompositeTrait<>(traitDef, newTraits);
-  }
-
+  /** Returns the {@code i}th trait. */
   public T trait(int i) {
     return traits[i];
   }
 
+  /** Returns the number of traits. */
   public int size() {
     return traits.length;
   }

http://git-wip-us.apache.org/repos/asf/calcite/blob/de9a7164/core/src/main/java/org/apache/calcite/plan/RelTraitDef.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/plan/RelTraitDef.java b/core/src/main/java/org/apache/calcite/plan/RelTraitDef.java
index 1f0bdf8..43b44d4 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelTraitDef.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelTraitDef.java
@@ -19,12 +19,8 @@ package org.apache.calcite.plan;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.convert.ConverterRule;
 
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
-
-import java.util.List;
-import javax.annotation.Nonnull;
+import com.google.common.collect.Interner;
+import com.google.common.collect.Interners;
 
 /**
  * RelTraitDef represents a class of {@link RelTrait}s. Implementations of
@@ -56,34 +52,12 @@ import javax.annotation.Nonnull;
 public abstract class RelTraitDef<T extends RelTrait> {
   //~ Instance fields --------------------------------------------------------
 
-  private final LoadingCache<T, T> canonicalMap =
-      CacheBuilder.newBuilder()
-          .softValues()
-          .build(CacheLoader.from(key -> key));
-
-  /** Cache of composite traits.
-   *
-   * <p>Uses soft values to allow GC.
+  /**
+   * Cache of traits.
    *
-   * <p>You can look up using a {@link RelCompositeTrait} whose constituent
-   * traits are not canonized.
+   * <p>Uses weak interner to allow GC.
    */
-  private final LoadingCache<Object, RelCompositeTrait> canonicalCompositeMap =
-      CacheBuilder.newBuilder()
-          .softValues()
-          .build(
-              new CacheLoader<Object, RelCompositeTrait>() {
-                @Override public RelCompositeTrait load(@Nonnull Object key) {
-                  if (key instanceof RelCompositeTrait) {
-                    return (RelCompositeTrait) key;
-                  }
-                  @SuppressWarnings("unchecked")
-                  final List<RelMultipleTrait> list =
-                      (List<RelMultipleTrait>) key;
-                  final RelTraitDef def = list.get(0).getTraitDef();
-                  return (RelCompositeTrait) RelCompositeTrait.of(def, list);
-                }
-              });
+  private final Interner<T> interner = Interners.newWeakInterner();
 
   //~ Constructors -----------------------------------------------------------
 
@@ -126,20 +100,13 @@ public abstract class RelTraitDef<T extends RelTrait> {
    * @return a canonical RelTrait.
    */
   public final T canonize(T trait) {
-    if (trait instanceof RelCompositeTrait) {
-      RelCompositeTrait relCompositeTrait = (RelCompositeTrait) trait;
-      return (T) canonizeComposite(relCompositeTrait);
+    if (!(trait instanceof RelCompositeTrait)) {
+      assert getTraitClass().isInstance(trait)
+          : getClass().getName()
+          + " cannot canonize a "
+          + trait.getClass().getName();
     }
-    assert getTraitClass().isInstance(trait)
-        : getClass().getName()
-        + " cannot canonize a "
-        + trait.getClass().getName();
-
-    return canonicalMap.getUnchecked(trait);
-  }
-
-  final RelCompositeTrait canonizeComposite(RelCompositeTrait compositeTrait) {
-    return canonicalCompositeMap.getUnchecked(compositeTrait);
+    return interner.intern(trait);
   }
 
   /**


[12/15] calcite git commit: [CALCITE-2679] In Elasticsearch adapter, implement DISTINCT and GROUP BY without aggregate function (Siyuan Liu)

Posted by jh...@apache.org.
[CALCITE-2679] In Elasticsearch adapter, implement DISTINCT and GROUP BY without aggregate function (Siyuan Liu)

This commit mainly fixed 3 bugs:
1. Group by and distinct query enter the wrong execution branch.
   [ElasticsearchTable:126]
2. Values in agg bucket loses a part after returning as a result.
   [ElasticsearchJson:83-93, 546-551]
3. Logic of removing empty agg blocks can not work. [Elasticsearch:240-254]

Close apache/calcite#927


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/96605a86
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/96605a86
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/96605a86

Branch: refs/heads/master
Commit: 96605a86cfc4586951700727f12d33fc00c1ce55
Parents: e809344
Author: liusiyuan1 <li...@360.cn>
Authored: Sun Nov 18 23:28:29 2018 +0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Sat Dec 1 14:42:33 2018 -0800

----------------------------------------------------------------------
 .../elasticsearch/ElasticsearchJson.java        | 42 ++++++---
 .../elasticsearch/ElasticsearchTable.java       | 29 ++++--
 .../elasticsearch/ElasticSearchAdapterTest.java | 96 +++++++++++++++-----
 3 files changed, 120 insertions(+), 47 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/96605a86/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java
index e389ecf..dd49dfa 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java
@@ -27,15 +27,14 @@ import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.ImmutableSet;
 
 import java.io.IOException;
 import java.time.Duration;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.Deque;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -74,9 +73,17 @@ final class ElasticsearchJson {
         rows.computeIfAbsent(r, ignore -> new ArrayList<>()).add(v);
     aggregations.forEach(a -> visitValueNodes(a, new ArrayList<>(), cons));
     rows.forEach((k, v) -> {
-      Map<String, Object> row = new LinkedHashMap<>(k.keys);
-      v.forEach(val -> row.put(val.getName(), val.value()));
-      consumer.accept(row);
+      if (v.stream().anyMatch(val -> val instanceof GroupValue)) {
+        v.forEach(tuple -> {
+          Map<String, Object> groupRow = new LinkedHashMap<>(k.keys);
+          groupRow.put(tuple.getName(), tuple.value());
+          consumer.accept(groupRow);
+        });
+      } else {
+        Map<String, Object> row = new LinkedHashMap<>(k.keys);
+        v.forEach(val -> row.put(val.getName(), val.value()));
+        consumer.accept(row);
+      }
     });
   }
 
@@ -178,7 +185,7 @@ final class ElasticsearchJson {
       Bucket bucket = (Bucket) aggregation;
       if (bucket.hasNoAggregations()) {
         // bucket with no aggregations is also considered a leaf node
-        visitValueNodes(MultiValue.of(bucket.getName(), bucket.key()), parents, consumer);
+        visitValueNodes(GroupValue.of(bucket.getName(), bucket.key()), parents, consumer);
         return;
       }
       parents.add(bucket);
@@ -561,13 +568,23 @@ final class ElasticsearchJson {
       return values().get("value");
     }
 
+  }
+
+  /**
+   * Distinguishes from {@link MultiValue}.
+   * In order that rows which have the same key can be put into result map.
+   */
+  static class GroupValue extends MultiValue {
+    GroupValue(String name, Map<String, Object> values) {
+      super(name, values);
+    }
+
     /**
-     * Constructs a {@link MultiValue} instance with a single value.
+     * Constructs a {@link GroupValue} instance with a single value.
      */
-    static MultiValue of(String name, Object value) {
-      return new MultiValue(name, Collections.singletonMap("value", value));
+    static GroupValue of(String name, Object value) {
+      return new GroupValue(name, Collections.singletonMap("value", value));
     }
-
   }
 
   /**
@@ -575,8 +592,9 @@ final class ElasticsearchJson {
    */
   static class AggregationsDeserializer extends StdDeserializer<Aggregations> {
 
-    private static final Set<String> IGNORE_TOKENS = new HashSet<>(Arrays.asList("meta",
-        "buckets", "value", "values", "value_as_string", "doc_count", "key", "key_as_string"));
+    private static final Set<String> IGNORE_TOKENS =
+        ImmutableSet.of("meta", "buckets", "value", "values", "value_as_string",
+            "doc_count", "key", "key_as_string");
 
     AggregationsDeserializer() {
       super(Aggregations.class);

http://git-wip-us.apache.org/repos/asf/calcite/blob/96605a86/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTable.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTable.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTable.java
index b009fff..e288a16 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTable.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTable.java
@@ -53,6 +53,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
@@ -122,7 +123,7 @@ public class ElasticsearchTable extends AbstractQueryableTable implements Transl
       List<Map.Entry<String, String>> aggregations,
       Long offset, Long fetch) throws IOException {
 
-    if (!aggregations.isEmpty()) {
+    if (!aggregations.isEmpty() || !groupBy.isEmpty()) {
       // process aggregations separately
       return aggregate(ops, fields, sort, groupBy, aggregations, offset, fetch);
     }
@@ -171,10 +172,6 @@ public class ElasticsearchTable extends AbstractQueryableTable implements Transl
       List<Map.Entry<String, String>> aggregations,
       Long offset, Long fetch) throws IOException {
 
-    if (aggregations.isEmpty()) {
-      throw new IllegalArgumentException("Missing Aggregations");
-    }
-
     if (!groupBy.isEmpty() && offset != null) {
       String message = "Currently ES doesn't support generic pagination "
           + "with aggregations. You can still use LIMIT keyword (without OFFSET). "
@@ -245,12 +242,23 @@ public class ElasticsearchTable extends AbstractQueryableTable implements Transl
       }
     }
 
+    final Consumer<JsonNode> emptyAggRemover = new Consumer<JsonNode>() {
+      @Override public void accept(JsonNode node) {
+        if (!node.has(AGGREGATIONS)) {
+          node.elements().forEachRemaining(this);
+          return;
+        }
+        JsonNode agg = node.get(AGGREGATIONS);
+        if (agg.size() == 0) {
+          ((ObjectNode) node).remove(AGGREGATIONS);
+        } else {
+          this.accept(agg);
+        }
+      }
+    };
+
     // cleanup query. remove empty AGGREGATIONS element (if empty)
-    JsonNode agg = query;
-    while (agg.has(AGGREGATIONS) && agg.get(AGGREGATIONS).elements().hasNext()) {
-      agg = agg.get(AGGREGATIONS);
-    }
-    ((ObjectNode) agg).remove(AGGREGATIONS);
+    emptyAggRemover.accept(query);
 
     ElasticsearchJson.Result res = transport.search(Collections.emptyMap()).apply(query);
 
@@ -258,6 +266,7 @@ public class ElasticsearchTable extends AbstractQueryableTable implements Transl
     if (res.aggregations() != null) {
       // collect values
       ElasticsearchJson.visitValueNodes(res.aggregations(), m -> {
+        // using 'Collectors.toMap' will trigger Java 8 bug here
         Map<String, Object> newMap = new LinkedHashMap<>();
         for (String key: m.keySet()) {
           newMap.put(fieldMap.getOrDefault(key, key), m.get(key));

http://git-wip-us.apache.org/repos/asf/calcite/blob/96605a86/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticSearchAdapterTest.java
----------------------------------------------------------------------
diff --git a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticSearchAdapterTest.java b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticSearchAdapterTest.java
index 73cf6bf..8b09b88 100644
--- a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticSearchAdapterTest.java
+++ b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticSearchAdapterTest.java
@@ -458,62 +458,108 @@ public class ElasticSearchAdapterTest {
 
   @Test
   public void groupBy() {
+    // distinct
+    calciteAssert()
+        .query("select distinct state\n"
+            + "from zips\n"
+            + "limit 6")
+        .queryContains(
+            ElasticsearchChecker.elasticsearchChecker("_source:false",
+                "size:0",
+                "aggregations:{'g_state':{'terms':{'field':'state','missing':'__MISSING__', 'size' : 6}}}"))
+        .returnsOrdered("state=AK",
+            "state=AL",
+            "state=AR",
+            "state=AZ",
+            "state=CA",
+            "state=CO");
+
+    // without aggregate function
+    calciteAssert()
+        .query("select state, city\n"
+            + "from zips\n"
+            + "group by state, city\n"
+            + "order by city limit 10")
+        .queryContains(
+            ElasticsearchChecker.elasticsearchChecker("'_source':false",
+                "size:0",
+                "aggregations:{'g_city':{'terms':{'field':'city','missing':'__MISSING__','size':10,'order':{'_key':'asc'}}",
+                "aggregations:{'g_state':{'terms':{'field':'state','missing':'__MISSING__','size':10}}}}}}"))
+        .returnsOrdered("state=SD; city=ABERDEEN",
+            "state=SC; city=AIKEN",
+            "state=TX; city=ALTON",
+            "state=IA; city=AMES",
+            "state=AK; city=ANCHORAGE",
+            "state=MD; city=BALTIMORE",
+            "state=ME; city=BANGOR",
+            "state=KS; city=BAVARIA",
+            "state=NJ; city=BAYONNE",
+            "state=OR; city=BEAVERTON");
+
     // ascending
     calciteAssert()
-        .query("select min(pop), max(pop), state from zips group by state "
+        .query("select min(pop), max(pop), state\n"
+            + "from zips\n"
+            + "group by state\n"
             + "order by state limit 3")
         .queryContains(
             ElasticsearchChecker.elasticsearchChecker("'_source':false",
-            "size:0",
-            "aggregations:{'g_state':{terms:{field:'state',missing:'__MISSING__',size:3,"
-                + " order:{'_key':'asc'}}",
-            "aggregations:{'EXPR$0':{min:{field:'pop'}},'EXPR$1':{max:{field:'pop'}}}}}"))
+                "size:0",
+                "aggregations:{'g_state':{terms:{field:'state',missing:'__MISSING__',size:3,"
+                    + " order:{'_key':'asc'}}",
+                "aggregations:{'EXPR$0':{min:{field:'pop'}},'EXPR$1':{max:{field:'pop'}}}}}"))
         .returnsOrdered("EXPR$0=23238; EXPR$1=32383; state=AK",
             "EXPR$0=42124; EXPR$1=44165; state=AL",
             "EXPR$0=37428; EXPR$1=53532; state=AR");
 
     // just one aggregation function
     calciteAssert()
-        .query("select min(pop), state from zips group by state"
-            + " order by state limit 3")
+        .query("select min(pop), state\n"
+            + "from zips\n"
+            + "group by state\n"
+            + "order by state limit 3")
         .queryContains(
             ElasticsearchChecker.elasticsearchChecker("'_source':false",
-            "size:0",
-            "aggregations:{'g_state':{terms:{field:'state',missing:'__MISSING__',"
-                + "size:3, order:{'_key':'asc'}}",
-            "aggregations:{'EXPR$0':{min:{field:'pop'}} }}}"))
+                "size:0",
+                "aggregations:{'g_state':{terms:{field:'state',missing:'__MISSING__',"
+                    + "size:3, order:{'_key':'asc'}}",
+                "aggregations:{'EXPR$0':{min:{field:'pop'}} }}}"))
         .returnsOrdered("EXPR$0=23238; state=AK",
             "EXPR$0=42124; state=AL",
             "EXPR$0=37428; state=AR");
 
     // group by count
     calciteAssert()
-        .query("select count(city), state from zips group by state "
+        .query("select count(city), state\n"
+            + "from zips\n"
+            + "group by state\n"
             + "order by state limit 3")
         .queryContains(
             ElasticsearchChecker.elasticsearchChecker("'_source':false",
-            "size:0",
-            "aggregations:{'g_state':{terms:{field:'state',missing:'__MISSING__',"
-                + " size:3, order:{'_key':'asc'}}",
-            "aggregations:{'EXPR$0':{'value_count':{field:'city'}} }}}"))
+                "size:0",
+                "aggregations:{'g_state':{terms:{field:'state',missing:'__MISSING__',"
+                    + " size:3, order:{'_key':'asc'}}",
+                "aggregations:{'EXPR$0':{'value_count':{field:'city'}} }}}"))
         .returnsOrdered("EXPR$0=3; state=AK",
             "EXPR$0=3; state=AL",
             "EXPR$0=3; state=AR");
 
     // descending
     calciteAssert()
-        .query("select min(pop), max(pop), state from zips group by state "
-            + " order by state desc limit 3")
+        .query("select min(pop), max(pop), state\n"
+            + "from zips\n"
+            + "group by state\n"
+            + "order by state desc limit 3")
         .queryContains(
             ElasticsearchChecker.elasticsearchChecker("'_source':false",
-            "size:0",
-            "aggregations:{'g_state':{terms:{field:'state',missing:'__MISSING__',"
-                + "size:3, order:{'_key':'desc'}}",
-            "aggregations:{'EXPR$0':{min:{field:'pop'}},'EXPR$1':"
-                + "{max:{field:'pop'}}}}}"))
+                "size:0",
+                "aggregations:{'g_state':{terms:{field:'state',missing:'__MISSING__',"
+                    + "size:3, order:{'_key':'desc'}}",
+                "aggregations:{'EXPR$0':{min:{field:'pop'}},'EXPR$1':"
+                    + "{max:{field:'pop'}}}}}"))
         .returnsOrdered("EXPR$0=25968; EXPR$1=33107; state=WY",
-                        "EXPR$0=45196; EXPR$1=70185; state=WV",
-                        "EXPR$0=51008; EXPR$1=57187; state=WI");
+            "EXPR$0=45196; EXPR$1=70185; state=WV",
+            "EXPR$0=51008; EXPR$1=57187; state=WI");
   }
 
   /**


[10/15] calcite git commit: [CALCITE-2720] RelMetadataQuery.getTableOrigin throws IndexOutOfBoundsException if RelNode has no columns (Zoltan Haindrich)

Posted by jh...@apache.org.
[CALCITE-2720] RelMetadataQuery.getTableOrigin throws IndexOutOfBoundsException if RelNode has no columns (Zoltan Haindrich)

An example is an Aggregate with empty groupKey an no aggregate
functions. Occurs during LoptOptimizeJoinRule.

Close apache/calcite#949


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/7f8556e6
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/7f8556e6
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/7f8556e6

Branch: refs/heads/master
Commit: 7f8556e6520ea692550c9540559e548126208dad
Parents: de9a716
Author: Zoltan Haindrich <ki...@rxd.hu>
Authored: Fri Nov 30 15:49:15 2018 +0100
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Nov 30 21:05:45 2018 -0800

----------------------------------------------------------------------
 .../calcite/rel/metadata/RelMetadataQuery.java       |  3 +++
 .../org/apache/calcite/test/RelMetadataTest.java     | 15 +++++++++++++++
 2 files changed, 18 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/7f8556e6/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQuery.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQuery.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQuery.java
index d8f3dd2..e1c37e6 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQuery.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQuery.java
@@ -412,6 +412,9 @@ public class RelMetadataQuery {
     // Determine the simple origin of the first column in the
     // RelNode.  If it's simple, then that means that the underlying
     // table is also simple, even if the column itself is derived.
+    if (rel.getRowType().getFieldCount() == 0) {
+      return null;
+    }
     final Set<RelColumnOrigin> colOrigins = getColumnOrigins(rel, 0);
     if (colOrigins == null || colOrigins.size() == 0) {
       return null;

http://git-wip-us.apache.org/repos/asf/calcite/blob/7f8556e6/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
index 4ee5881..4628875 100644
--- a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
@@ -87,6 +87,7 @@ import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.tools.FrameworkConfig;
 import org.apache.calcite.tools.Frameworks;
 import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilder.AggCall;
 import org.apache.calcite.util.BuiltInMethod;
 import org.apache.calcite.util.ImmutableBitSet;
 import org.apache.calcite.util.ImmutableIntList;
@@ -2385,6 +2386,20 @@ public class RelMetadataTest extends SqlToRelTestBase {
     }
   };
 
+  /** Tests calling {@link RelMetadataQuery#getTableOrigin} for
+   * an aggregate with no columns. Previously threw. */
+  @Test public void testEmptyAggregateTableOrigin() {
+    final FrameworkConfig config = RelBuilderTest.config().build();
+    final RelBuilder builder = RelBuilder.create(config);
+    RelMetadataQuery mq = RelMetadataQuery.instance();
+    RelNode agg = builder
+        .scan("EMP")
+        .aggregate(builder.groupKey())
+        .build();
+    final RelOptTable tableOrigin = mq.getTableOrigin(agg);
+    assertThat(tableOrigin, nullValue());
+  }
+
   @Test public void testGetPredicatesForJoin() throws Exception {
     final FrameworkConfig config = RelBuilderTest.config().build();
     final RelBuilder builder = RelBuilder.create(config);


[15/15] calcite git commit: [CALCITE-2699] TIMESTAMPADD function now applies to DATE and TIME as well as TIMESTAMP (xuqianjin)

Posted by jh...@apache.org.
[CALCITE-2699] TIMESTAMPADD function now applies to DATE and TIME as well as TIMESTAMP (xuqianjin)

Close apache/calcite#936


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/18caf38f
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/18caf38f
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/18caf38f

Branch: refs/heads/master
Commit: 18caf38f83802015043eb06ad8aa49d1efe7ff68
Parents: fbf193b
Author: xuqianjin <x1...@163.com>
Authored: Fri Nov 23 22:15:20 2018 +0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Sat Dec 1 14:43:59 2018 -0800

----------------------------------------------------------------------
 .../calcite/adapter/enumerable/RexImpTable.java | 15 +++++---
 .../sql/fun/SqlTimestampAddFunction.java        | 12 ++++---
 .../calcite/sql/test/SqlOperatorBaseTest.java   | 37 ++++++++++++++++++++
 site/_docs/reference.md                         |  2 +-
 4 files changed, 56 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/18caf38f/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
index ada4333..7776ac9 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
@@ -2617,11 +2617,16 @@ public class RexImpTable {
         case MINUS:
           trop1 = Expressions.negate(trop1);
         }
-        final BuiltInMethod method =
-            operand0.getType().getSqlTypeName() == SqlTypeName.TIMESTAMP
-                ? BuiltInMethod.ADD_MONTHS
-                : BuiltInMethod.ADD_MONTHS_INT;
-        return Expressions.call(method.method, trop0, trop1);
+        switch (typeName) {
+        case TIME:
+          return Expressions.convert_(trop0, long.class);
+        default:
+          final BuiltInMethod method =
+              operand0.getType().getSqlTypeName() == SqlTypeName.TIMESTAMP
+                  ? BuiltInMethod.ADD_MONTHS
+                  : BuiltInMethod.ADD_MONTHS_INT;
+          return Expressions.call(method.method, trop0, trop1);
+        }
 
       case INTERVAL_DAY:
       case INTERVAL_DAY_HOUR:

http://git-wip-us.apache.org/repos/asf/calcite/blob/18caf38f/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampAddFunction.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampAddFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampAddFunction.java
index bc459d0..72b00f0 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampAddFunction.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampAddFunction.java
@@ -29,13 +29,13 @@ import org.apache.calcite.sql.type.SqlTypeName;
 
 /**
  * The <code>TIMESTAMPADD</code> function, which adds an interval to a
- * timestamp.
+ * datetime (TIMESTAMP, TIME or DATE).
  *
  * <p>The SQL syntax is
  *
  * <blockquote>
  * <code>TIMESTAMPADD(<i>timestamp interval</i>, <i>quantity</i>,
- * <i>timestamp</i>)</code>
+ * <i>datetime</i>)</code>
  * </blockquote>
  *
  * <p>The interval time unit can one of the following literals:<ul>
@@ -51,7 +51,7 @@ import org.apache.calcite.sql.type.SqlTypeName;
  * <li>YEAR (and synonym  SQL_TSI_YEAR)
  * </ul>
  *
- * <p>Returns modified timestamp.
+ * <p>Returns modified datetime.
  */
 public class SqlTimestampAddFunction extends SqlFunction {
 
@@ -85,7 +85,11 @@ public class SqlTimestampAddFunction extends SqlFunction {
             MICROSECOND_PRECISION);
         break;
       default:
-        type = typeFactory.createSqlType(SqlTypeName.TIMESTAMP);
+        if (operandType2.getSqlTypeName() == SqlTypeName.TIME) {
+          type = typeFactory.createSqlType(SqlTypeName.TIME);
+        } else {
+          type = typeFactory.createSqlType(SqlTypeName.TIMESTAMP);
+        }
       }
       break;
     default:

http://git-wip-us.apache.org/repos/asf/calcite/blob/18caf38f/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
index 78a2fb7..1a1b750 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
@@ -6957,6 +6957,43 @@ public abstract class SqlOperatorBaseTest {
         "2016-06-30", "DATE NOT NULL");
     tester.checkScalar("timestampadd(MONTH, -1, date '2016-03-31')",
         "2016-02-29", "DATE NOT NULL");
+
+    // TIMESTAMPADD with time; returns a time value.The interval is positive.
+    tester.checkScalar("timestampadd(SECOND, 1, time '23:59:59')",
+        "00:00:00", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(MINUTE, 1, time '00:00:00')",
+        "00:01:00", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(MINUTE, 1, time '23:59:59')",
+        "00:00:59", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(HOUR, 1, time '23:59:59')",
+        "00:59:59", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(DAY, 15, time '23:59:59')",
+        "23:59:59", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(WEEK, 3, time '23:59:59')",
+        "23:59:59", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(MONTH, 6, time '23:59:59')",
+        "23:59:59", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(QUARTER, 1, time '23:59:59')",
+        "23:59:59", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(YEAR, 10, time '23:59:59')",
+        "23:59:59", "TIME(0) NOT NULL");
+    // TIMESTAMPADD with time; returns a time value .The interval is negative.
+    tester.checkScalar("timestampadd(SECOND, -1, time '00:00:00')",
+        "23:59:59", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(MINUTE, -1, time '00:00:00')",
+        "23:59:00", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(HOUR, -1, time '00:00:00')",
+        "23:00:00", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(DAY, -1, time '23:59:59')",
+        "23:59:59", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(WEEK, -1, time '23:59:59')",
+        "23:59:59", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(MONTH, -1, time '23:59:59')",
+        "23:59:59", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(QUARTER, -1, time '23:59:59')",
+        "23:59:59", "TIME(0) NOT NULL");
+    tester.checkScalar("timestampadd(YEAR, -1, time '23:59:59')",
+        "23:59:59", "TIME(0) NOT NULL");
   }
 
   @Test public void testTimestampAddFractionalSeconds() {

http://git-wip-us.apache.org/repos/asf/calcite/blob/18caf38f/site/_docs/reference.md
----------------------------------------------------------------------
diff --git a/site/_docs/reference.md b/site/_docs/reference.md
index e43e255..ab2f4f3 100644
--- a/site/_docs/reference.md
+++ b/site/_docs/reference.md
@@ -1473,7 +1473,7 @@ Not implemented:
 | {fn HOUR(date)} | Equivalent to `EXTRACT(HOUR FROM date)`. Returns an integer between 0 and 23.
 | {fn MINUTE(date)} | Equivalent to `EXTRACT(MINUTE FROM date)`. Returns an integer between 0 and 59.
 | {fn SECOND(date)} | Equivalent to `EXTRACT(SECOND FROM date)`. Returns an integer between 0 and 59.
-| {fn TIMESTAMPADD(timeUnit, count, timestamp)} | Adds an interval of *count* *timeUnit*s to a timestamp
+| {fn TIMESTAMPADD(timeUnit, count, datetime)} | Adds an interval of *count* *timeUnit*s to a datetime
 | {fn TIMESTAMPDIFF(timeUnit, timestamp1, timestamp2)} | Subtracts *timestamp1* from *timestamp2* and returns the result in *timeUnit*s
 
 Not implemented: