Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/main' into integ-metafields-ro…
Browse files Browse the repository at this point in the history
…uting

Signed-off-by: acarbonetto <[email protected]>
  • Loading branch information
acarbonetto committed Jun 27, 2023
2 parents ede3956 + 3302ec8 commit be90b0f
Show file tree
Hide file tree
Showing 35 changed files with 1,330 additions and 154 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import org.opensearch.sql.ast.expression.Function;
import org.opensearch.sql.ast.expression.QualifiedName;
import org.opensearch.sql.ast.expression.UnresolvedExpression;
import org.opensearch.sql.expression.Expression;
import org.opensearch.sql.expression.FunctionExpression;
import org.opensearch.sql.expression.NamedExpression;
import org.opensearch.sql.expression.ReferenceExpression;
import org.opensearch.sql.expression.function.BuiltinFunctionName;
Expand Down Expand Up @@ -105,7 +107,18 @@ private void validateArgs(List<UnresolvedExpression> args) {
* @param field : Nested field to generate path of.
* @return : Path of field derived from last level of nesting.
*/
private ReferenceExpression generatePath(String field) {
public static ReferenceExpression generatePath(String field) {
return new ReferenceExpression(field.substring(0, field.lastIndexOf(".")), STRING);
}

/**
* Check if supplied expression is a nested function.
* @param expr Expression checking if is nested function.
* @return True if expression is a nested function.
*/
public static Boolean isNestedFunction(Expression expr) {
return (expr instanceof FunctionExpression
&& ((FunctionExpression) expr).getFunctionName().getFunctionName()
.equalsIgnoreCase(BuiltinFunctionName.NESTED.name()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
import static java.util.Collections.emptyList;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME;
import static org.opensearch.sql.analysis.NestedAnalyzer.isNestedFunction;
import static org.opensearch.sql.ast.dsl.AstDSL.aggregate;
import static org.opensearch.sql.ast.dsl.AstDSL.alias;
import static org.opensearch.sql.ast.dsl.AstDSL.argument;
Expand Down Expand Up @@ -39,6 +41,7 @@
import static org.opensearch.sql.data.type.ExprCoreType.LONG;
import static org.opensearch.sql.data.type.ExprCoreType.STRING;
import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP;
import static org.opensearch.sql.expression.DSL.literal;
import static org.opensearch.sql.utils.MLCommonsConstants.ACTION;
import static org.opensearch.sql.utils.MLCommonsConstants.ALGO;
import static org.opensearch.sql.utils.MLCommonsConstants.ASYNC;
Expand Down Expand Up @@ -574,6 +577,10 @@ public void project_nested_field_arg() {
function("nested", qualifiedName("message", "info")), null)
)
);

assertTrue(isNestedFunction(DSL.nested(DSL.ref("message.info", STRING))));
assertFalse(isNestedFunction(DSL.literal("fieldA")));
assertFalse(isNestedFunction(DSL.match(DSL.namedArgument("field", literal("message")))));
}

@Test
Expand Down
11 changes: 11 additions & 0 deletions docs/user/dql/functions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4469,6 +4469,17 @@ Example with ``field`` and ``path`` parameters in the SELECT and WHERE clause::
| b |
+---------------------------------+

Example with ``field`` and ``path`` parameters in the SELECT and ORDER BY clause::

os> SELECT nested(message.info, message) FROM nested ORDER BY nested(message.info, message) DESC;
fetched rows / total rows = 2/2
+---------------------------------+
| nested(message.info, message) |
|---------------------------------|
| b |
| a |
+---------------------------------+


System Functions
================
Expand Down
2 changes: 1 addition & 1 deletion integ-test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ dependencies {
testRuntimeOnly('org.junit.jupiter:junit-jupiter-engine:5.6.2')

testImplementation group: 'com.h2database', name: 'h2', version: '2.1.214'
testImplementation group: 'org.xerial', name: 'sqlite-jdbc', version: '3.32.3.3'
testImplementation group: 'org.xerial', name: 'sqlite-jdbc', version: '3.41.2.2'
testImplementation group: 'com.google.code.gson', name: 'gson', version: '2.8.9'

// Needed for BWC tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import lombok.EqualsAndHashCode;
import java.util.stream.Collectors;
import lombok.Getter;
import lombok.ToString;
import org.json.JSONPropertyName;
Expand All @@ -24,7 +25,6 @@
* query with SELECT columns or just *, order of column and row may matter or not. So the internal data structure of this
* class is passed in from outside either list or set, hash map or linked hash map etc.
*/
@EqualsAndHashCode(exclude = "databaseName")
@ToString
public class DBResult {

Expand Down Expand Up @@ -191,4 +191,24 @@ private static <T extends Comparable<T>> List<T> sort(Collection<T> collection)
return list;
}

public boolean equals(final Object o) {
if (o == this) {
return true;
}
if (!(o instanceof DBResult)) {
return false;
}
final DBResult other = (DBResult) o;
// H2 calculates the value before setting column name
// for example, for query "select 1 + 1" it returns a column named "2" instead of "1 + 1"
boolean skipColumnNameCheck = databaseName.equalsIgnoreCase("h2") || other.databaseName.equalsIgnoreCase("h2");
if (!skipColumnNameCheck && !schema.equals(other.schema)) {
return false;
}
if (skipColumnNameCheck && !schema.stream().map(Type::getType).collect(Collectors.toList())
.equals(other.schema.stream().map(Type::getType).collect(Collectors.toList()))) {
return false;
}
return dataRows.equals(other.dataRows);
}
}
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
69 changes: 69 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java
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 @@ -187,6 +189,40 @@ public void nested_function_with_order_by_clause() {
rows("zz"));
}

@Test
public void nested_function_with_order_by_clause_desc() {
String query =
"SELECT nested(message.info) FROM " + TEST_INDEX_NESTED_TYPE
+ " ORDER BY nested(message.info, message) DESC";
JSONObject result = executeJdbcRequest(query);

assertEquals(6, result.getInt("total"));
verifyDataRows(result,
rows("zz"),
rows("c"),
rows("c"),
rows("a"),
rows("b"),
rows("a"));
}

@Test
public void nested_function_and_field_with_order_by_clause() {
String query =
"SELECT nested(message.info), myNum FROM " + TEST_INDEX_NESTED_TYPE
+ " ORDER BY nested(message.info, message), myNum";
JSONObject result = executeJdbcRequest(query);

assertEquals(6, result.getInt("total"));
verifyDataRows(result,
rows("a", 1),
rows("c", 4),
rows("a", 4),
rows("b", 2),
rows("c", 3),
rows("zz", new JSONArray(List.of(3, 4))));
}

// Nested function in GROUP BY clause is not yet implemented for JDBC format. This test ensures
// that the V2 engine falls back to legacy implementation.
// TODO Fix the test when NESTED is supported in GROUP BY in the V2 engine.
Expand Down Expand Up @@ -366,4 +402,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,
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"))
)
)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@
"ignore_above": 256
}
}
},
"moveInDate" : {
"properties": {
"dateAndTime": {
"type": "date",
"format": "basic_date_time"
}
}
}
}
},
Expand Down
10 changes: 5 additions & 5 deletions integ-test/src/test/resources/nested_simple.json
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"}]}]}
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
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
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

0 comments on commit be90b0f

Please sign in to comment.