Skip to content

Commit

Permalink
Bug fix, return object type for field which has implicit object datat…
Browse files Browse the repository at this point in the history
…ype when describe the table (opendistro-for-elasticsearch#377)

* Bug fix, return object type for field which has implicit object datatype when describe the table

(cherry picked from commit c30e342)
  • Loading branch information
penghuo committed May 2, 2020
1 parent 5ea731a commit 71d2adb
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -159,7 +165,7 @@ private Map<String, String> flattenMappingMetaData(Map<String, Object> mappingMe
Map<String, Object> metaData = (Map<String, Object>) 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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<JSONArray> describeRow(Object... expectedObjects) {
return new TypeSafeMatcher<JSONArray>() {
@Override
public void describeTo(Description description) {
description.appendText(String.join(",", Arrays.asList(expectedObjects).toString()));
}

@Override
protected boolean matchesSafely(JSONArray array) {
List<Object> 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);
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -148,6 +148,17 @@ public static <T> void verify(JSONArray array, Matcher<T>... matchers) {
assertThat(objects, containsInAnyOrder(matchers));
}

@SuppressWarnings("unchecked")
public static <T> void verifySome(JSONArray array, Matcher<T>... matchers) {
List<T> objects = new ArrayList<>();
array.iterator().forEachRemaining(o -> objects.add((T) o));

assertThat(matchers.length, greaterThan(0));
for (Matcher<T> matcher : matchers) {
assertThat(objects, hasItems(matcher));
}
}

public static TypeSafeMatcher<JSONObject> schema(String expectedName, String expectedAlias, String expectedType) {
return new TypeSafeMatcher<JSONObject>() {
@Override
Expand Down

0 comments on commit 71d2adb

Please sign in to comment.