Add tests for disabling positions and copy the check to text fields

core/src/main/java/org/elasticsearch/index/mapper/core/StringFieldMapper.java

@@ -163,14 +163,6 @@ public class StringFieldMapper extends FieldMapper implements AllFieldMapper.Inc
 
         @Override
         public StringFieldMapper build(BuilderContext context) {
-            if (positionIncrementGap != POSITION_INCREMENT_GAP_USE_ANALYZER) {
-                if (fieldType.tokenized() == false && fieldType.indexOptions() != IndexOptions.NONE) {
-                    throw new IllegalArgumentException("Cannot set position_increment_gap on not_analyzed field [" + fieldType.name() + "]");
-                }
-                fieldType.setIndexAnalyzer(new NamedAnalyzer(fieldType.indexAnalyzer(), positionIncrementGap));
-                fieldType.setSearchAnalyzer(new NamedAnalyzer(fieldType.searchAnalyzer(), positionIncrementGap));
-                fieldType.setSearchQuoteAnalyzer(new NamedAnalyzer(fieldType.searchQuoteAnalyzer(), positionIncrementGap));
-            }
             // if the field is not analyzed, then by default, we should omit norms and have docs only
             // index options, as probably what the user really wants
             // if they are set explicitly, we will use those values
@@ -186,6 +178,15 @@ public class StringFieldMapper extends FieldMapper implements AllFieldMapper.Inc
                     fieldType.setIndexOptions(IndexOptions.DOCS);
                 }
             }
+            if (positionIncrementGap != POSITION_INCREMENT_GAP_USE_ANALYZER) {
+                if (fieldType.indexOptions().compareTo(IndexOptions.DOCS_AND_FREQS_AND_POSITIONS) < 0) {
+                    throw new IllegalArgumentException("Cannot set position_increment_gap on field ["
+                        + name + "] without positions enabled");
+                }
+                fieldType.setIndexAnalyzer(new NamedAnalyzer(fieldType.indexAnalyzer(), positionIncrementGap));
+                fieldType.setSearchAnalyzer(new NamedAnalyzer(fieldType.searchAnalyzer(), positionIncrementGap));
+                fieldType.setSearchQuoteAnalyzer(new NamedAnalyzer(fieldType.searchQuoteAnalyzer(), positionIncrementGap));
+            }
             setupFieldType(context);
             StringFieldMapper fieldMapper = new StringFieldMapper(
                     name, fieldType(), defaultFieldType, positionIncrementGap, ignoreAbove,

core/src/main/java/org/elasticsearch/index/mapper/core/TextFieldMapper.java

@@ -119,6 +119,10 @@ public class TextFieldMapper extends FieldMapper implements AllFieldMapper.Inclu
         @Override
         public TextFieldMapper build(BuilderContext context) {
             if (positionIncrementGap != POSITION_INCREMENT_GAP_USE_ANALYZER) {
+                if (fieldType.indexOptions().compareTo(IndexOptions.DOCS_AND_FREQS_AND_POSITIONS) < 0) {
+                    throw new IllegalArgumentException("Cannot set position_increment_gap on field ["
+                        + name + "] without positions enabled");
+                }
                 fieldType.setIndexAnalyzer(new NamedAnalyzer(fieldType.indexAnalyzer(), positionIncrementGap));
                 fieldType.setSearchAnalyzer(new NamedAnalyzer(fieldType.searchAnalyzer(), positionIncrementGap));
                 fieldType.setSearchQuoteAnalyzer(new NamedAnalyzer(fieldType.searchQuoteAnalyzer(), positionIncrementGap));

core/src/test/java/org/elasticsearch/index/mapper/core/TextFieldMapperTests.java

@@ -44,6 +44,7 @@ import org.elasticsearch.test.ESSingleNodeTestCase;
 import org.junit.Before;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -474,4 +475,32 @@ public class TextFieldMapperTests extends ESSingleNodeTestCase {
         Exception e = expectThrows(MapperParsingException.class, () -> parser.parse("type", new CompressedXContent(mapping)));
         assertEquals("[analyzer] must not have a [null] value", e.getMessage());
     }
+
+    public void testNotIndexedFieldPositionIncrement() throws IOException {
+        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("properties").startObject("field")
+            .field("type", "text")
+            .field("index", false)
+            .field("position_increment_gap", 10)
+            .endObject().endObject().endObject().endObject().string();
+
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> parser.parse("type", new CompressedXContent(mapping)));
+        assertEquals("Cannot set position_increment_gap on field [field] without positions enabled", e.getMessage());
+    }
+
+    public void testAnalyzedFieldPositionIncrementWithoutPositions() throws IOException {
+        for (String indexOptions : Arrays.asList("docs", "freqs")) {
+            String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+                .startObject("properties").startObject("field")
+                .field("type", "text")
+                .field("index_options", indexOptions)
+                .field("position_increment_gap", 10)
+                .endObject().endObject().endObject().endObject().string();
+
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> parser.parse("type", new CompressedXContent(mapping)));
+            assertEquals("Cannot set position_increment_gap on field [field] without positions enabled", e.getMessage());
+        }
+    }
 }

core/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java

@@ -678,16 +678,33 @@ public class SimpleStringMappingTests extends ESSingleNodeTestCase {
         assertThat(e.getMessage(), containsString("Fielddata is disabled"));
     }
 
-    public void testKeywordFieldPositionIncrement() throws IOException {
+    public void testNonAnalyzedFieldPositionIncrement() throws IOException {
-        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+        for (String index : Arrays.asList("no", "not_analyzed")) {
-            .startObject("properties").startObject("field")
+            String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
-            .field("type", "string")
+                .startObject("properties").startObject("field")
-            .field("index", "not_analyzed")
+                .field("type", "string")
-            .field("position_increment_gap", 10)
+                .field("index", index)
-            .endObject().endObject().endObject().endObject().string();
+                .field("position_increment_gap", 10)
+                .endObject().endObject().endObject().endObject().string();
 
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
-            () -> parser.parse("type", new CompressedXContent(mapping)));
+                () -> parser.parse("type", new CompressedXContent(mapping)));
-        assertThat(e.getMessage(), containsString("Cannot set position_increment_gap on not_analyzed field"));
+            assertEquals("Cannot set position_increment_gap on field [field] without positions enabled", e.getMessage());
+        }
+    }
+
+    public void testAnalyzedFieldPositionIncrementWithoutPositions() throws IOException {
+        for (String indexOptions : Arrays.asList("docs", "freqs")) {
+            String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+                .startObject("properties").startObject("field")
+                .field("type", "string")
+                .field("index_options", indexOptions)
+                .field("position_increment_gap", 10)
+                .endObject().endObject().endObject().endObject().string();
+
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> parser.parse("type", new CompressedXContent(mapping)));
+            assertEquals("Cannot set position_increment_gap on field [field] without positions enabled", e.getMessage());
+        }
     }
 }