Improve similarity integration.

docs/reference/index-modules/similarity.asciidoc

@@ -82,20 +82,6 @@ This similarity has the following options:
 
 Type name: `BM25`
 
-[float]
-[[classic-similarity]]
-==== Classic similarity
-
-The classic similarity that is based on the TF/IDF model. This
-similarity has the following option:
-
-`discount_overlaps`::
-    Determines whether overlap tokens (Tokens with
-    0 position increment) are ignored when computing norm. By default this
-    is true, meaning overlap tokens do not count when computing norms.
-
-Type name: `classic`
-
 [float]
 [[dfr]]
 ==== DFR similarity
@@ -541,7 +527,7 @@ PUT /index
     "index": {
       "similarity": {
         "default": {
-          "type": "classic"
+          "type": "boolean"
         }
       }
     }
@@ -563,7 +549,7 @@ PUT /index/_settings
   "index": {
     "similarity": {
       "default": {
-        "type": "classic"
+        "type": "boolean"
       }
     }
   }

docs/reference/mapping/params/similarity.asciidoc

@@ -44,13 +44,9 @@ PUT my_index
         "default_field": { <1>
           "type": "text"
         },
-        "classic_field": {
-          "type": "text",
-          "similarity": "classic" <2>
-        },
         "boolean_sim_field": {
           "type": "text",
-          "similarity": "boolean" <3>
+          "similarity": "boolean" <2>
         }
       }
     }
@@ -59,5 +55,4 @@ PUT my_index
 --------------------------------------------------
 // CONSOLE
 <1> The `default_field` uses the `BM25` similarity.
-<2> The `classic_field` uses the `classic` similarity (ie TF/IDF).
-<3> The `boolean_sim_field` uses the `boolean` similarity.
+<2> The `boolean_sim_field` uses the `boolean` similarity.

docs/reference/migration/migrate_7_0/mappings.asciidoc

@@ -24,3 +24,16 @@ the index setting `index.mapping.nested_objects.limit`.
 ==== The `update_all_types` option has been removed
 
 This option is useless now that all indices have at most one type.
+
+=== The `classic` similarity has been removed
+
+The `classic` similarity relied on coordination factors for scoring to be good
+in presence of stopwords in the query. This feature has been removed from
+Lucene, which means that the `classic` similarity now produces scores of lower
+quality. It is advised to switch to `BM25` instead, which is widely accepted
+as a better alternative.
+
+=== Similarities fail when unsupported options are provided
+
+An error will now be thrown when unknown configuration options are provided
+to similarities. Such unknown parameters were ignored before.

modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java

@@ -337,9 +337,7 @@ public void testNonDefaultSimilarity() throws Exception {
             hasChildQuery(CHILD_DOC, new TermQueryBuilder("custom_string", "value"), ScoreMode.None);
         HasChildQueryBuilder.LateParsingQuery query = (HasChildQueryBuilder.LateParsingQuery) hasChildQueryBuilder.toQuery(shardContext);
         Similarity expected = SimilarityService.BUILT_IN.get(similarity)
-            .create(similarity, Settings.EMPTY,
-                    Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build(), null)
-            .get();
+            .apply(Settings.EMPTY, Version.CURRENT, null);
         assertThat(((PerFieldSimilarityWrapper) query.getSimilarity()).get("custom_string"), instanceOf(expected.getClass()));
     }
 

modules/parent-join/src/test/java/org/elasticsearch/join/query/LegacyHasChildQueryBuilderTests.java

@@ -323,9 +323,7 @@ public void testNonDefaultSimilarity() throws Exception {
             hasChildQuery(CHILD_TYPE, new TermQueryBuilder("custom_string", "value"), ScoreMode.None);
         HasChildQueryBuilder.LateParsingQuery query = (HasChildQueryBuilder.LateParsingQuery) hasChildQueryBuilder.toQuery(shardContext);
         Similarity expected = SimilarityService.BUILT_IN.get(similarity)
-            .create(similarity, Settings.EMPTY,
-                    Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build(), null)
-            .get();
+            .apply(Settings.EMPTY, Version.CURRENT, null);
         assertThat(((PerFieldSimilarityWrapper) query.getSimilarity()).get("custom_string"), instanceOf(expected.getClass()));
     }
 

server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java

