Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

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 (#377)

* Bug fix, return object type for field which has implicit object datatype when describe the table
  • Loading branch information
penghuo authored Mar 12, 2020
1 parent 56a2f44 commit c30e342
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 c30e342

Please sign in to comment.