From 0b8b11d5f64db2c8789e239c90299ef22f001776 Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Wed, 13 Nov 2024 12:42:57 +0300 Subject: [PATCH 01/13] search dv only ip masks Signed-off-by: mikhail-khludnev --- .../test/search/340_doc_values_field.yml | 88 ++++++++++++++ .../index/mapper/IpFieldMapper.java | 109 ++++++++++-------- 2 files changed, 149 insertions(+), 48 deletions(-) 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 index a133060f07c6f..83d725d6752dd 100644 --- 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 @@ -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 index db8da8a949d6f..dcf8dc61cda6c 100644 --- 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,44 @@ protected Object parseSourceValue(Object value) { @Override public Query termQuery(Object value, @Nullable QueryShardContext context) { failIfNotIndexedAndNoDocValues(); - Query query; - if (value instanceof InetAddress) { - query = InetAddressPoint.newExactQuery(name(), (InetAddress) value); - } else { - if (value instanceof BytesRef) { - value = ((BytesRef) value).utf8ToString(); - } - String term = value.toString(); - if (term.contains("/")) { - final Tuple cidr = InetAddresses.parseCidr(term); - query = InetAddressPoint.newPrefixQuery(name(), cidr.v1(), cidr.v2()); + final PointRangeQuery pointQuery; + { + final Query query; + if (value instanceof InetAddress) { + query = InetAddressPoint.newExactQuery(name(), (InetAddress) value); } else { - InetAddress address = InetAddresses.forString(term); - query = InetAddressPoint.newExactQuery(name(), address); + if (value instanceof BytesRef) { + value = ((BytesRef) value).utf8ToString(); + } + String term = value.toString(); + if (term.contains("/")) { + final Tuple cidr = InetAddresses.parseCidr(term); + query = InetAddressPoint.newPrefixQuery(name(), cidr.v1(), cidr.v2()); + } else { + InetAddress address = InetAddresses.forString(term); + query = InetAddressPoint.newExactQuery(name(), address); + } } + pointQuery = (PointRangeQuery) query; } + final Supplier dvQuery = () -> SortedSetDocValuesField.newSlowRangeQuery( + name(), + new BytesRef(pointQuery.getLowerPoint()), + new BytesRef(pointQuery.getUpperPoint()), + true, + true + ); + if (isSearchable() && hasDocValues()) { - String term = value.toString(); - if (term.contains("/")) { - final Tuple cidr = InetAddresses.parseCidr(term); - return InetAddressPoint.newPrefixQuery(name(), cidr.v1(), cidr.v2()); - } - return new IndexOrDocValuesQuery( - query, - SortedSetDocValuesField.newSlowExactQuery(name(), new BytesRef(((PointRangeQuery) query).getLowerPoint())) - ); - } - if (hasDocValues()) { - String term = value.toString(); - if (term.contains("/")) { - final Tuple cidr = InetAddresses.parseCidr(term); - return InetAddressPoint.newPrefixQuery(name(), cidr.v1(), cidr.v2()); + return new IndexOrDocValuesQuery(pointQuery, dvQuery.get()); + } else { + if (isSearchable()) { + return pointQuery; + } else { + assert hasDocValues(); + return dvQuery.get(); } - return SortedSetDocValuesField.newSlowExactQuery(name(), new BytesRef(((PointRangeQuery) query).getLowerPoint())); } - return query; } @Override @@ -285,7 +288,25 @@ public Query termsQuery(List values, QueryShardContext context) { } addresses[i++] = address; } - return InetAddressPoint.newSetQuery(name(), addresses); + Supplier dvQuery = () -> { + List bytesRefs = Arrays.stream(addresses) + .distinct() + .map(InetAddressPoint::encode) + .map(BytesRef::new) + .collect(Collectors.toList()); + return SortedSetDocValuesField.newSlowSetQuery(name(), bytesRefs); + }; + Supplier pointQuery = () -> InetAddressPoint.newSetQuery(name(), addresses); + if (isSearchable() && hasDocValues()) { + return new IndexOrDocValuesQuery(pointQuery.get(), dvQuery.get()); + } else { + if (isSearchable()) { + return pointQuery.get(); + } else { + assert hasDocValues(); + return dvQuery.get(); + } + } } @Override @@ -293,26 +314,18 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower failIfNotIndexedAndNoDocValues(); return rangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, (lower, upper) -> { Query query = InetAddressPoint.newRangeQuery(name(), lower, upper); + Supplier dvQuery = () -> SortedSetDocValuesField.newSlowRangeQuery( + ((PointRangeQuery) query).getField(), + new BytesRef(((PointRangeQuery) query).getLowerPoint()), + new BytesRef(((PointRangeQuery) query).getUpperPoint()), + true, + true + ); if (isSearchable() && hasDocValues()) { - return new IndexOrDocValuesQuery( - query, - SortedSetDocValuesField.newSlowRangeQuery( - ((PointRangeQuery) query).getField(), - new BytesRef(((PointRangeQuery) query).getLowerPoint()), - new BytesRef(((PointRangeQuery) query).getUpperPoint()), - true, - true - ) - ); + return new IndexOrDocValuesQuery(query, dvQuery.get()); } if (hasDocValues()) { - return SortedSetDocValuesField.newSlowRangeQuery( - ((PointRangeQuery) query).getField(), - new BytesRef(((PointRangeQuery) query).getLowerPoint()), - new BytesRef(((PointRangeQuery) query).getUpperPoint()), - true, - true - ); + return dvQuery.get(); } return query; }); From e69ae3c7aea2d2dc0c490d0d18d4a1f839b6d9d4 Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Wed, 13 Nov 2024 17:39:25 +0300 Subject: [PATCH 02/13] search dv only ip masks: changes Signed-off-by: Mikhail Khludnev --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c19019ece6c5c..22639dec3ac51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,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 From 43803dc9ad24d49daec83018dc9c5db19a386b4f Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Wed, 13 Nov 2024 18:31:48 +0300 Subject: [PATCH 03/13] drop fancy closures Signed-off-by: mikhail-khludnev --- .../index/mapper/IpFieldMapper.java | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java index dcf8dc61cda6c..31401744ab4a8 100644 --- a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java @@ -288,23 +288,26 @@ public Query termsQuery(List values, QueryShardContext context) { } addresses[i++] = address; } - Supplier dvQuery = () -> { + Query dvQuery = null; + if (hasDocValues()) { List bytesRefs = Arrays.stream(addresses) .distinct() .map(InetAddressPoint::encode) .map(BytesRef::new) .collect(Collectors.toList()); - return SortedSetDocValuesField.newSlowSetQuery(name(), bytesRefs); - }; - Supplier pointQuery = () -> InetAddressPoint.newSetQuery(name(), addresses); + dvQuery = SortedSetDocValuesField.newSlowSetQuery(name(), bytesRefs); + } + Query pointQuery = null; + if (isSearchable()) { + pointQuery = InetAddressPoint.newSetQuery(name(), addresses); + } if (isSearchable() && hasDocValues()) { - return new IndexOrDocValuesQuery(pointQuery.get(), dvQuery.get()); + return new IndexOrDocValuesQuery(pointQuery, dvQuery); } else { if (isSearchable()) { - return pointQuery.get(); + return pointQuery; } else { - assert hasDocValues(); - return dvQuery.get(); + return dvQuery; } } } @@ -314,18 +317,21 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower failIfNotIndexedAndNoDocValues(); return rangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, (lower, upper) -> { Query query = InetAddressPoint.newRangeQuery(name(), lower, upper); - Supplier dvQuery = () -> SortedSetDocValuesField.newSlowRangeQuery( - ((PointRangeQuery) query).getField(), - new BytesRef(((PointRangeQuery) query).getLowerPoint()), - new BytesRef(((PointRangeQuery) query).getUpperPoint()), - true, - true - ); + Query dvQuery = null; + if (hasDocValues()) { + dvQuery = SortedSetDocValuesField.newSlowRangeQuery( + ((PointRangeQuery) query).getField(), + new BytesRef(((PointRangeQuery) query).getLowerPoint()), + new BytesRef(((PointRangeQuery) query).getUpperPoint()), + true, + true + ); + } if (isSearchable() && hasDocValues()) { - return new IndexOrDocValuesQuery(query, dvQuery.get()); + return new IndexOrDocValuesQuery(query, dvQuery); } if (hasDocValues()) { - return dvQuery.get(); + return dvQuery; } return query; }); From 605a63974a97528968f31464edd42922b455b884 Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Wed, 13 Nov 2024 18:39:11 +0300 Subject: [PATCH 04/13] drop fancy closures. one more Signed-off-by: mikhail-khludnev --- .../index/mapper/IpFieldMapper.java | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java index 31401744ab4a8..baa391191613c 100644 --- a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java @@ -246,22 +246,24 @@ public Query termQuery(Object value, @Nullable QueryShardContext context) { } pointQuery = (PointRangeQuery) query; } - final Supplier dvQuery = () -> SortedSetDocValuesField.newSlowRangeQuery( - name(), - new BytesRef(pointQuery.getLowerPoint()), - new BytesRef(pointQuery.getUpperPoint()), - true, - true - ); + Query dvQuery = null; + if (hasDocValues()) { + dvQuery = SortedSetDocValuesField.newSlowRangeQuery( + name(), + new BytesRef(pointQuery.getLowerPoint()), + new BytesRef(pointQuery.getUpperPoint()), + true, + true + ); + } if (isSearchable() && hasDocValues()) { - return new IndexOrDocValuesQuery(pointQuery, dvQuery.get()); + return new IndexOrDocValuesQuery(pointQuery, dvQuery); } else { if (isSearchable()) { return pointQuery; } else { - assert hasDocValues(); - return dvQuery.get(); + return dvQuery; } } } From 737b009c7abdc5dfe1e540eab6f3098d4fc570e1 Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Wed, 13 Nov 2024 22:30:17 +0300 Subject: [PATCH 05/13] fix unit tests; add some more dvOnly Signed-off-by: mikhail-khludnev --- .../index/mapper/IpFieldTypeTests.java | 107 +++++++++++++++++- 1 file changed, 103 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/org/opensearch/index/mapper/IpFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/IpFieldTypeTests.java index 0a2435553b19e..a5403ef81481f 100644 --- a/server/src/test/java/org/opensearch/index/mapper/IpFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/IpFieldTypeTests.java @@ -50,6 +50,8 @@ import java.net.InetAddress; import java.util.Arrays; import java.util.Collections; +import java.util.List; +import java.util.Objects; public class IpFieldTypeTests extends FieldTypeTestCase { @@ -76,7 +78,7 @@ public void testValueForSearch() { } public void testTermQuery() { - MappedFieldType ft = new IpFieldMapper.IpFieldType("field"); + MappedFieldType ft = new IpFieldMapper.IpFieldType("field", true, false, true, null, Collections.emptyMap()); String ip = "2001:db8::2:1"; @@ -104,20 +106,94 @@ public void testTermQuery() { String prefix = ip + "/64"; query = InetAddressPoint.newPrefixQuery("field", InetAddresses.forString(ip), 64); - assertEquals(query, ft.termQuery(prefix, null)); + assertEquals( + new IndexOrDocValuesQuery( + query, + SortedSetDocValuesField.newSlowRangeQuery( + "field", + ipToByteRef("2001:db8:0:0:0:0:0:0"), + ipToByteRef("2001:db8:0:0:ffff:ffff:ffff:ffff"), + true, + true + ) + ), + ft.termQuery(prefix, null) + ); ip = "192.168.1.7"; prefix = ip + "/16"; query = InetAddressPoint.newPrefixQuery("field", InetAddresses.forString(ip), 16); - assertEquals(query, ft.termQuery(prefix, null)); + assertEquals( + new IndexOrDocValuesQuery( + query, + SortedSetDocValuesField.newSlowRangeQuery( + "field", + ipToByteRef("::ffff:192.168.0.0"), + ipToByteRef("::ffff:192.168.255.255"), + true, + true + ) + ), + ft.termQuery(prefix, null) + ); MappedFieldType unsearchable = new IpFieldMapper.IpFieldType("field", false, false, false, null, Collections.emptyMap()); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> unsearchable.termQuery("::1", null)); assertEquals("Cannot search on field [field] since it is both not indexed, and does not have doc_values enabled.", e.getMessage()); } + public void testDvOnlyTermQuery() { + IpFieldMapper.IpFieldType dvOnly = new IpFieldMapper.IpFieldType("field", false, false, true, null, Collections.emptyMap()); + String ip = "2001:db8::2:1"; + + Query query = InetAddressPoint.newExactQuery("field", InetAddresses.forString(ip)); + + assertEquals( + SortedSetDocValuesField.newSlowExactQuery("field", new BytesRef(((PointRangeQuery) query).getLowerPoint())), + dvOnly.termQuery(ip, null) + ); + + ip = "192.168.1.7"; + query = InetAddressPoint.newExactQuery("field", InetAddresses.forString(ip)); + assertEquals( + SortedSetDocValuesField.newSlowExactQuery("field", new BytesRef(((PointRangeQuery) query).getLowerPoint())), + dvOnly.termQuery(ip, null) + ); + + ip = "2001:db8::2:1"; + String prefix = ip + "/64"; + + assertEquals( + SortedSetDocValuesField.newSlowRangeQuery( + "field", + ipToByteRef("2001:db8:0:0:0:0:0:0"), + ipToByteRef("2001:db8:0:0:ffff:ffff:ffff:ffff"), + true, + true + ), + dvOnly.termQuery(prefix, null) + ); + + ip = "192.168.1.7"; + prefix = ip + "/16"; + assertEquals( + SortedSetDocValuesField.newSlowRangeQuery( + "field", + ipToByteRef("::ffff:192.168.0.0"), + ipToByteRef("::ffff:192.168.255.255"), + true, + true + ), + dvOnly.termQuery(prefix, null) + ); + } + + private static BytesRef ipToByteRef(String ipString) { + return new BytesRef(Objects.requireNonNull(InetAddresses.ipStringToBytes(ipString))); + } + public void testTermsQuery() { - MappedFieldType ft = new IpFieldMapper.IpFieldType("field"); + MappedFieldType ft = new IpFieldMapper.IpFieldType("field", true, false, false, null, Collections.emptyMap()); assertEquals( InetAddressPoint.newSetQuery("field", InetAddresses.forString("::2"), InetAddresses.forString("::5")), @@ -139,6 +215,29 @@ public void testTermsQuery() { ); } + public void testDvOnlyTermsQuery() { + MappedFieldType dvOnly = new IpFieldMapper.IpFieldType("field", false, false, true, null, Collections.emptyMap()); + + assertEquals( + SortedSetDocValuesField.newSlowSetQuery("field", List.of(ipToByteRef("::2"), ipToByteRef("::5"))), + dvOnly.termsQuery(Arrays.asList(InetAddresses.forString("::2"), InetAddresses.forString("::5")), null) + ); + assertEquals( + SortedSetDocValuesField.newSlowSetQuery("field", List.of(ipToByteRef("::2"), ipToByteRef("::5"))), + dvOnly.termsQuery(Arrays.asList("::2", "::5"), null) + ); + + // if the list includes a prefix query we fallback to a bool query + assertEquals( + new ConstantScoreQuery( + new BooleanQuery.Builder().add(dvOnly.termQuery("::42", null), Occur.SHOULD) + .add(dvOnly.termQuery("::2/16", null), Occur.SHOULD) + .build() + ), + dvOnly.termsQuery(Arrays.asList("::42", "::2/16"), null) + ); + } + public void testRangeQuery() { MappedFieldType ft = new IpFieldMapper.IpFieldType("field"); Query query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::"), InetAddressPoint.MAX_VALUE); From c56ee660bee07804cacc64537618aa02b266ae3e Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Wed, 13 Nov 2024 22:39:39 +0300 Subject: [PATCH 06/13] drop skipping dvOnly in 2.x Signed-off-by: mikhail-khludnev --- .../rest-api-spec/test/search/340_doc_values_field.yml | 4 ---- 1 file changed, 4 deletions(-) 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 index 83d725d6752dd..e22f5f395ada6 100644 --- 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 @@ -1119,10 +1119,6 @@ - match: { hits.total: 0 } --- "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" - do: indices.create: index: test-doc-values From 6eb18ba8321f48d71b6c61c8813e44cc50f65b81 Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Thu, 14 Nov 2024 09:28:22 +0300 Subject: [PATCH 07/13] drop redundant brackets Signed-off-by: mikhail-khludnev --- .../index/mapper/IpFieldMapper.java | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java index baa391191613c..a6118736d615a 100644 --- a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java @@ -227,24 +227,20 @@ protected Object parseSourceValue(Object value) { public Query termQuery(Object value, @Nullable QueryShardContext context) { failIfNotIndexedAndNoDocValues(); final PointRangeQuery pointQuery; - { - final Query query; - if (value instanceof InetAddress) { - query = InetAddressPoint.newExactQuery(name(), (InetAddress) value); + if (value instanceof InetAddress) { + 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 cidr = InetAddresses.parseCidr(term); + pointQuery = (PointRangeQuery) InetAddressPoint.newPrefixQuery(name(), cidr.v1(), cidr.v2()); } else { - if (value instanceof BytesRef) { - value = ((BytesRef) value).utf8ToString(); - } - String term = value.toString(); - if (term.contains("/")) { - final Tuple cidr = InetAddresses.parseCidr(term); - query = InetAddressPoint.newPrefixQuery(name(), cidr.v1(), cidr.v2()); - } else { - InetAddress address = InetAddresses.forString(term); - query = InetAddressPoint.newExactQuery(name(), address); - } + InetAddress address = InetAddresses.forString(term); + pointQuery = (PointRangeQuery) InetAddressPoint.newExactQuery(name(), address); } - pointQuery = (PointRangeQuery) query; } Query dvQuery = null; if (hasDocValues()) { From de335b94f1f0af832914c4ec47db480aa024059a Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Thu, 14 Nov 2024 09:37:27 +0300 Subject: [PATCH 08/13] extract conditions Signed-off-by: mikhail-khludnev --- .../index/mapper/IpFieldMapper.java | 29 ++++++------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java index a6118736d615a..8336b09bdb818 100644 --- a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java @@ -252,7 +252,10 @@ public Query termQuery(Object value, @Nullable QueryShardContext context) { true ); } + return indexOrDvQuery(pointQuery, dvQuery); + } + private Query indexOrDvQuery(Query pointQuery, Query dvQuery) { if (isSearchable() && hasDocValues()) { return new IndexOrDocValuesQuery(pointQuery, dvQuery); } else { @@ -299,39 +302,25 @@ public Query termsQuery(List values, QueryShardContext context) { if (isSearchable()) { pointQuery = InetAddressPoint.newSetQuery(name(), addresses); } - if (isSearchable() && hasDocValues()) { - return new IndexOrDocValuesQuery(pointQuery, dvQuery); - } else { - if (isSearchable()) { - return pointQuery; - } else { - return dvQuery; - } - } + return indexOrDvQuery(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); + PointRangeQuery pointQuery = (PointRangeQuery) InetAddressPoint.newRangeQuery(name(), lower, upper); Query dvQuery = null; if (hasDocValues()) { dvQuery = SortedSetDocValuesField.newSlowRangeQuery( - ((PointRangeQuery) query).getField(), - new BytesRef(((PointRangeQuery) query).getLowerPoint()), - new BytesRef(((PointRangeQuery) query).getUpperPoint()), + pointQuery.getField(), + new BytesRef(pointQuery.getLowerPoint()), + new BytesRef(pointQuery.getUpperPoint()), true, true ); } - if (isSearchable() && hasDocValues()) { - return new IndexOrDocValuesQuery(query, dvQuery); - } - if (hasDocValues()) { - return dvQuery; - } - return query; + return indexOrDvQuery(pointQuery, dvQuery); }); } From f7eecfc1313d0fb07d06fdb0333b775f4660d67f Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Thu, 14 Nov 2024 09:40:56 +0300 Subject: [PATCH 09/13] asserts Signed-off-by: mikhail-khludnev --- .../main/java/org/opensearch/index/mapper/IpFieldMapper.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java index 8336b09bdb818..49e5d104ab310 100644 --- a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java @@ -257,11 +257,15 @@ public Query termQuery(Object value, @Nullable QueryShardContext context) { private Query indexOrDvQuery(Query pointQuery, Query dvQuery) { if (isSearchable() && hasDocValues()) { + assert pointQuery!=null; + assert dvQuery!=null; return new IndexOrDocValuesQuery(pointQuery, dvQuery); } else { if (isSearchable()) { + assert pointQuery!=null; return pointQuery; } else { + assert dvQuery!=null; return dvQuery; } } From a859408944681226a1c1c04af559bfe7ff97b902 Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Thu, 14 Nov 2024 11:04:23 +0300 Subject: [PATCH 10/13] spotless apply Signed-off-by: mikhail-khludnev --- .../java/org/opensearch/index/mapper/IpFieldMapper.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java index 49e5d104ab310..4f8d81c2ca200 100644 --- a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java @@ -257,15 +257,15 @@ public Query termQuery(Object value, @Nullable QueryShardContext context) { private Query indexOrDvQuery(Query pointQuery, Query dvQuery) { if (isSearchable() && hasDocValues()) { - assert pointQuery!=null; - assert dvQuery!=null; + assert pointQuery != null; + assert dvQuery != null; return new IndexOrDocValuesQuery(pointQuery, dvQuery); } else { if (isSearchable()) { - assert pointQuery!=null; + assert pointQuery != null; return pointQuery; } else { - assert dvQuery!=null; + assert dvQuery != null; return dvQuery; } } From fa143025da3caf45e93533b9fc02bcd3c60cfc26 Mon Sep 17 00:00:00 2001 From: mikhail-khludnev Date: Thu, 14 Nov 2024 12:17:30 +0300 Subject: [PATCH 11/13] bring back skip before Signed-off-by: mikhail-khludnev --- .../rest-api-spec/test/search/340_doc_values_field.yml | 4 ++++ 1 file changed, 4 insertions(+) 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 index e22f5f395ada6..647aaf2c9088b 100644 --- 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 @@ -1119,6 +1119,10 @@ - match: { hits.total: 0 } --- "search on fields with only doc_values enabled": + - skip: + features: [ "headers" ] + version: " - 2.18.99" + reason: "searching with only doc_values was finally added in 2.19.0" - do: indices.create: index: test-doc-values From f5226222f0f2d238817e6551e428dbfb5d58e76f Mon Sep 17 00:00:00 2001 From: mikhail-khludnev Date: Thu, 14 Nov 2024 19:06:08 +0300 Subject: [PATCH 12/13] combine asserts Signed-off-by: mikhail-khludnev --- .../main/java/org/opensearch/index/mapper/IpFieldMapper.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java index 4f8d81c2ca200..d30a282c57f45 100644 --- a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java @@ -257,8 +257,7 @@ public Query termQuery(Object value, @Nullable QueryShardContext context) { private Query indexOrDvQuery(Query pointQuery, Query dvQuery) { if (isSearchable() && hasDocValues()) { - assert pointQuery != null; - assert dvQuery != null; + assert pointQuery != null && dvQuery != null; return new IndexOrDocValuesQuery(pointQuery, dvQuery); } else { if (isSearchable()) { From ca7569dc29899c29c2563f605e3e55a1f47715a1 Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Fri, 15 Nov 2024 22:33:46 +0300 Subject: [PATCH 13/13] inline, copy-paste Signed-off-by: mikhail-khludnev --- .../index/mapper/IpFieldMapper.java | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java index d30a282c57f45..c51cada9f3143 100644 --- a/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/IpFieldMapper.java @@ -252,21 +252,10 @@ public Query termQuery(Object value, @Nullable QueryShardContext context) { true ); } - return indexOrDvQuery(pointQuery, dvQuery); - } - - private Query indexOrDvQuery(Query pointQuery, Query dvQuery) { if (isSearchable() && hasDocValues()) { - assert pointQuery != null && dvQuery != null; return new IndexOrDocValuesQuery(pointQuery, dvQuery); } else { - if (isSearchable()) { - assert pointQuery != null; - return pointQuery; - } else { - assert dvQuery != null; - return dvQuery; - } + return isSearchable() ? pointQuery : dvQuery; } } @@ -305,7 +294,11 @@ public Query termsQuery(List values, QueryShardContext context) { if (isSearchable()) { pointQuery = InetAddressPoint.newSetQuery(name(), addresses); } - return indexOrDvQuery(pointQuery, dvQuery); + if (isSearchable() && hasDocValues()) { + return new IndexOrDocValuesQuery(pointQuery, dvQuery); + } else { + return isSearchable() ? pointQuery : dvQuery; + } } @Override @@ -323,7 +316,11 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower true ); } - return indexOrDvQuery(pointQuery, dvQuery); + if (isSearchable() && hasDocValues()) { + return new IndexOrDocValuesQuery(pointQuery, dvQuery); + } else { + return isSearchable() ? pointQuery : dvQuery; + } }); }