@@ -21,7 +21,9 @@
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.apache.logging.log4j.util.Supplier;
 import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.search.similarities.Similarity;
 import org.elasticsearch.Version;
+import org.elasticsearch.common.TriFunction;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.settings.IndexScopedSettings;
 import org.elasticsearch.common.settings.Settings;
@@ -32,8 +34,8 @@
 import org.elasticsearch.index.analysis.NamedAnalyzer;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.similarity.SimilarityService;
-import org.elasticsearch.index.similarity.SimilarityProvider;
 import org.elasticsearch.indices.mapper.MapperRegistry;
+import org.elasticsearch.script.ScriptService;
 
 import java.util.AbstractMap;
 import java.util.Collection;
@@ -143,22 +145,23 @@ private void checkMappingsCompatibility(IndexMetaData indexMetaData) {
 
             IndexSettings indexSettings = new IndexSettings(indexMetaData, this.settings);
 
-            final Map<String, SimilarityProvider.Factory> similarityMap = new AbstractMap<String, SimilarityProvider.Factory>() {
+            final Map<String, TriFunction<Settings, Version, ScriptService, Similarity>> similarityMap
+                    = new AbstractMap<String, TriFunction<Settings, Version, ScriptService, Similarity>>() {
                 @Override
                 public boolean containsKey(Object key) {
                     return true;
                 }
 
                 @Override
-                public SimilarityProvider.Factory get(Object key) {
+                public TriFunction<Settings, Version, ScriptService, Similarity> get(Object key) {
                     assert key instanceof String : "key must be a string but was: " + key.getClass();
                     return SimilarityService.BUILT_IN.get(SimilarityService.DEFAULT_SIMILARITY);
                 }
 
                 // this entrySet impl isn't fully correct but necessary as SimilarityService will iterate
                 // over all similarities
                 @Override
-                public Set<Entry<String, SimilarityProvider.Factory>> entrySet() {
+                public Set<Entry<String, TriFunction<Settings, Version, ScriptService, Similarity>>> entrySet() {
                     return Collections.emptySet();
                 }
             };

server/src/main/java/org/elasticsearch/index/IndexModule.java

@@ -19,9 +19,12 @@
 
 package org.elasticsearch.index;
 
+import org.apache.lucene.search.similarities.Similarity;
 import org.apache.lucene.util.SetOnce;
+import org.elasticsearch.Version;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.TriFunction;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
@@ -40,7 +43,6 @@
 import org.elasticsearch.index.shard.IndexingOperationListener;
 import org.elasticsearch.index.shard.SearchOperationListener;
 import org.elasticsearch.index.shard.ShardId;
-import org.elasticsearch.index.similarity.BM25SimilarityProvider;
 import org.elasticsearch.index.similarity.SimilarityProvider;
 import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.index.store.IndexStore;
@@ -107,7 +109,7 @@ public final class IndexModule {
     final SetOnce<EngineFactory> engineFactory = new SetOnce<>();
     private SetOnce<IndexSearcherWrapperFactory> indexSearcherWrapper = new SetOnce<>();
     private final Set<IndexEventListener> indexEventListeners = new HashSet<>();
-    private final Map<String, SimilarityProvider.Factory> similarities = new HashMap<>();
+    private final Map<String, TriFunction<Settings, Version, ScriptService, Similarity>> similarities = new HashMap<>();
     private final Map<String, Function<IndexSettings, IndexStore>> storeTypes = new HashMap<>();
     private final SetOnce<BiFunction<IndexSettings, IndicesQueryCache, QueryCache>> forceQueryCacheProvider = new SetOnce<>();
     private final List<SearchOperationListener> searchOperationListeners = new ArrayList<>();
@@ -251,7 +253,7 @@ public void addIndexStore(String type, Function<IndexSettings, IndexStore> provi
      * @param name Name of the SimilarityProvider
      * @param similarity SimilarityProvider to register
      */
-    public void addSimilarity(String name, SimilarityProvider.Factory similarity) {
+    public void addSimilarity(String name, TriFunction<Settings, Version, ScriptService, Similarity> similarity) {
         ensureNotFrozen();
         if (similarities.containsKey(name) || SimilarityService.BUILT_IN.containsKey(name)) {
             throw new IllegalArgumentException("similarity for name: [" + name + " is already registered");