Sql test bench bug fix, improvement and more test cases (#343)

* Fix column type order issue in schema

* Use column label (alias) if present

* Handle date time and float

* Add more test cases

src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/runner/ComparisonTest.java

@@ -26,6 +26,7 @@
 import com.amazon.opendistroforelasticsearch.sql.correctness.testset.TestQuerySet;
 import com.amazon.opendistroforelasticsearch.sql.utils.StringUtils;
 import com.google.common.collect.Iterators;
+import com.google.common.collect.Lists;
 
 import java.util.Arrays;
 import java.util.Comparator;
@@ -118,6 +119,7 @@ public void close() {
 
     /** Execute the query and compare with current result */
     private TestCaseReport compareWithOtherDb(String sql, DBResult esResult) {
+        List<DBResult> mismatchResults = Lists.newArrayList(esResult);
         StringBuilder reasons = new StringBuilder();
         for (int i = 0; i < otherDbConnections.length; i++) {
             try {
@@ -126,9 +128,11 @@ private TestCaseReport compareWithOtherDb(String sql, DBResult esResult) {
                     return new SuccessTestCase(nextId(), sql);
                 }
 
+                mismatchResults.add(otherDbResult);
+
                 // Cannot find any database result match
                 if (i == otherDbConnections.length - 1) {
-                    return new FailedTestCase(nextId(), sql, Arrays.asList(esResult, otherDbResult));
+                    return new FailedTestCase(nextId(), sql, mismatchResults);
                 }
             } catch (Exception e) {
                 // Ignore and move on to next database

src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/runner/connection/JDBCConnection.java

@@ -18,6 +18,7 @@
 import com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset.DBResult;
 import com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset.Row;
 import com.amazon.opendistroforelasticsearch.sql.utils.StringUtils;
+import com.google.common.base.Strings;
 import org.json.JSONObject;
 
 import java.sql.Connection;
@@ -140,7 +141,13 @@ private String getValueList(String[] fieldValues) {
     private void populateMetaData(ResultSet resultSet, DBResult result) throws SQLException {
         ResultSetMetaData metaData = resultSet.getMetaData();
         for (int i = 1; i <= metaData.getColumnCount(); i++) {
-            result.addColumn(metaData.getColumnName(i), metaData.getColumnTypeName(i));
+
+            // Use label name (alias) if present.
+            String colName = metaData.getColumnLabel(i);
+            if (Strings.isNullOrEmpty(colName)) {
+                colName = metaData.getColumnName(i);
+            }
+            result.addColumn(colName, metaData.getColumnTypeName(i));
         }
     }
 

src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/runner/resultset/DBResult.java

@@ -16,14 +16,16 @@
 package com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset;
 
 import com.amazon.opendistroforelasticsearch.sql.utils.StringUtils;
+import com.google.common.collect.ImmutableSet;
 import lombok.EqualsAndHashCode;
+import lombok.Getter;
 import lombok.ToString;
 import org.json.JSONPropertyName;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
-import java.util.LinkedHashMap;
-import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 /**
@@ -35,11 +37,15 @@
 @ToString
 public class DBResult {
 
+    /** Possible types for floating point number */
+    private static final Set<String> FLOAT_TYPES = ImmutableSet.of("FLOAT", "DOUBLE", "REAL");
+
     /** Database name for display */
     private final String databaseName;
 
     /** Column name and types from result set meta data */
-    private final Map<String, String> colNameAndTypes;
+    @Getter
+    private final Collection<Type> schema;
 
     /** Data rows from result set */
     private final Collection<Row> dataRows;
@@ -49,21 +55,27 @@ public class DBResult {
      * with specific column names in SELECT but without ORDER BY.
      */
     public DBResult(String databaseName) {
-        this(databaseName, new LinkedHashMap<>(), new HashSet<>());
+        this(databaseName, new ArrayList<>(), new HashSet<>());
     }
 
-    public DBResult(String databaseName, Map<String, String> colNameAndTypes, Collection<Row> rows) {
+    public DBResult(String databaseName, Collection<Type> schema, Collection<Row> rows) {
         this.databaseName = databaseName;
-        this.colNameAndTypes = colNameAndTypes;
+        this.schema = schema;
         this.dataRows = rows;
     }
 
     public int columnSize() {
-        return colNameAndTypes.size();
+        return schema.size();
     }
 
     public void addColumn(String name, String type) {
-        colNameAndTypes.put(StringUtils.toUpper(name), StringUtils.toUpper(type));
+        type = StringUtils.toUpper(type);
+
+        // Ignore float type by assigning all type names string to it.
+        if (FLOAT_TYPES.contains(type)) {
+            type = FLOAT_TYPES.toString();
+        }
+        schema.add(new Type(StringUtils.toUpper(name), type));
     }
 
     public void addRow(Row row) {
@@ -75,11 +87,6 @@ public String getDatabaseName() {
         return databaseName;
     }
 
-    @JSONPropertyName("schema")
-    public Map<String, String> getColumnNameAndTypes() {
-        return colNameAndTypes;
-    }
-
     /** Flatten for simplifying json generated */
     public Collection<Collection<Object>> getDataRows() {
         return dataRows.stream().map(Row::getValues).collect(Collectors.toSet());

src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/runner/resultset/Row.java

@@ -49,7 +49,7 @@ public void add(Object value) {
     private Object roundFloatNum(Object value) {
         if (value instanceof Float) {
             BigDecimal decimal = BigDecimal.valueOf((Float) value).setScale(2, RoundingMode.CEILING);
-            value = decimal.floatValue();
+            value = decimal.doubleValue(); // Convert to double too
         } else if (value instanceof Double) {
             BigDecimal decimal = BigDecimal.valueOf((Double) value).setScale(2, RoundingMode.CEILING);
             value = decimal.doubleValue();

src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/runner/resultset/Type.java

@@ -0,0 +1,32 @@
+/*
+ *   Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License").
+ *   You may not use this file except in compliance with the License.
+ *   A copy of the License is located at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   or in the "license" file accompanying this file. This file is distributed
+ *   on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *   express or implied. See the License for the specific language governing
+ *   permissions and limitations under the License.
+ */
+
+package com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset;
+
+import lombok.Data;
+
+/**
+ * Column type in schema
+ */
+@Data
+public class Type {
+
+    /** Column name */
+    private final String name;
+
+    /** Column type */
+    private final String type;
+
+}

src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/tests/ComparisonTestTest.java

@@ -23,8 +23,8 @@
 import com.amazon.opendistroforelasticsearch.sql.correctness.runner.connection.DBConnection;
 import com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset.DBResult;
 import com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset.Row;
+import com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset.Type;
 import com.amazon.opendistroforelasticsearch.sql.correctness.testset.TestQuerySet;
-import com.google.common.collect.ImmutableMap;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -63,10 +63,10 @@ public void setUp() {
     @Test
     public void testSuccess() {
         when(esConnection.select(anyString())).thenReturn(
-            new DBResult("ES", ImmutableMap.of("firstname", "text"), asList(new Row(asList("John"))))
+            new DBResult("ES", asList(new Type("firstname", "text")), asList(new Row(asList("John"))))
         );
         when(otherDbConnection.select(anyString())).thenReturn(
-            new DBResult("Other DB", ImmutableMap.of("firstname", "text"), asList(new Row(asList("John"))))
+            new DBResult("Other DB", asList(new Type("firstname", "text")), asList(new Row(asList("John"))))
         );
 
         TestReport expected = new TestReport();
@@ -77,8 +77,8 @@ public void testSuccess() {
 
     @Test
     public void testFailureDueToInconsistency() {
-        DBResult esResult = new DBResult("ES", ImmutableMap.of("firstname", "text"), asList(new Row(asList("John"))));
-        DBResult otherDbResult = new DBResult("Other DB", ImmutableMap.of("firstname", "text"), asList(new Row(asList("JOHN"))));
+        DBResult esResult = new DBResult("ES", asList(new Type("firstname", "text")), asList(new Row(asList("John"))));
+        DBResult otherDbResult = new DBResult("Other DB", asList(new Type("firstname", "text")), asList(new Row(asList("JOHN"))));
         when(esConnection.select(anyString())).thenReturn(esResult);
         when(otherDbConnection.select(anyString())).thenReturn(otherDbResult);
 
@@ -96,9 +96,9 @@ public void testSuccessFinally() {
             esConnection, new DBConnection[]{otherDbConnection, anotherDbConnection}
         );
 
-        DBResult esResult = new DBResult("ES", ImmutableMap.of("firstname", "text"), asList(new Row(asList("John"))));
-        DBResult otherDbResult = new DBResult("Other DB", ImmutableMap.of("firstname", "text"), asList(new Row(asList("JOHN"))));
-        DBResult anotherDbResult = new DBResult("Another DB", ImmutableMap.of("firstname", "text"), asList(new Row(asList("John"))));
+        DBResult esResult = new DBResult("ES", asList(new Type("firstname", "text")), asList(new Row(asList("John"))));
+        DBResult otherDbResult = new DBResult("Other DB", asList(new Type("firstname", "text")), asList(new Row(asList("JOHN"))));
+        DBResult anotherDbResult = new DBResult("Another DB", asList(new Type("firstname", "text")), asList(new Row(asList("John"))));
         when(esConnection.select(anyString())).thenReturn(esResult);
         when(otherDbConnection.select(anyString())).thenReturn(otherDbResult);
         when(anotherDbConnection.select(anyString())).thenReturn(anotherDbResult);
@@ -117,15 +117,15 @@ public void testFailureDueToEventualInconsistency() {
             esConnection, new DBConnection[]{otherDbConnection, anotherDbConnection}
         );
 
-        DBResult esResult = new DBResult("ES", ImmutableMap.of("firstname", "text"), asList(new Row(asList("John"))));
-        DBResult otherDbResult = new DBResult("Other DB", ImmutableMap.of("firstname", "text"), asList(new Row(asList("JOHN"))));
-        DBResult anotherDbResult = new DBResult("ZZZ DB", ImmutableMap.of("firstname", "text"), asList(new Row(asList("Hank"))));
+        DBResult esResult = new DBResult("ES", asList(new Type("firstname", "text")), asList(new Row(asList("John"))));
+        DBResult otherDbResult = new DBResult("Other DB", asList(new Type("firstname", "text")), asList(new Row(asList("JOHN"))));
+        DBResult anotherDbResult = new DBResult("ZZZ DB", asList(new Type("firstname", "text")), asList(new Row(asList("Hank"))));
         when(esConnection.select(anyString())).thenReturn(esResult);
         when(otherDbConnection.select(anyString())).thenReturn(otherDbResult);
         when(anotherDbConnection.select(anyString())).thenReturn(anotherDbResult);
 
         TestReport expected = new TestReport();
-        expected.addTestCase(new FailedTestCase(1, "SELECT * FROM accounts", asList(esResult, anotherDbResult)));
+        expected.addTestCase(new FailedTestCase(1, "SELECT * FROM accounts", asList(esResult, otherDbResult, anotherDbResult)));
         TestReport actual = correctnessTest.verify(querySet("SELECT * FROM accounts"));
         assertEquals(expected, actual);
     }
@@ -143,7 +143,7 @@ public void testErrorDueToESException() {
     @Test
     public void testErrorDueToNoOtherDBSupportThisQuery() {
         when(esConnection.select(anyString())).thenReturn(
-            new DBResult("ES", ImmutableMap.of("firstname", "text"), asList(new Row(asList("John"))))
+            new DBResult("ES", asList(new Type("firstname", "text")), asList(new Row(asList("John"))))
         );
         when(otherDbConnection.select(anyString())).thenThrow(new RuntimeException("Unsupported feature"));
 
@@ -162,11 +162,11 @@ public void testSuccessWhenOneDBSupportThisQuery() {
         );
 
         when(esConnection.select(anyString())).thenReturn(
-            new DBResult("ES", ImmutableMap.of("firstname", "text"), asList(new Row(asList("John"))))
+            new DBResult("ES", asList(new Type("firstname", "text")), asList(new Row(asList("John"))))
         );
         when(otherDbConnection.select(anyString())).thenThrow(new RuntimeException("Unsupported feature"));
         when(anotherDbConnection.select(anyString())).thenReturn(
-            new DBResult("Another DB", ImmutableMap.of("firstname", "text"), asList(new Row(asList("John"))))
+            new DBResult("Another DB", asList(new Type("firstname", "text")), asList(new Row(asList("John"))))
         );
 
         TestReport expected = new TestReport();

src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/tests/DBResultTest.java

@@ -16,9 +16,11 @@
 package com.amazon.opendistroforelasticsearch.sql.correctness.tests;
 
 import com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset.DBResult;
-import com.google.common.collect.ImmutableMap;
+import com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset.Type;
 import org.junit.Test;
 
+import java.util.Arrays;
+
 import static java.util.Collections.emptyList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
@@ -30,22 +32,22 @@ public class DBResultTest {
 
     @Test
     public void dbResultFromDifferentDbNameShouldEqual() {
-        DBResult result1 = new DBResult("DB 1", ImmutableMap.of("name", "VARCHAR"), emptyList());
-        DBResult result2 = new DBResult("DB 2", ImmutableMap.of("name", "VARCHAR"), emptyList());
+        DBResult result1 = new DBResult("DB 1", Arrays.asList(new Type("name", "VARCHAR")), emptyList());
+        DBResult result2 = new DBResult("DB 2", Arrays.asList(new Type("name", "VARCHAR")), emptyList());
         assertEquals(result1, result2);
     }
 
     @Test
     public void dbResultWithDifferentColumnShouldNotEqual() {
-        DBResult result1 = new DBResult("DB 1", ImmutableMap.of("name", "VARCHAR"), emptyList());
-        DBResult result2 = new DBResult("DB 2", ImmutableMap.of("age", "INT"), emptyList());
+        DBResult result1 = new DBResult("DB 1", Arrays.asList(new Type("name", "VARCHAR")), emptyList());
+        DBResult result2 = new DBResult("DB 2", Arrays.asList(new Type("age", "INT")), emptyList());
         assertNotEquals(result1, result2);
     }
 
     @Test
     public void dbResultWithDifferentColumnTypeShouldNotEqual() {
-        DBResult result1 = new DBResult("DB 1", ImmutableMap.of("age", "FLOAT"), emptyList());
-        DBResult result2 = new DBResult("DB 2", ImmutableMap.of("age", "INT"), emptyList());
+        DBResult result1 = new DBResult("DB 1", Arrays.asList(new Type("age", "FLOAT")), emptyList());
+        DBResult result2 = new DBResult("DB 2", Arrays.asList(new Type("age", "INT")), emptyList());
         assertNotEquals(result1, result2);
     }
 

src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/tests/JDBCConnectionTest.java

@@ -17,7 +17,7 @@
 
 import com.amazon.opendistroforelasticsearch.sql.correctness.runner.connection.JDBCConnection;
 import com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset.DBResult;
-import com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset.Row;
+import com.amazon.opendistroforelasticsearch.sql.correctness.runner.resultset.Type;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Sets;
 import org.junit.Before;
@@ -116,8 +116,11 @@ public void testSelectQuery() throws SQLException {
         DBResult result = conn.select("SELECT * FROM test");
         assertEquals("Test DB", result.getDatabaseName());
         assertEquals(
-            ImmutableMap.of("NAME", "VARCHAR", "AGE", "INT"),
-            result.getColumnNameAndTypes()
+            Arrays.asList(
+                new Type("NAME", "VARCHAR"),
+                new Type("AGE", "INT")
+            ),
+            result.getSchema()
         );
         assertEquals(
             Sets.newHashSet(
@@ -128,6 +131,23 @@ public void testSelectQuery() throws SQLException {
         );
     }
 
+    @Test
+    public void testSelectQueryWithAlias() throws SQLException {
+        ResultSetMetaData metaData = mockMetaData(ImmutableMap.of("name", "VARCHAR", "age", "INT"), "n", "a");
+        ResultSet resultSet = mockResultSet(new Object[]{"John", 25}, new Object[]{"Hank", 30});
+        when(statement.executeQuery(anyString())).thenReturn(resultSet);
+        when(resultSet.getMetaData()).thenReturn(metaData);
+
+        DBResult result = conn.select("SELECT * FROM test");
+        assertEquals(
+            Arrays.asList(
+                new Type("N", "VARCHAR"),
+                new Type("A", "INT")
+            ),
+            result.getSchema()
+        );
+    }
+
     @Test
     public void testSelectQueryWithFloatInResultSet() throws SQLException {
         ResultSetMetaData metaData = mockMetaData(ImmutableMap.of("name", "VARCHAR", "balance", "FLOAT"));
@@ -140,6 +160,13 @@ public void testSelectQueryWithFloatInResultSet() throws SQLException {
         when(resultSet.getMetaData()).thenReturn(metaData);
 
         DBResult result = conn.select("SELECT * FROM test");
+        assertEquals(
+            Arrays.asList(
+                new Type("NAME", "VARCHAR"),
+                new Type("BALANCE", "[FLOAT, DOUBLE, REAL]")
+            ),
+            result.getSchema()
+        );
         assertEquals(
             Sets.newHashSet(
                 Arrays.asList("John", 25.13),
@@ -167,7 +194,7 @@ private ResultSet mockResultSet(Object[]... rows) throws SQLException {
         return resultSet;
     }
 
-    private ResultSetMetaData mockMetaData(Map<String, String> nameAndTypes) throws SQLException {
+    private ResultSetMetaData mockMetaData(Map<String, String> nameAndTypes, String... aliases) throws SQLException {
         ResultSetMetaData metaData = mock(ResultSetMetaData.class);
 
         OngoingStubbing<String> getColumnName = when(metaData.getColumnName(anyInt()));
@@ -180,6 +207,13 @@ private ResultSetMetaData mockMetaData(Map<String, String> nameAndTypes) throws
             getColumnTypeName = getColumnTypeName.thenReturn(value);
         }
 
+        if (aliases.length > 0) {
+            OngoingStubbing<String> getColumnLabel = when(metaData.getColumnLabel(anyInt()));
+            for (String alias : aliases) {
+                getColumnLabel = getColumnLabel.thenReturn(alias);
+            }
+        }
+
         when(metaData.getColumnCount()).thenReturn(nameAndTypes.size());
         return metaData;
     }