Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Array and ExprValue Parsing With Inner Hits #1737

Merged
merged 3 commits into from
Jun 27, 2023
+613 −112
Merged

Support Array and ExprValue Parsing With Inner Hits #1737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a70d5d
Add support for Array and ExprValue Parsing With Inner Hits
forestmvey Jun 14, 2023
3bbd105
Adding schema validation for IT test, and another UT for nested arrays.
forestmvey Jun 21, 2023
5d657c6
Making handleAggregationResponse a private function.
forestmvey Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion integ-test/src/test/java/org/opensearch/sql/legacy/ObjectFieldSelectIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

import org.json.JSONArray;
import org.json.JSONObject;
import org.junit.Assume;
import org.junit.Test;
import org.opensearch.sql.legacy.utils.StringUtils;

Expand Down
35 changes: 35 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package org.opensearch.sql.sql;

import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_MULTI_NESTED_TYPE;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_SIMPLE;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_TYPE;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_TYPE_WITHOUT_ARRAYS;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_WITH_NULLS;
Expand All @@ -31,6 +32,7 @@ public void init() throws IOException {
loadIndex(Index.NESTED_WITHOUT_ARRAYS);
loadIndex(Index.EMPLOYEE_NESTED);
loadIndex(Index.NESTED_WITH_NULLS);
loadIndex(Index.NESTED_SIMPLE);
}

@Test
Expand Down Expand Up @@ -366,4 +368,37 @@ public void test_nested_in_where_as_predicate_expression_with_relevance_query()
assertEquals(1, result.getInt("total"));
verifyDataRows(result, rows(10, "a"));
}

