Search dv only IP masks (opensearch-project#16628)

* search dv only ip masks Signed-off-by: mikhail-khludnev <[email protected]> * search dv only ip masks: changes Signed-off-by: Mikhail Khludnev <[email protected]> * drop fancy closures Signed-off-by: mikhail-khludnev <[email protected]> * drop fancy closures. one more Signed-off-by: mikhail-khludnev <[email protected]> * fix unit tests; add some more dvOnly Signed-off-by: mikhail-khludnev <[email protected]> * drop skipping dvOnly in 2.x Signed-off-by: mikhail-khludnev <[email protected]> * drop redundant brackets Signed-off-by: mikhail-khludnev <[email protected]> * extract conditions Signed-off-by: mikhail-khludnev <[email protected]> * asserts Signed-off-by: mikhail-khludnev <[email protected]> * spotless apply Signed-off-by: mikhail-khludnev <[email protected]> * bring back skip before Signed-off-by: mikhail-khludnev <[email protected]> * combine asserts Signed-off-by: mikhail-khludnev <[email protected]> * inline, copy-paste Signed-off-by: mikhail-khludnev <[email protected]> --------- Signed-off-by: mikhail-khludnev <[email protected]> Signed-off-by: Mikhail Khludnev <[email protected]> Co-authored-by: mikhail-khludnev <[email protected]>
andrross · Nov 15, 2024 · d4d70d8 · d4d70d8
1 parent 8e097ee
commit d4d70d8
Show file tree

Hide file tree

Showing 4 changed files with 240 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -49,6 +49,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Support retrieving doc values of unsigned long field ([#16543](https://github.com/opensearch-project/OpenSearch/pull/16543))
 - Fix rollover alias supports restored searchable snapshot index([#16483](https://github.com/opensearch-project/OpenSearch/pull/16483))
 - Fix permissions error on scripted query against remote snapshot ([#16544](https://github.com/opensearch-project/OpenSearch/pull/16544))
+- Fix `doc_values` only (`index:false`) IP field searching for masks ([#16628](https://github.com/opensearch-project/OpenSearch/pull/16628))
 
 ### Security
 

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/340_doc_values_field.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/340_doc_values_field.yml
@@ -449,6 +449,28 @@
 
   - match: { hits.total: 2 }
 
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-iodvq
+        body:
+          query:
+            term:
+              ip_field: "192.168.0.1/24"
+
+  - match: { hits.total: 3 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-iodvq
+        body:
+          query:
+            term:
+              ip_field: "192.168.0.1/31"
+
+  - match: { hits.total: 1 }
+
   - do:
       search:
         rest_total_hits_as_int: true
@@ -987,6 +1009,28 @@
 
   - match: { hits.total: 2 }
 
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-index
+        body:
+          query:
+            term:
+              ip_field: "192.168.0.1/24"
+
+  - match: { hits.total: 3 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-index
+        body:
+          query:
+            term:
+              ip_field: "192.168.0.1/31"
+
+  - match: { hits.total: 1 }
+
   - do:
       search:
         rest_total_hits_as_int: true
@@ -1077,8 +1121,8 @@
 "search on fields with only doc_values enabled":
   - skip:
       features: [ "headers" ]
-      version: " - 2.99.99"
-      reason: "searching with only doc_values was added in 3.0.0"
+      version: " - 2.18.99"
+      reason: "searching with only doc_values was finally added in 2.19.0"
   - do:
       indices.create:
         index: test-doc-values
@@ -1372,6 +1416,28 @@
 
   - match: { hits.total: 2 }
 
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-doc-values
+        body:
+          query:
+            terms:
+              ip_field: ["192.168.0.1", "192.168.0.2"]
+
+  - match: { hits.total: 2 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-doc-values
+        body:
+          query:
+            terms:
+              ip_field: ["192.168.0.1/31", "192.168.0.3"]
+
+  - match: { hits.total: 2 }
+
   - do:
       search:
         rest_total_hits_as_int: true
@@ -1516,6 +1582,28 @@
 
   - match: { hits.total: 2 }
 
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-doc-values
+        body:
+          query:
+            term:
+              ip_field: "192.168.0.1/31"
+
+  - match: { hits.total: 1 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-doc-values
+        body:
+          query:
+            term:
+              ip_field: "192.168.0.1/24"
+
+  - match: { hits.total: 3 }
+
   - do:
       search:
         rest_total_hits_as_int: true

diff --git a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java
@@ -64,6 +64,7 @@
 import java.util.Map;
 import java.util.function.BiFunction;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 
 /**
  * A {@link FieldMapper} for ip addresses.
@@ -225,42 +226,37 @@ protected Object parseSourceValue(Object value) {
         @Override
         public Query termQuery(Object value, @Nullable QueryShardContext context) {
             failIfNotIndexedAndNoDocValues();
-            Query query;
+            final PointRangeQuery pointQuery;
             if (value instanceof InetAddress) {
-                query = InetAddressPoint.newExactQuery(name(), (InetAddress) value);
+                pointQuery = (PointRangeQuery) InetAddressPoint.newExactQuery(name(), (InetAddress) value);
             } else {
                 if (value instanceof BytesRef) {
                     value = ((BytesRef) value).utf8ToString();
                 }
                 String term = value.toString();
                 if (term.contains("/")) {
                     final Tuple<InetAddress, Integer> cidr = InetAddresses.parseCidr(term);
-                    query = InetAddressPoint.newPrefixQuery(name(), cidr.v1(), cidr.v2());
+                    pointQuery = (PointRangeQuery) InetAddressPoint.newPrefixQuery(name(), cidr.v1(), cidr.v2());
                 } else {
                     InetAddress address = InetAddresses.forString(term);
-                    query = InetAddressPoint.newExactQuery(name(), address);
+                    pointQuery = (PointRangeQuery) InetAddressPoint.newExactQuery(name(), address);
                 }
             }
-            if (isSearchable() && hasDocValues()) {
-                String term = value.toString();
-                if (term.contains("/")) {
-                    final Tuple<InetAddress, Integer> cidr = InetAddresses.parseCidr(term);
-                    return InetAddressPoint.newPrefixQuery(name(), cidr.v1(), cidr.v2());
-                }
-                return new IndexOrDocValuesQuery(
-                    query,
-                    SortedSetDocValuesField.newSlowExactQuery(name(), new BytesRef(((PointRangeQuery) query).getLowerPoint()))
+            Query dvQuery = null;
+            if (hasDocValues()) {
+                dvQuery = SortedSetDocValuesField.newSlowRangeQuery(
+                    name(),
+                    new BytesRef(pointQuery.getLowerPoint()),
+                    new BytesRef(pointQuery.getUpperPoint()),
+                    true,
+                    true
                 );
             }
-            if (hasDocValues()) {
-                String term = value.toString();
-                if (term.contains("/")) {
-                    final Tuple<InetAddress, Integer> cidr = InetAddresses.parseCidr(term);
-                    return InetAddressPoint.newPrefixQuery(name(), cidr.v1(), cidr.v2());
-                }
-                return SortedSetDocValuesField.newSlowExactQuery(name(), new BytesRef(((PointRangeQuery) query).getLowerPoint()));
+            if (isSearchable() && hasDocValues()) {
+                return new IndexOrDocValuesQuery(pointQuery, dvQuery);
+            } else {
+                return isSearchable() ? pointQuery : dvQuery;
             }
-            return query;
         }
 
         @Override
@@ -285,36 +281,46 @@ public Query termsQuery(List<?> values, QueryShardContext context) {
                 }
                 addresses[i++] = address;
             }
-            return InetAddressPoint.newSetQuery(name(), addresses);
+            Query dvQuery = null;
+            if (hasDocValues()) {
+                List<BytesRef> bytesRefs = Arrays.stream(addresses)
+                    .distinct()
+                    .map(InetAddressPoint::encode)
+                    .map(BytesRef::new)
+                    .collect(Collectors.<BytesRef>toList());
+                dvQuery = SortedSetDocValuesField.newSlowSetQuery(name(), bytesRefs);
+            }
+            Query pointQuery = null;
+            if (isSearchable()) {
+                pointQuery = InetAddressPoint.newSetQuery(name(), addresses);
+            }
+            if (isSearchable() && hasDocValues()) {
+                return new IndexOrDocValuesQuery(pointQuery, dvQuery);
+            } else {
+                return isSearchable() ? pointQuery : dvQuery;
+            }
         }
 
         @Override
         public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, QueryShardContext context) {
             failIfNotIndexedAndNoDocValues();
             return rangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, (lower, upper) -> {
-                Query query = InetAddressPoint.newRangeQuery(name(), lower, upper);
-                if (isSearchable() && hasDocValues()) {
-                    return new IndexOrDocValuesQuery(
-                        query,
-                        SortedSetDocValuesField.newSlowRangeQuery(
-                            ((PointRangeQuery) query).getField(),
-                            new BytesRef(((PointRangeQuery) query).getLowerPoint()),
-                            new BytesRef(((PointRangeQuery) query).getUpperPoint()),
-                            true,
-                            true
-                        )
-                    );
-                }
+                PointRangeQuery pointQuery = (PointRangeQuery) InetAddressPoint.newRangeQuery(name(), lower, upper);
+                Query dvQuery = null;
                 if (hasDocValues()) {
-                    return SortedSetDocValuesField.newSlowRangeQuery(
-                        ((PointRangeQuery) query).getField(),
-                        new BytesRef(((PointRangeQuery) query).getLowerPoint()),
-                        new BytesRef(((PointRangeQuery) query).getUpperPoint()),
+                    dvQuery = SortedSetDocValuesField.newSlowRangeQuery(
+                        pointQuery.getField(),
+                        new BytesRef(pointQuery.getLowerPoint()),
+                        new BytesRef(pointQuery.getUpperPoint()),
                         true,
                         true
                     );
                 }
-                return query;
+                if (isSearchable() && hasDocValues()) {
+                    return new IndexOrDocValuesQuery(pointQuery, dvQuery);
+                } else {
+                    return isSearchable() ? pointQuery : dvQuery;
+                }
             });
         }