From 8620525cb4e853db28597bbf19e2be5310434b3a Mon Sep 17 00:00:00 2001 From: forestmvey Date: Mon, 10 Apr 2023 13:46:47 -0700 Subject: [PATCH] Fix filter push down with nested push down with added IT and UT tests. Add comments to clarify implementation. Signed-off-by: forestmvey --- docs/user/dql/functions.rst | 2 +- .../java/org/opensearch/sql/sql/NestedIT.java | 13 +++++++ .../request/OpenSearchRequestBuilder.java | 9 ++++- .../scan/OpenSearchIndexScanQueryBuilder.java | 3 ++ .../request/OpenSearchRequestBuilderTest.java | 37 +++++++++++++++++++ 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 32fc62516e..14fda09349 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -4308,7 +4308,7 @@ Description ``nested(field | [field, path])`` The ``nested`` function maps to the ``nested`` query used in search engine. It returns nested field types in documents that match the provided specified field(s). -If the user does not provide the ``path`` parameter it will be generated dynamically. The ``field`` ``user.office.cubicle`` would dynamically generate the path +If the user does not provide the ``path`` parameter it will be generated dynamically. For example the ``field`` ``user.office.cubicle`` would dynamically generate the path ``user.office``. Example with ``field`` and ``path`` parameters:: diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java index c3e3250d97..48ea940161 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java @@ -190,6 +190,19 @@ public void nested_function_mixed_with_non_nested_types_test() { rows("zz", null, null)); } + @Test + public void nested_function_with_relevance_query() { + String query = + "SELECT nested(message.info), someField FROM " + TEST_INDEX_NESTED_TYPE + " WHERE match(someField, 'b')"; + JSONObject result = executeJdbcRequest(query); + + assertEquals(3, result.getInt("total")); + verifyDataRows(result, + rows("a", "b"), + rows("c", "b"), + rows("a", "b")); + } + @Test public void nested_with_non_nested_type_test() { String query = "SELECT nested(someField) FROM " + TEST_INDEX_NESTED_TYPE; diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java index 96587e1a02..b245944cde 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java @@ -263,8 +263,13 @@ fieldNames, createEmptyNestedQuery(path) * Initialize bool query for push down. */ private void initBoolQueryFilter() { - sourceBuilder.query(QueryBuilders.boolQuery()); - query().filter(boolQuery()); + if (sourceBuilder.query() == null) { + sourceBuilder.query(QueryBuilders.boolQuery()); + } else { + sourceBuilder.query(QueryBuilders.boolQuery().must(sourceBuilder.query())); + } + + sourceBuilder.query(QueryBuilders.boolQuery().filter(sourceBuilder.query())); } /** diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanQueryBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanQueryBuilder.java index 80befce0da..2985d8f3ab 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanQueryBuilder.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanQueryBuilder.java @@ -106,6 +106,9 @@ public boolean pushDownNested(LogicalNested nested) { indexScan.getRequestBuilder().pushDownProjects( findReferenceExpressions(nested.getProjectList())); // Return false intentionally to keep the original nested operator + // Since we return false we need to pushDownProject here as it won't be + // pushed down due to no matching push down rule. + // TODO: improve LogicalPlanOptimizer pushdown api. return false; } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java index 4a5fbd4873..5975a1f79f 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java @@ -285,6 +285,43 @@ void testPushDownMultipleNestedWithSamePath() { requestBuilder.getSourceBuilder()); } + @Test + void testPushDownNestedWithFilter() { + List> args = List.of( + Map.of( + "field", new ReferenceExpression("message.info", STRING), + "path", new ReferenceExpression("message", STRING) + ) + ); + + List projectList = + List.of( + new NamedExpression("message.info", DSL.nested(DSL.ref("message.info", STRING)), null) + ); + + LogicalNested nested = new LogicalNested(null, args, projectList); + requestBuilder.getSourceBuilder().query(QueryBuilders.rangeQuery("myNum").gt(3)); + requestBuilder.pushDownNested(nested.getFields()); + + NestedQueryBuilder nestedQuery = nestedQuery("message", matchAllQuery(), ScoreMode.None) + .innerHit(new InnerHitBuilder().setFetchSourceContext( + new FetchSourceContext(true, new String[]{"message.info"}, null))); + + assertEquals( + new SearchSourceBuilder() + .query( + QueryBuilders.boolQuery().filter( + QueryBuilders.boolQuery() + .must(QueryBuilders.rangeQuery("myNum").gt(3)) + .must(nestedQuery) + ) + ) + .from(DEFAULT_OFFSET) + .size(DEFAULT_LIMIT) + .timeout(DEFAULT_QUERY_TIMEOUT), + requestBuilder.getSourceBuilder()); + } + @Test void testPushTypeMapping() { Map typeMapping = Map.of("intA", OpenSearchDataType.of(INTEGER));