OAK-12353: Group Elasticsearch dynamic boost nested documents by boost score

oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java

@@ -22,6 +22,7 @@
 import org.apache.jackrabbit.oak.plugins.index.AsyncIndexInfoService;
 import org.apache.jackrabbit.oak.plugins.index.IndexEditorProvider;
 import org.apache.jackrabbit.oak.plugins.index.IndexInfoProvider;
+import org.apache.jackrabbit.oak.plugins.index.elastic.index.ElasticDocument;
 import org.apache.jackrabbit.oak.plugins.index.elastic.index.ElasticIndexEditorProvider;
 import org.apache.jackrabbit.oak.plugins.index.elastic.index.ElasticRetryPolicy;
 import org.apache.jackrabbit.oak.plugins.index.elastic.query.ElasticIndexProvider;
@@ -236,6 +237,9 @@ private void activate(BundleContext bundleContext, Config config) {
         oakRegs.add(whiteboard.register(FeatureToggle.class,
                 new FeatureToggle(ElasticIndexStatistics.FT_OAK_12248, ElasticIndexStatistics.FT_OAK_12248_ENABLE),
                 emptyMap()));
+        oakRegs.add(whiteboard.register(FeatureToggle.class,
+                new FeatureToggle(ElasticDocument.FT_OAK_12353, ElasticDocument.FT_OAK_12353_ENABLE),
+                emptyMap()));
         if (System.getProperty(QueryEngineSettings.OAK_INFERENCE_ENABLED) != null) {
             this.isInferenceEnabled = Boolean.parseBoolean(System.getProperty(QueryEngineSettings.OAK_INFERENCE_ENABLED));
         } else {

oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticDocument.java

@@ -35,13 +35,27 @@
 import java.util.Map;
 import java.util.HashMap;
 import java.util.Set;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.stream.Collectors;
 
 import static org.apache.jackrabbit.oak.plugins.index.elastic.util.ElasticIndexUtils.toFloats;
 
 @JsonInclude(JsonInclude.Include.NON_EMPTY)
 public class ElasticDocument {
 
+    public static final String FT_OAK_12353 = "FT_OAK-12353";
+    /**
+     * When {@code true}, dynamic boost values sharing the same boost score are grouped into a
+     * single nested document per property, with {@code value} holding an array of the grouped
+     * values, instead of one nested document per value. This reduces the number of nested
+     * documents generated for properties with many dynamic-boost values that share a boost score.
+     * Default is {@code true} (feature enabled); set to {@code false} via the feature toggle to
+     * revert to the pre-fix behaviour of one nested document per value.
+     */
+    public static final AtomicBoolean FT_OAK_12353_ENABLE = new AtomicBoolean(true);
+
     @JsonProperty(FieldNames.PATH)
     public final String path;
     @JsonProperty(ElasticIndexDefinition.PATH_RANDOM_VALUE)
@@ -66,6 +80,12 @@ public class ElasticDocument {
     @JsonProperty(ElasticIndexDefinition.LAST_UPDATED)
     private long lastUpdated;
 
+    // fieldName -> boost -> values sharing that boost. Only populated when FT_OAK_12353_ENABLE is
+    // true, in which case it replaces the corresponding entries that would otherwise be added to
+    // "properties" directly by addDynamicBoostField.
+    @JsonIgnore
+    private final Map<String, LinkedHashMap<Double, LinkedHashSet<String>>> dynamicBoostGroups;
+
     // Internal set with properties that need to be removed from the document on update operations
     @JsonIgnore
     private final Set<String> propertiesToRemove;
@@ -87,6 +107,7 @@ public class ElasticDocument {
         this.dbFullText = new LinkedHashSet<>();
         this.similarityTags = new LinkedHashSet<>();
         this.propertiesToRemove = new HashSet<>();
+        this.dynamicBoostGroups = new LinkedHashMap<>();
     }
 
     void addFulltext(String value) {
@@ -175,12 +196,18 @@ void indexAncestors(String path) {
     }
 
     void addDynamicBoostField(String fieldName, String value, double boost) {
-        addProperty(fieldName,
-                Map.of(
-                        ElasticIndexHelper.DYNAMIC_BOOST_NESTED_VALUE, value,
-                        ElasticIndexHelper.DYNAMIC_BOOST_NESTED_BOOST, boost
-                )
-        );
+        if (FT_OAK_12353_ENABLE.get()) {
+            dynamicBoostGroups.computeIfAbsent(fieldName, k -> new LinkedHashMap<>())
+                    .computeIfAbsent(boost, k -> new LinkedHashSet<>())
+                    .add(value);
+        } else {
+            addProperty(fieldName,
+                    Map.of(
+                            ElasticIndexHelper.DYNAMIC_BOOST_NESTED_VALUE, value,
+                            ElasticIndexHelper.DYNAMIC_BOOST_NESTED_BOOST, boost
+                    )
+            );
+        }
 
         // add value into the dynamic boost specific fulltext field. We cannot add this in the standard
         // field since dynamic boosted terms require lower weight compared to standard terms
@@ -197,7 +224,21 @@ void setLastUpdated(long lastUpdated) {
 
     @JsonAnyGetter
     public Map<String, Object> getProperties() {
-        return properties;
+        if (dynamicBoostGroups.isEmpty()) {
+            return properties;
+        }
+        Map<String, Object> merged = new LinkedHashMap<>(properties);
+        dynamicBoostGroups.forEach((fieldName, boostToValues) -> {
+            Set<Object> nestedDocs = boostToValues.entrySet().stream()
+                    .map(entry -> Map.of(
+                            ElasticIndexHelper.DYNAMIC_BOOST_NESTED_VALUE,
+                            entry.getValue().size() == 1 ? entry.getValue().iterator().next() : entry.getValue(),
+                            ElasticIndexHelper.DYNAMIC_BOOST_NESTED_BOOST, entry.getKey()
+                    ))
+                    .collect(Collectors.toCollection(LinkedHashSet::new));
+            merged.put(fieldName, nestedDocs);
+        });
+        return merged;
     }
 
     public void removeProperty(String fieldName) {
@@ -222,6 +263,9 @@ public String toString() {
         if (!dynamicProperties.isEmpty()) {
             buff.append("dynamicProperties:").append(dynamicProperties).append('\n');
         }
+        if (!dynamicBoostGroups.isEmpty()) {
+            buff.append("dynamicBoostGroups:").append(dynamicBoostGroups).append('\n');
+        }
         return buff.toString();
     }
 

oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexHelper.java

@@ -350,9 +350,13 @@ private static void mapIndexRules(@NotNull TypeMapping.Builder builder,
             for (PropertyDefinition pd : indexDefinition.getDynamicBoostProperties()) {
                 builder.properties(ElasticIndexUtils.fieldName(pd.nodeName),
                         b1 -> b1.nested(
+                                // norms disabled: values sharing a boost score are grouped into a single nested
+                                // doc (see ElasticDocument#FT_OAK_12353), so field length varies by group size and
+                                // would otherwise skew BM25 length normalization; boost is applied explicitly via
+                                // field_value_factor, so length normalization on this field isn't meaningful anyway.
                                 b2 -> b2.properties(DYNAMIC_BOOST_NESTED_VALUE,
                                                 b3 -> b3.text(
-                                                        b4 -> b4.analyzer("oak_analyzer")))
+                                                        b4 -> b4.analyzer("oak_analyzer").norms(false)))
                                         .properties(DYNAMIC_BOOST_NESTED_BOOST,
                                                 b3 -> b3.double_(f -> f)
                                         )

oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticDynamicBoostTest.java

@@ -20,6 +20,8 @@
 import org.apache.jackrabbit.oak.api.ContentRepository;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.plugins.index.DynamicBoostCommonTest;
+import org.apache.jackrabbit.oak.plugins.index.elastic.index.ElasticDocument;
+import org.junit.After;
 import org.junit.ClassRule;
 import org.junit.Test;
 
@@ -85,6 +87,63 @@ public void dynamicBoostAnalyzed() throws Exception {
         });
     }
 
+    @After
+    public void resetDynamicBoostGroupingToggle() {
+        ElasticDocument.FT_OAK_12353_ENABLE.set(true);
+    }
+
+    /**
+     * Predicted tags sharing the same boost score are grouped into a single nested document
+     * (see {@link ElasticDocument#FT_OAK_12353_ENABLE}). This verifies that querying still
+     * matches on any of the grouped values, both with the grouping enabled (default) and
+     * disabled.
+     */
+    @Test
+    public void dynamicBoostQueriesGroupedValuesSharingSameBoostScore() throws Exception {
+        createAssetsIndexAndProperties(false, false);
+
+        Tree testParent = createNodeWithType(root.getTree("/"), "test", JcrConstants.NT_UNSTRUCTURED, "");
+
+        Tree predicted1 = createAssetNodeWithPredicted(testParent, "asset1", "flower with a lot of red and a bit of blue");
+        createPredictedTag(predicted1, "red", 5.0);
+        createPredictedTag(predicted1, "blue", 5.0);
+        createPredictedTag(predicted1, "green", 5.0);
+        createPredictedTag(predicted1, "special", 9.0);
+
+        root.commit();
+
+        assertEventually(() -> {
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'red')]", XPATH, List.of("/test/asset1"));
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'blue')]", XPATH, List.of("/test/asset1"));
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'green')]", XPATH, List.of("/test/asset1"));
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'special')]", XPATH, List.of("/test/asset1"));
+        });
+    }
+
+    @Test
+    public void dynamicBoostQueriesValuesSharingSameBoostScoreWhenGroupingDisabled() throws Exception {
+        ElasticDocument.FT_OAK_12353_ENABLE.set(false);
+
+        createAssetsIndexAndProperties(false, false);
+
+        Tree testParent = createNodeWithType(root.getTree("/"), "test", JcrConstants.NT_UNSTRUCTURED, "");
+
+        Tree predicted1 = createAssetNodeWithPredicted(testParent, "asset1", "flower with a lot of red and a bit of blue");
+        createPredictedTag(predicted1, "red", 5.0);
+        createPredictedTag(predicted1, "blue", 5.0);
+        createPredictedTag(predicted1, "green", 5.0);
+        createPredictedTag(predicted1, "special", 9.0);
+
+        root.commit();
+
+        assertEventually(() -> {
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'red')]", XPATH, List.of("/test/asset1"));
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'blue')]", XPATH, List.of("/test/asset1"));
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'green')]", XPATH, List.of("/test/asset1"));
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'special')]", XPATH, List.of("/test/asset1"));
+        });
+    }
+
     @Test
     public void dynamicBoostNotIncludedInFullText() throws Exception {
         createAssetsIndexAndProperties(false, false, false);

oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticDocumentTest.java

@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.index.elastic.index;
+
+import org.junit.After;
+import org.junit.Test;
+
+import java.time.LocalDate;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class ElasticDocumentTest {
+
+    @After
+    public void resetToggle() {
+        ElasticDocument.FT_OAK_12353_ENABLE.set(true);
+    }
+
+    @Test
+    public void dynamicBoostValuesAreNotGroupedWhenToggleDisabled() {
+        ElasticDocument.FT_OAK_12353_ENABLE.set(false);
+
+        ElasticDocument doc = new ElasticDocument("/test");
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "Replacement Cost", 1.0);
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "Theft", 1.0);
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "GENERAL INSURANCE COMPANY", 0.988);
+
+        Object value = doc.getProperties().get("predictedTagsDynamicBoost");
+        assertTrue(value instanceof Set);
+        @SuppressWarnings("unchecked")
+        Set<Map<String, Object>> nestedDocs = (Set<Map<String, Object>>) value;
+        assertEquals(3, nestedDocs.size());
+        for (Map<String, Object> nestedDoc : nestedDocs) {
+            assertTrue(nestedDoc.get(ElasticIndexHelper.DYNAMIC_BOOST_NESTED_VALUE) instanceof String);
+        }
+    }
+
+    @Test
+    public void dynamicBoostValuesAreGroupedByBoostByDefault() {
+        ElasticDocument doc = new ElasticDocument("/test");
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "Replacement Cost", 1.0);
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "Theft", 1.0);
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "Alberta", 1.0);
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "GENERAL INSURANCE COMPANY", 0.988);
+
+        Object value = doc.getProperties().get("predictedTagsDynamicBoost");
+        assertTrue(value instanceof Set);
+        @SuppressWarnings("unchecked")
+        Set<Map<String, Object>> nestedDocs = (Set<Map<String, Object>>) value;
+        // one nested doc for the 3 values sharing boost=1.0, one for the distinct boost=0.988
+        assertEquals(2, nestedDocs.size());
+
+        boolean foundGrouped = false;
+        boolean foundSingle = false;
+        for (Map<String, Object> nestedDoc : nestedDocs) {
+            Object boost = nestedDoc.get(ElasticIndexHelper.DYNAMIC_BOOST_NESTED_BOOST);
+            Object nestedValue = nestedDoc.get(ElasticIndexHelper.DYNAMIC_BOOST_NESTED_VALUE);
+            if (Double.valueOf(1.0).equals(boost)) {
+                assertTrue(nestedValue instanceof Collection);
+                @SuppressWarnings("unchecked")
+                Collection<String> values = (Collection<String>) nestedValue;
+                assertEquals(List.of("Replacement Cost", "Theft", "Alberta"), new ArrayList<>(values));
+                foundGrouped = true;
+            } else if (Double.valueOf(0.988).equals(boost)) {
+                assertEquals("GENERAL INSURANCE COMPANY", nestedValue);
+                foundSingle = true;
+            }
+        }
+        assertTrue(foundGrouped);
+        assertTrue(foundSingle);
+    }
+
+    @Test
+    public void ft_oak_12353_toggleShouldBeRemoved() {
+        // Time-bombed: if this test fails, the feature toggle FT_OAK-12353 and its guard in
+        // ElasticDocument#addDynamicBoostField/#getProperties should be removed — the grouping
+        // has been enabled by default in production long enough.
+        assertTrue("Feature toggle " + ElasticDocument.FT_OAK_12353 + " is overdue for removal",
+                LocalDate.now().isBefore(LocalDate.of(2027, 8, 12)));
+    }
+}

oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexHelperTest.java

@@ -59,6 +59,30 @@ public void manyFields() {
         assertEquals(true, request.settings().index().mapping().ignoreMalformed());
     }
 
+    @Test
+    public void dynamicBoostValueFieldHasNormsDisabled() {
+        IndexDefinitionBuilder builder = new ElasticIndexDefinitionBuilder();
+        IndexDefinitionBuilder.IndexRule indexRuleA = builder.indexRule("typeA");
+        indexRuleA.property("foo").type("String");
+        indexRuleA.property("predictedTagsDynamicBoost", "jcr:content/metadata/predictedTags/.*", true)
+                .getBuilderTree().setProperty(FulltextIndexConstants.PROP_DYNAMIC_BOOST, true);
+        NodeState nodeState = builder.build();
+
+        ElasticIndexDefinition definition =
+            new ElasticIndexDefinition(nodeState, nodeState, "path", "prefix");
+        CreateIndexRequest request = ElasticIndexHelper.createIndexRequest("prefix.path", definition);
+
+        Property dynamicBoostField = request.mappings().properties()
+                .get(ElasticIndexUtils.fieldName("predictedTagsDynamicBoost"));
+        assertThat(dynamicBoostField, notNullValue());
+        assertThat(dynamicBoostField._kind(), is(Property.Kind.Nested));
+
+        Property valueField = dynamicBoostField.nested().properties().get(ElasticIndexHelper.DYNAMIC_BOOST_NESTED_VALUE);
+        assertThat(valueField, notNullValue());
+        assertThat(valueField._kind(), is(Property.Kind.Text));
+        assertEquals(false, valueField.text().norms());
+    }
+
     @Test
     public void multiRulesWithSamePropertyNames() {
         IndexDefinitionBuilder builder = new ElasticIndexDefinitionBuilder();

oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/FulltextIndexConstants.java

@@ -485,6 +485,6 @@ public static IndexingMode from(String indexingMode) {
      * needed from an outside process that does not have visibility to the specific index module.
      */
     Map<String, String> INDEX_VERSION_BY_TYPE = Map.of(
-            "elasticsearch", "1.4.0"
+            "elasticsearch", "1.5.0"
     );
 }