From 9d9133a451d3fbfe441d4aecb97a5be0e761f37e Mon Sep 17 00:00:00 2001 From: Shay Banon Date: Sat, 24 Sep 2011 01:59:21 +0300 Subject: [PATCH] required _routing fails when path points to an integer field, closes #1357. --- .../index/mapper/core/ByteFieldMapper.java | 4 ++++ .../index/mapper/core/DoubleFieldMapper.java | 4 ++++ .../index/mapper/core/FloatFieldMapper.java | 4 ++++ .../index/mapper/core/IntegerFieldMapper.java | 4 ++++ .../index/mapper/core/LongFieldMapper.java | 4 ++++ .../index/mapper/core/NumberFieldMapper.java | 2 ++ .../index/mapper/core/ShortFieldMapper.java | 4 ++++ .../mapper/internal/RoutingFieldMapper.java | 16 ++++++++++++- .../routing/SimpleRoutingTests.java | 24 +++++++++++++++++++ 9 files changed, 65 insertions(+), 1 deletion(-) diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java index 47abfb0bb4c..529f97a4cbf 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java @@ -307,5 +307,9 @@ public class ByteFieldMapper extends NumberFieldMapper { } return null; } + + @Override public String numericAsString() { + return Byte.toString(number); + } } } \ No newline at end of file diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java index b927db76294..2542071c8ce 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java @@ -308,5 +308,9 @@ public class DoubleFieldMapper extends NumberFieldMapper { } return null; } + + @Override public String numericAsString() { + return Double.toString(number); + } } } \ No newline at end of file diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java index ed68c1e62c4..ee3726bc9e5 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java @@ -304,5 +304,9 @@ public class FloatFieldMapper extends NumberFieldMapper { } return null; } + + @Override public String numericAsString() { + return Float.toString(number); + } } } \ No newline at end of file diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java index 0e053aefc4f..0b7fca922eb 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java @@ -308,5 +308,9 @@ public class IntegerFieldMapper extends NumberFieldMapper { } return null; } + + @Override public String numericAsString() { + return Integer.toString(number); + } } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java index ffa104cc813..8b5929d3724 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java @@ -307,5 +307,9 @@ public class LongFieldMapper extends NumberFieldMapper { } return null; } + + @Override public String numericAsString() { + return Long.toString(number); + } } } \ No newline at end of file diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java index f7be4fc34e5..cdb970dd354 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java @@ -248,5 +248,7 @@ public abstract class NumberFieldMapper extends AbstractFieldM @Override public Reader readerValue() { return null; } + + public abstract String numericAsString(); } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java index b443f6b896d..99e4f37c924 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java @@ -307,5 +307,9 @@ public class ShortFieldMapper extends NumberFieldMapper { } return null; } + + @Override public String numericAsString() { + return Short.toString(number); + } } } \ No newline at end of file diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/internal/RoutingFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/internal/RoutingFieldMapper.java index 420463fc2a3..8da786aa809 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/internal/RoutingFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/internal/RoutingFieldMapper.java @@ -33,6 +33,7 @@ import org.elasticsearch.index.mapper.MergeMappingException; import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.index.mapper.RootMapper; import org.elasticsearch.index.mapper.core.AbstractFieldMapper; +import org.elasticsearch.index.mapper.core.NumberFieldMapper; import java.io.IOException; import java.util.Map; @@ -156,10 +157,23 @@ public class RoutingFieldMapper extends AbstractFieldMapper implements I String routing = context.sourceToParse().routing(); if (path != null && routing != null) { // we have a path, check if we can validate we have the same routing value as the one in the doc... - String value = context.doc().get(path); + String value = null; + Fieldable field = context.doc().getFieldable(path); + if (field != null) { + value = field.stringValue(); + if (value == null) { + // maybe its a numeric field... + if (field instanceof NumberFieldMapper.CustomNumericField) { + value = ((NumberFieldMapper.CustomNumericField) field).numericAsString(); + } + } + } if (value == null) { value = context.ignoredValue(path); } + if (value == null) { + // maybe its a numeric field + } if (!routing.equals(value)) { throw new MapperParsingException("External routing [" + routing + "] and document path routing [" + value + "] mismatch"); } diff --git a/modules/test/integration/src/test/java/org/elasticsearch/test/integration/routing/SimpleRoutingTests.java b/modules/test/integration/src/test/java/org/elasticsearch/test/integration/routing/SimpleRoutingTests.java index 2c8344a3244..7df8cb273ea 100644 --- a/modules/test/integration/src/test/java/org/elasticsearch/test/integration/routing/SimpleRoutingTests.java +++ b/modules/test/integration/src/test/java/org/elasticsearch/test/integration/routing/SimpleRoutingTests.java @@ -300,4 +300,28 @@ public class SimpleRoutingTests extends AbstractNodesTests { assertThat(client.prepareGet("test", "type1", "1").setRouting("0").execute().actionGet().exists(), equalTo(true)); } } + + @Test public void testRequiredRoutingWithPathNumericType() throws Exception { + client.admin().indices().prepareDelete().execute().actionGet(); + + client.admin().indices().prepareCreate("test") + .addMapping("type1", XContentFactory.jsonBuilder().startObject().startObject("type1") + .startObject("_routing").field("required", true).field("path", "routing_field").endObject() + .endObject().endObject()) + .execute().actionGet(); + client.admin().cluster().prepareHealth().setWaitForGreenStatus().execute().actionGet(); + + logger.info("--> indexing with id [1], and routing [0]"); + client.prepareIndex("test", "type1", "1").setSource("field", "value1", "routing_field", 0).execute().actionGet(); + client.admin().indices().prepareRefresh().execute().actionGet(); + + logger.info("--> verifying get with no routing, should not find anything"); + for (int i = 0; i < 5; i++) { + assertThat(client.prepareGet("test", "type1", "1").execute().actionGet().exists(), equalTo(false)); + } + logger.info("--> verifying get with routing, should find"); + for (int i = 0; i < 5; i++) { + assertThat(client.prepareGet("test", "type1", "1").setRouting("0").execute().actionGet().exists(), equalTo(true)); + } + } }