From d8c815c6be80d592cf22f4a1c5060c2ca1857c2a Mon Sep 17 00:00:00 2001 From: Yevhen Tienkaiev Date: Tue, 19 Apr 2022 07:38:54 +0300 Subject: [PATCH] Add `positive_score_impact` support for `rank_features` (#2725) Adds positive_score_impact support for rank_features field mapper. Signed-off-by: Yevhen Tienkaiev --- .../index/mapper/RankFeaturesFieldMapper.java | 41 ++++++++++++++++--- .../mapper/RankFeaturesFieldMapperTests.java | 31 +++++++++++++- .../mapper/RankFeaturesFieldTypeTests.java | 2 +- .../test/rank_feature/10_basic.yml | 21 ++++++++++ .../test/rank_features/10_basic.yml | 40 ++++++++++++++++++ 5 files changed, 126 insertions(+), 9 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/RankFeaturesFieldMapper.java b/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/RankFeaturesFieldMapper.java index 43853eb40f4..21a0acd508a 100644 --- a/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/RankFeaturesFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/RankFeaturesFieldMapper.java @@ -42,7 +42,6 @@ import org.opensearch.index.query.QueryShardContext; import org.opensearch.search.lookup.SearchLookup; import java.io.IOException; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -55,8 +54,18 @@ public class RankFeaturesFieldMapper extends ParametrizedFieldMapper { public static final String CONTENT_TYPE = "rank_features"; + private static RankFeaturesFieldType ft(FieldMapper in) { + return ((RankFeaturesFieldMapper) in).fieldType(); + } + public static class Builder extends ParametrizedFieldMapper.Builder { + private final Parameter positiveScoreImpact = Parameter.boolParam( + "positive_score_impact", + false, + m -> ft(m).positiveScoreImpact, + true + ); private final Parameter> meta = Parameter.metaParam(); public Builder(String name) { @@ -66,16 +75,17 @@ public class RankFeaturesFieldMapper extends ParametrizedFieldMapper { @Override protected List> getParameters() { - return Collections.singletonList(meta); + return List.of(meta, positiveScoreImpact); } @Override public RankFeaturesFieldMapper build(BuilderContext context) { return new RankFeaturesFieldMapper( name, - new RankFeaturesFieldType(buildFullName(context), meta.getValue()), + new RankFeaturesFieldType(buildFullName(context), meta.getValue(), positiveScoreImpact.getValue()), multiFieldsBuilder.build(this, context), - copyTo.build() + copyTo.build(), + positiveScoreImpact.getValue() ); } } @@ -84,9 +94,12 @@ public class RankFeaturesFieldMapper extends ParametrizedFieldMapper { public static final class RankFeaturesFieldType extends MappedFieldType { - public RankFeaturesFieldType(String name, Map meta) { + private final boolean positiveScoreImpact; + + public RankFeaturesFieldType(String name, Map meta, boolean positiveScoreImpact) { super(name, false, false, false, TextSearchInfo.NONE, meta); setIndexAnalyzer(Lucene.KEYWORD_ANALYZER); + this.positiveScoreImpact = positiveScoreImpact; } @Override @@ -94,6 +107,10 @@ public class RankFeaturesFieldMapper extends ParametrizedFieldMapper { return CONTENT_TYPE; } + public boolean positiveScoreImpact() { + return positiveScoreImpact; + } + @Override public Query existsQuery(QueryShardContext context) { throw new IllegalArgumentException("[rank_features] fields do not support [exists] queries"); @@ -115,9 +132,18 @@ public class RankFeaturesFieldMapper extends ParametrizedFieldMapper { } } - private RankFeaturesFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo) { + private final boolean positiveScoreImpact; + + private RankFeaturesFieldMapper( + String simpleName, + MappedFieldType mappedFieldType, + MultiFields multiFields, + CopyTo copyTo, + boolean positiveScoreImpact + ) { super(simpleName, mappedFieldType, multiFields, copyTo); assert fieldType.indexOptions().compareTo(IndexOptions.DOCS_AND_FREQS) <= 0; + this.positiveScoreImpact = positiveScoreImpact; } @Override @@ -164,6 +190,9 @@ public class RankFeaturesFieldMapper extends ParametrizedFieldMapper { + "] in the same document" ); } + if (positiveScoreImpact == false) { + value = 1 / value; + } context.doc().addWithKey(key, new FeatureField(name(), feature, value)); } else { throw new IllegalArgumentException( diff --git a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/RankFeaturesFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/RankFeaturesFieldMapperTests.java index 129ba6b1262..55d825d1b53 100644 --- a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/RankFeaturesFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/RankFeaturesFieldMapperTests.java @@ -67,8 +67,8 @@ public class RankFeaturesFieldMapperTests extends MapperTestCase { } @Override - protected void registerParameters(ParameterChecker checker) { - // no parameters to configure + protected void registerParameters(ParameterChecker checker) throws IOException { + checker.registerConflictCheck("positive_score_impact", b -> b.field("positive_score_impact", false)); } @Override @@ -95,6 +95,33 @@ public class RankFeaturesFieldMapperTests extends MapperTestCase { assertTrue(freq1 < freq2); } + public void testNegativeScoreImpact() throws Exception { + DocumentMapper mapper = createDocumentMapper( + fieldMapping(b -> b.field("type", "rank_features").field("positive_score_impact", false)) + ); + + ParsedDocument doc1 = mapper.parse(source(this::writeField)); + + IndexableField[] fields = doc1.rootDoc().getFields("field"); + assertEquals(2, fields.length); + assertThat(fields[0], Matchers.instanceOf(FeatureField.class)); + FeatureField featureField1 = null; + FeatureField featureField2 = null; + for (IndexableField field : fields) { + if (field.stringValue().equals("foo")) { + featureField1 = (FeatureField) field; + } else if (field.stringValue().equals("bar")) { + featureField2 = (FeatureField) field; + } else { + throw new UnsupportedOperationException(); + } + } + + int freq1 = RankFeatureFieldMapperTests.getFrequency(featureField1.tokenStream(null, null)); + int freq2 = RankFeatureFieldMapperTests.getFrequency(featureField2.tokenStream(null, null)); + assertTrue(freq1 > freq2); + } + public void testRejectMultiValuedFields() throws MapperParsingException, IOException { DocumentMapper mapper = createDocumentMapper(mapping(b -> { b.startObject("field").field("type", "rank_features").endObject(); diff --git a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/RankFeaturesFieldTypeTests.java b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/RankFeaturesFieldTypeTests.java index b8c653bc97c..8ece0d63f05 100644 --- a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/RankFeaturesFieldTypeTests.java +++ b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/RankFeaturesFieldTypeTests.java @@ -37,7 +37,7 @@ import java.util.Collections; public class RankFeaturesFieldTypeTests extends FieldTypeTestCase { public void testIsNotAggregatable() { - MappedFieldType fieldType = new RankFeaturesFieldMapper.RankFeaturesFieldType("field", Collections.emptyMap()); + MappedFieldType fieldType = new RankFeaturesFieldMapper.RankFeaturesFieldType("field", Collections.emptyMap(), true); assertFalse(fieldType.isAggregatable()); } } diff --git a/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_feature/10_basic.yml b/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_feature/10_basic.yml index 6fea35eb21f..ac951263ca2 100644 --- a/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_feature/10_basic.yml +++ b/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_feature/10_basic.yml @@ -157,3 +157,24 @@ setup: - match: hits.hits.1._id: "1" + +--- +"Negative linear": + + - do: + search: + index: test + body: + query: + rank_feature: + field: url_length + linear: {} + + - match: + hits.total.value: 2 + + - match: + hits.hits.0._id: "2" + + - match: + hits.hits.1._id: "1" diff --git a/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_features/10_basic.yml b/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_features/10_basic.yml index d4d5d2a3604..2644b9e777f 100644 --- a/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_features/10_basic.yml +++ b/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_features/10_basic.yml @@ -9,6 +9,9 @@ setup: properties: tags: type: rank_features + negative_reviews: + type: rank_features + positive_score_impact: false - do: index: @@ -18,6 +21,9 @@ setup: tags: foo: 3 bar: 5 + negative_reviews: + 1star: 10 + 2star: 1 - do: index: @@ -27,6 +33,9 @@ setup: tags: bar: 6 quux: 10 + negative_reviews: + 1star: 1 + 2star: 10 - do: indices.refresh: {} @@ -97,3 +106,34 @@ setup: - match: hits.hits.1._id: "1" + +--- +"Linear negative impact": + + - do: + search: + index: test + body: + query: + rank_feature: + field: negative_reviews.1star + linear: {} + + - match: + hits.hits.0._id: "2" + - match: + hits.hits.1._id: "1" + + - do: + search: + index: test + body: + query: + rank_feature: + field: negative_reviews.2star + linear: {} + + - match: + hits.hits.0._id: "1" + - match: + hits.hits.1._id: "2"