@Test
public void nested_function_with_date_types_as_object_arrays_within_arrays_test() {
String query = "SELECT nested(address.moveInDate) FROM " + TEST_INDEX_NESTED_SIMPLE;
JSONObject result = executeJdbcRequest(query);

assertEquals(11, result.getInt("total"));
verifySchema(result,
schema("nested(address.moveInDate)", null, "object")
);
verifyDataRows(result,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what would be the data type of nested(address.moveInDate), object or array?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be of type object. address would be of type nested, I'll add the schema verification.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see. one more question.
some rows is JSONObject, some rows are JSONArray, how does JDBC handle it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some cases this breaks JDBC, but there is no change in the expected output with this PR. An example of this would be in ObjectFieldSelectIT where we do a select on an OBJECT type array, and a NESTED type array.

ObjectFieldSelectIT.java#L69-L93
people.json
people-mapping

When selecting on an array that is not of type NESTED, the first value in the array is returned and is functional with JDBC. As shown by the query:

SELECT accounts FROM people;

While on the other hand the query that does a basic SELECT on an NESTED type array returns the whole array which breaks JDBC.

Query:

SELECT projects FROM people;

A user can get around this by using the nested function to flatten the response for a correct JDBC output.

rows(new JSONObject(Map.of("dateAndTime","1984-04-12 09:07:42"))),
rows(new JSONArray(
List.of(
Map.of("dateAndTime", "2023-05-03 08:07:42"),
Map.of("dateAndTime", "2001-11-11 04:07:44"))
)
),
rows(new JSONObject(Map.of("dateAndTime", "1966-03-19 03:04:55"))),
rows(new JSONObject(Map.of("dateAndTime", "2011-06-01 01:01:42"))),
rows(new JSONObject(Map.of("dateAndTime", "1901-08-11 04:03:33"))),
rows(new JSONObject(Map.of("dateAndTime", "2023-05-03 08:07:42"))),
rows(new JSONObject(Map.of("dateAndTime", "2001-11-11 04:07:44"))),
rows(new JSONObject(Map.of("dateAndTime", "1977-07-13 09:04:41"))),
rows(new JSONObject(Map.of("dateAndTime", "1933-12-12 05:05:45"))),
rows(new JSONObject(Map.of("dateAndTime", "1909-06-17 01:04:21"))),
rows(new JSONArray(
List.of(
Map.of("dateAndTime", "2001-11-11 04:07:44"))
)
)
);
}
}
8 changes: 8 additions & 0 deletions integ-test/src/test/resources/indexDefinitions/nested_simple_index_mapping.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@
"ignore_above": 256
}
}
},
"moveInDate" : {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did we want to test this with a nested type too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a inner object to the nested object address. We have IT tests to test with the nested function in NestedIT.java#L373-L400. Is this what you were thinking of testing?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I think I was wondering if we had nested within nested.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we already have tests for nested in nested too.
NestedIT.java#L106-L118

"properties": {
"dateAndTime": {
"type": "date",
"format": "basic_date_time"
}
}
}
}
},
Expand Down
10 changes: 5 additions & 5 deletions integ-test/src/test/resources/nested_simple.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{"index":{"_id":"1"}}
{"name":"abbas","age":24,"address":[{"city":"New york city","state":"NY"},{"city":"bellevue","state":"WA"},{"city":"seattle","state":"WA"},{"city":"chicago","state":"IL"}]}
{"name":"abbas","age":24,"address":[{"city":"New york city","state":"NY","moveInDate":{"dateAndTime":"19840412T090742.000Z"}},{"city":"bellevue","state":"WA","moveInDate":[{"dateAndTime":"20230503T080742.000Z"},{"dateAndTime":"20011111T040744.000Z"}]},{"city":"seattle","state":"WA","moveInDate":{"dateAndTime":"19660319T030455.000Z"}},{"city":"chicago","state":"IL","moveInDate":{"dateAndTime":"20110601T010142.000Z"}}]}
{"index":{"_id":"2"}}
{"name":"chen","age":32,"address":[{"city":"Miami","state":"Florida"},{"city":"los angeles","state":"CA"}]}
{"name":"chen","age":32,"address":[{"city":"Miami","state":"Florida","moveInDate":{"dateAndTime":"19010811T040333.000Z"}},{"city":"los angeles","state":"CA","moveInDate":{"dateAndTime":"20230503T080742.000Z"}}]}
{"index":{"_id":"3"}}
{"name":"peng","age":26,"address":[{"city":"san diego","state":"CA"},{"city":"austin","state":"TX"}]}
{"name":"peng","age":26,"address":[{"city":"san diego","state":"CA","moveInDate":{"dateAndTime":"20011111T040744.000Z"}},{"city":"austin","state":"TX","moveInDate":{"dateAndTime":"19770713T090441.000Z"}}]}
{"index":{"_id":"4"}}
{"name":"andy","age":19,"id":4,"address":[{"city":"houston","state":"TX"}]}
{"name":"andy","age":19,"id":4,"address":[{"city":"houston","state":"TX","moveInDate":{"dateAndTime":"19331212T050545.000Z"}}]}
{"index":{"_id":"5"}}
{"name":"david","age":25,"address":[{"city":"raleigh","state":"NC"},{"city":"charlotte","state":"SC"}]}
{"name":"david","age":25,"address":[{"city":"raleigh","state":"NC","moveInDate":{"dateAndTime":"19090617T010421.000Z"}},{"city":"charlotte","state":"SC","moveInDate":[{"dateAndTime":"20011111T040744.000Z"}]}]}
25 changes: 25 additions & 0 deletions opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/Content.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,36 @@ public interface Content {
*/
boolean isNumber();

/**
* Is float value.
*/
boolean isFloat();

/**
* Is double value.
*/
boolean isDouble();

/**
* Is long value.
*/
boolean isLong();

/**
* Is boolean value.
*/
boolean isBoolean();

/**
* Is string value.
*/
boolean isString();

/**
* Is array value.
*/
boolean isArray();

/**
* Get integer value.
*/
Expand Down
26 changes: 26 additions & 0 deletions opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/ObjectContent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package org.opensearch.sql.opensearch.data.utils;

import com.fasterxml.jackson.databind.node.ArrayNode;
import java.util.AbstractMap;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -103,6 +104,31 @@ public boolean isNumber() {
return value instanceof Number;
}

@Override
public boolean isFloat() {
return value instanceof Float;
}

@Override
public boolean isDouble() {
return value instanceof Double;
}

@Override
public boolean isLong() {
return value instanceof Long;
}

@Override
public boolean isBoolean() {
return value instanceof Boolean;
}

@Override
public boolean isArray() {
return value instanceof ArrayNode;
}

@Override
public boolean isString() {
return value instanceof String;
Expand Down
30 changes: 27 additions & 3 deletions opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,36 @@ public boolean isNumber() {
return value().isNumber();
}

@Override
public boolean isLong() {
return value().isLong();
}

@Override
public boolean isFloat() {
return value().isFloat();
}

@Override
public boolean isDouble() {
return value().isDouble();
}

@Override
public boolean isString() {
return value().isTextual();
}

@Override
public boolean isBoolean() {
return value().isBoolean();
}

@Override
public boolean isArray() {
return value().isArray();
}

@Override
public Object objectValue() {
return value();
Expand Down Expand Up @@ -126,11 +151,10 @@ public Pair<Double, Double> geoValue() {
}

/**
* Return the first element if is OpenSearch Array.
* https://www.elastic.co/guide/en/elasticsearch/reference/current/array.html.
* Getter for value. If value is array the whole array is returned.
*/
private JsonNode value() {
return value.isArray() ? value.get(0) : value;
return value;
}

/**
Expand Down
Loading