From 03d717dc32b526c588a60cfa7bf12c1fb5880da4 Mon Sep 17 00:00:00 2001
From: Ignacio Vera <ivera@apache.org>
Date: Tue, 1 Oct 2019 10:52:39 +0200
Subject: [PATCH] Provide better error when updating geo_shape field mapper
 settings (#47281) (#47338)

---
 .../index/mapper/GeoShapeFieldMapper.java     | 11 ++++++++++
 .../mapper/LegacyGeoShapeFieldMapper.java     | 11 ++++++++++
 .../search/geo/GeoShapeIntegrationIT.java     | 22 +++++++++++++++++++
 3 files changed, 44 insertions(+)

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java
index aa5a74c4370..a8015d35087 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java
@@ -104,6 +104,17 @@ public class GeoShapeFieldMapper extends AbstractGeometryFieldMapper<Geometry, G
             multiFields, copyTo);
     }
 
+    @Override
+    protected void doMerge(Mapper mergeWith) {
+        if (mergeWith instanceof LegacyGeoShapeFieldMapper) {
+            LegacyGeoShapeFieldMapper legacy = (LegacyGeoShapeFieldMapper) mergeWith;
+            throw new IllegalArgumentException("[" + fieldType().name() + "] with field mapper [" + fieldType().typeName() + "] " +
+                "using [BKD] strategy cannot be merged with " + "[" + legacy.fieldType().typeName() + "] with [" +
+                legacy.fieldType().strategy() + "] strategy");
+        }
+        super.doMerge(mergeWith);
+    }
+
     @Override
     public GeoShapeFieldType fieldType() {
         return (GeoShapeFieldType) super.fieldType();
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java
index 0a76a477d87..907a7b2e901 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java
@@ -536,6 +536,17 @@ public class LegacyGeoShapeFieldMapper extends AbstractGeometryFieldMapper<Shape
         }
     }
 
+    @Override
+    protected void doMerge(Mapper mergeWith) {
+        if (mergeWith instanceof GeoShapeFieldMapper) {
+            GeoShapeFieldMapper fieldMapper = (GeoShapeFieldMapper) mergeWith;
+            throw new IllegalArgumentException("[" + fieldType().name() + "] with field mapper [" + fieldType().typeName() + "] " +
+                "using [" + fieldType().strategy() + "] strategy cannot be merged with " + "[" + fieldMapper.typeName() +
+                "] with [BKD] strategy");
+        }
+        super.doMerge(mergeWith);
+    }
+
     @Override
     protected String contentType() {
         return CONTENT_TYPE;
diff --git a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java
index a7faa040172..67efde49486 100644
--- a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java
+++ b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java
@@ -35,6 +35,7 @@ import org.elasticsearch.test.ESIntegTestCase;
 import static org.elasticsearch.index.query.QueryBuilders.geoShapeQuery;
 import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 
@@ -123,6 +124,27 @@ public class GeoShapeIntegrationIT extends ESIntegTestCase {
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
     }
 
+    public void testMappingUpdate() throws Exception {
+        // create index
+        assertAcked(client().admin().indices().prepareCreate("test")
+            .addMapping("geometry", "shape", "type=geo_shape").get());
+        ensureGreen();
+
+        String update ="{\n" +
+            "  \"properties\": {\n" +
+            "    \"shape\": {\n" +
+            "      \"type\": \"geo_shape\",\n" +
+            "      \"strategy\": \"recursive\"\n" +
+            "    }\n" +
+            "  }\n" +
+            "}";
+
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin().indices()
+            .preparePutMapping("test").setType("geometry")
+            .setSource(update, XContentType.JSON).get());
+        assertThat(e.getMessage(), containsString("using [BKD] strategy cannot be merged with"));
+    }
+
     /**
      * Test that the indexed shape routing can be provided if it is required
      */