Fix filter push down with nested push down with added IT and UT tests…

…. Add comments to clarify implementation. Signed-off-by: forestmvey <forestv@bitquilltech.com>
Bit-Quill · Apr 10, 2023 · 8620525 · 8620525
1 parent df5ddb0
commit 8620525
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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/...main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanQueryBuilder.java b/...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/...rch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java b/...rch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java
@@ -285,6 +285,43 @@ void testPushDownMultipleNestedWithSamePath() {
         requestBuilder.getSourceBuilder());
   }
 
+  @Test
+  void testPushDownNestedWithFilter() {
+    List<Map<String, ReferenceExpression>> args = List.of(
+        Map.of(
+            "field", new ReferenceExpression("message.info", STRING),
+            "path", new ReferenceExpression("message", STRING)
+        )
+    );
+
+    List<NamedExpression> 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<String, OpenSearchDataType> typeMapping = Map.of("intA", OpenSearchDataType.of(INTEGER));