From 71d2adbaefa9ffbecf71196ec1fd15ecef0686f2 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Thu, 12 Mar 2020 14:17:55 -0700 Subject: [PATCH] Bug fix, return object type for field which has implicit object datatype when describe the table (#377) * Bug fix, return object type for field which has implicit object datatype when describe the table (cherry picked from commit c30e342ac4b8f7f6a12b0efa5a01658cafceea04) --- .../executor/format/DescribeResultSet.java | 8 ++- .../sql/esintgtest/MetaDataQueriesIT.java | 51 +++++++++++++++++++ .../sql/util/MatcherUtils.java | 13 ++++- 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/DescribeResultSet.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/DescribeResultSet.java index 66a5e9f8b5..04044f8360 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/DescribeResultSet.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/DescribeResultSet.java @@ -36,6 +36,12 @@ public class DescribeResultSet extends ResultSet { private static final int DEFAULT_NUM_PREC_RADIX = 10; private static final String IS_AUTOINCREMENT = "NO"; + /** + * You are not required to set the field type to object explicitly, as this is the default value. + * https://www.elastic.co/guide/en/elasticsearch/reference/current/object.html + */ + private static final String DEFAULT_OBJECT_DATATYPE = "object"; + private IndexStatement statement; private Object queryResult; @@ -159,7 +165,7 @@ private Map flattenMappingMetaData(Map mappingMe Map metaData = (Map) entry.getValue(); String fullPath = addToPath(currPath, entry.getKey()); - flattenedMapping.put(fullPath, (String) metaData.get("type")); + flattenedMapping.put(fullPath, (String) metaData.getOrDefault("type", DEFAULT_OBJECT_DATATYPE)); if (metaData.containsKey("properties")) { flattenedMapping = flattenMappingMetaData(metaData, fullPath, flattenedMapping); } diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/MetaDataQueriesIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/MetaDataQueriesIT.java index cee43fcc06..221e306760 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/MetaDataQueriesIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/MetaDataQueriesIT.java @@ -16,14 +16,19 @@ package com.amazon.opendistroforelasticsearch.sql.esintgtest; import org.elasticsearch.client.Request; +import org.hamcrest.Description; +import org.hamcrest.TypeSafeMatcher; import org.json.JSONArray; import org.json.JSONObject; import org.junit.Test; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import static com.amazon.opendistroforelasticsearch.sql.esintgtest.TestsConstants.TEST_INDEX_GAME_OF_THRONES; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.verifySome; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasItems; @@ -290,6 +295,31 @@ public void describeSingleIndex() throws IOException { assertThat(row.get(5), not(equalTo(JSONObject.NULL))); } + @Test + public void describeSingleIndexWithObjectFieldShouldPass() throws IOException { + JSONObject response = + executeQuery(String.format("DESCRIBE TABLES LIKE %s", TEST_INDEX_GAME_OF_THRONES)); + + // Schema for DESCRIBE is filled with a lot of fields that aren't used so only the important + // ones are checked for here + String[] fields = {"TABLE_NAME", "COLUMN_NAME", "TYPE_NAME"}; + checkContainsColumns(getSchema(response), fields); + + JSONArray dataRows = getDataRows(response); + assertThat(dataRows.length(), greaterThan(0)); + assertThat(dataRows.getJSONArray(0).length(), equalTo(DESCRIBE_FIELD_LENGTH)); + + verifySome(dataRows, + describeRow(TEST_INDEX_GAME_OF_THRONES, "nickname", "text"), + describeRow(TEST_INDEX_GAME_OF_THRONES, "name", "object"), + describeRow(TEST_INDEX_GAME_OF_THRONES, "name.firstname", "text"), + describeRow(TEST_INDEX_GAME_OF_THRONES, "name.lastname", "text"), + describeRow(TEST_INDEX_GAME_OF_THRONES, "name.ofHerName", "integer"), + describeRow(TEST_INDEX_GAME_OF_THRONES, "name.ofHisName", "integer"), + describeRow(TEST_INDEX_GAME_OF_THRONES, "house", "text"), + describeRow(TEST_INDEX_GAME_OF_THRONES, "gender", "text")); + } + @Test public void describeCaseSensitivityCheck() throws IOException { JSONObject response = executeQuery(String.format("describe tables like %s", TestsConstants.TEST_INDEX_ACCOUNT)); @@ -374,4 +404,25 @@ private String getClusterName() throws IOException { String response = executeRequest(new Request("GET", "_cluster/health")); return new JSONObject(response).optString("cluster_name", ""); } + + public static TypeSafeMatcher describeRow(Object... expectedObjects) { + return new TypeSafeMatcher() { + @Override + public void describeTo(Description description) { + description.appendText(String.join(",", Arrays.asList(expectedObjects).toString())); + } + + @Override + protected boolean matchesSafely(JSONArray array) { + List actualObjects = new ArrayList<>(); + // TABLE_NAME + actualObjects.add(array.get(2)); + // COLUMN_NAME + actualObjects.add(array.get(3)); + // TYPE_NAME + actualObjects.add(array.get(5)); + return Arrays.asList(expectedObjects).equals(actualObjects); + } + }; + } } diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/util/MatcherUtils.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/util/MatcherUtils.java index 03a118797a..2f4d526b15 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/util/MatcherUtils.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/util/MatcherUtils.java @@ -34,9 +34,9 @@ import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.arrayContainingInAnyOrder; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.emptyArray; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasItems; import static org.junit.Assert.assertEquals; @@ -148,6 +148,17 @@ public static void verify(JSONArray array, Matcher... matchers) { assertThat(objects, containsInAnyOrder(matchers)); } + @SuppressWarnings("unchecked") + public static void verifySome(JSONArray array, Matcher... matchers) { + List objects = new ArrayList<>(); + array.iterator().forEachRemaining(o -> objects.add((T) o)); + + assertThat(matchers.length, greaterThan(0)); + for (Matcher matcher : matchers) { + assertThat(objects, hasItems(matcher)); + } + } + public static TypeSafeMatcher schema(String expectedName, String expectedAlias, String expectedType) { return new TypeSafeMatcher() { @Override