From 7cf80aca993906ada9573e3016871c422251adb8 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Thu, 3 Jan 2013 18:22:17 +0100 Subject: [PATCH] Changed how the stored values of numeric fields are stored in the index. Before numeric values were stored in binary representation, now the values in numeric representation. --- .../index/mapper/core/ByteFieldMapper.java | 2 +- .../index/mapper/core/DoubleFieldMapper.java | 2 +- .../index/mapper/core/FloatFieldMapper.java | 2 +- .../index/mapper/core/IntegerFieldMapper.java | 2 +- .../index/mapper/core/LongFieldMapper.java | 2 +- .../index/mapper/core/NumberFieldMapper.java | 5 +- .../index/mapper/core/ShortFieldMapper.java | 2 +- .../geo/LatLonMappingGeoPointTests.java | 42 ++++--- .../lucene/StoredNumericValuesTest.java | 103 ++++++++++++++++++ 9 files changed, 131 insertions(+), 31 deletions(-) create mode 100644 src/test/java/org/elasticsearch/test/unit/index/mapper/lucene/StoredNumericValuesTest.java diff --git a/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java index f1326df26be..75b8a8ea17b 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java @@ -364,7 +364,7 @@ public class ByteFieldMapper extends NumberFieldMapper { private final NumberFieldMapper mapper; public CustomByteNumericField(NumberFieldMapper mapper, byte number, FieldType fieldType) { - super(mapper, mapper.fieldType.stored() ? new byte[]{number} : null, fieldType); + super(mapper, mapper.fieldType.stored() ? number : null, fieldType); this.mapper = mapper; this.number = number; } diff --git a/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java index 35cb15f6cdf..7fe42f66121 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java @@ -364,7 +364,7 @@ public class DoubleFieldMapper extends NumberFieldMapper { private final NumberFieldMapper mapper; public CustomDoubleNumericField(NumberFieldMapper mapper, double number, FieldType fieldType) { - super(mapper, mapper.fieldType().stored() ? Numbers.doubleToBytes(number) : null, fieldType); + super(mapper, mapper.fieldType().stored() ? number : null, fieldType); this.mapper = mapper; this.number = number; } diff --git a/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java index 1ebc4278bd3..a9b9248092f 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java @@ -359,7 +359,7 @@ public class FloatFieldMapper extends NumberFieldMapper { private final NumberFieldMapper mapper; public CustomFloatNumericField(NumberFieldMapper mapper, float number, FieldType fieldType) { - super(mapper, mapper.fieldType().stored() ? Numbers.floatToBytes(number) : null, fieldType); + super(mapper, mapper.fieldType().stored() ? number : null, fieldType); this.mapper = mapper; this.number = number; } diff --git a/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java index a3949dd24fa..3e54b432879 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java @@ -364,7 +364,7 @@ public class IntegerFieldMapper extends NumberFieldMapper { private final NumberFieldMapper mapper; public CustomIntegerNumericField(NumberFieldMapper mapper, int number, FieldType fieldType) { - super(mapper, mapper.fieldType().stored() ? Numbers.intToBytes(number) : null, fieldType); + super(mapper, mapper.fieldType().stored() ? number : null, fieldType); this.mapper = mapper; this.number = number; } diff --git a/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java index f6544248645..e49ba34a607 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java @@ -363,7 +363,7 @@ public class LongFieldMapper extends NumberFieldMapper { private final NumberFieldMapper mapper; public CustomLongNumericField(NumberFieldMapper mapper, long number, FieldType fieldType) { - super(mapper, mapper.fieldType.stored() ? Numbers.longToBytes(number) : null, fieldType); + super(mapper, mapper.fieldType.stored() ? number : null, fieldType); this.mapper = mapper; this.number = number; } diff --git a/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java index d7cf27d99d2..4f9b0df2f35 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java @@ -25,7 +25,6 @@ import org.apache.lucene.document.FieldType; import org.apache.lucene.index.FieldInfo.IndexOptions; import org.apache.lucene.search.Filter; import org.apache.lucene.search.Query; -import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.NumericUtils; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.Nullable; @@ -290,11 +289,11 @@ public abstract class NumberFieldMapper extends AbstractFieldM protected final NumberFieldMapper mapper; - public CustomNumericField(NumberFieldMapper mapper, byte[] value, FieldType fieldType) { + public CustomNumericField(NumberFieldMapper mapper, Number value, FieldType fieldType) { super(mapper.names().indexName(), fieldType); this.mapper = mapper; if (value != null) { - this.fieldsData = new BytesRef(value); + this.fieldsData = value; } } diff --git a/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java index dd7ee406598..3dd358d137b 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java @@ -367,7 +367,7 @@ public class ShortFieldMapper extends NumberFieldMapper { private final NumberFieldMapper mapper; public CustomShortNumericField(NumberFieldMapper mapper, short number, FieldType fieldType) { - super(mapper, mapper.fieldType().stored() ? Numbers.shortToBytes(number) : null, fieldType); + super(mapper, mapper.fieldType().stored() ? number : null, fieldType); this.mapper = mapper; this.number = number; } diff --git a/src/test/java/org/elasticsearch/test/unit/index/mapper/geo/LatLonMappingGeoPointTests.java b/src/test/java/org/elasticsearch/test/unit/index/mapper/geo/LatLonMappingGeoPointTests.java index cc4262b34b9..1ab84da2bb3 100644 --- a/src/test/java/org/elasticsearch/test/unit/index/mapper/geo/LatLonMappingGeoPointTests.java +++ b/src/test/java/org/elasticsearch/test/unit/index/mapper/geo/LatLonMappingGeoPointTests.java @@ -19,8 +19,6 @@ package org.elasticsearch.test.unit.index.mapper.geo; -import org.apache.lucene.util.BytesRef; -import org.elasticsearch.common.Numbers; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.MapperParsingException; @@ -187,9 +185,9 @@ public class LatLonMappingGeoPointTests { .bytes()); assertThat(doc.rootDoc().getField("point.lat"), notNullValue()); - assertThat(doc.rootDoc().getField("point.lat").binaryValue(), nullValue()); + assertThat(doc.rootDoc().getField("point.lat").numericValue(), nullValue()); assertThat(doc.rootDoc().getField("point.lon"), notNullValue()); - assertThat(doc.rootDoc().getField("point.lon").binaryValue(), nullValue()); + assertThat(doc.rootDoc().getField("point.lon").numericValue(), nullValue()); assertThat(doc.rootDoc().getField("point.geohash"), nullValue()); assertThat(doc.rootDoc().get("point"), equalTo("1.2,1.3")); } @@ -209,9 +207,9 @@ public class LatLonMappingGeoPointTests { .bytes()); assertThat(doc.rootDoc().getField("point.lat"), notNullValue()); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getField("point.lat").binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.2))); + assertThat(doc.rootDoc().getField("point.lat").numericValue().doubleValue(), equalTo(1.2)); assertThat(doc.rootDoc().getField("point.lon"), notNullValue()); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getField("point.lon").binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.3))); + assertThat(doc.rootDoc().getField("point.lon").numericValue().doubleValue(), equalTo(1.3)); assertThat(doc.rootDoc().getField("point.geohash"), nullValue()); assertThat(doc.rootDoc().get("point"), equalTo("1.2,1.3")); } @@ -235,11 +233,11 @@ public class LatLonMappingGeoPointTests { assertThat(doc.rootDoc().getFields("point.lat").length, equalTo(2)); assertThat(doc.rootDoc().getFields("point.lon").length, equalTo(2)); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lat")[0].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.2))); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lon")[0].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.3))); + assertThat(doc.rootDoc().getFields("point.lat")[0].numericValue().doubleValue(), equalTo(1.2)); + assertThat(doc.rootDoc().getFields("point.lon")[0].numericValue().doubleValue(), equalTo(1.3)); assertThat(doc.rootDoc().getFields("point")[0].stringValue(), equalTo("1.2,1.3")); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lat")[1].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.4))); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lon")[1].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.5))); + assertThat(doc.rootDoc().getFields("point.lat")[1].numericValue().doubleValue(), equalTo(1.4)); + assertThat(doc.rootDoc().getFields("point.lon")[1].numericValue().doubleValue(), equalTo(1.5)); assertThat(doc.rootDoc().getFields("point")[1].stringValue(), equalTo("1.4,1.5")); } @@ -277,9 +275,9 @@ public class LatLonMappingGeoPointTests { .bytes()); assertThat(doc.rootDoc().getField("point.lat"), notNullValue()); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getField("point.lat").binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.2))); + assertThat(doc.rootDoc().getField("point.lat").numericValue().doubleValue(), equalTo(1.2)); assertThat(doc.rootDoc().getField("point.lon"), notNullValue()); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getField("point.lon").binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.3))); + assertThat(doc.rootDoc().getField("point.lon").numericValue().doubleValue(), equalTo(1.3)); assertThat(doc.rootDoc().get("point"), equalTo("1.2,1.3")); } @@ -302,11 +300,11 @@ public class LatLonMappingGeoPointTests { assertThat(doc.rootDoc().getFields("point.lat").length, equalTo(2)); assertThat(doc.rootDoc().getFields("point.lon").length, equalTo(2)); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lat")[0].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.2))); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lon")[0].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.3))); + assertThat(doc.rootDoc().getFields("point.lat")[0].numericValue().doubleValue(), equalTo(1.2)); + assertThat(doc.rootDoc().getFields("point.lon")[0].numericValue().doubleValue(), equalTo(1.3)); assertThat(doc.rootDoc().getFields("point")[0].stringValue(), equalTo("1.2,1.3")); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lat")[1].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.4))); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lon")[1].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.5))); + assertThat(doc.rootDoc().getFields("point.lat")[1].numericValue().doubleValue(), equalTo(1.4)); + assertThat(doc.rootDoc().getFields("point.lon")[1].numericValue().doubleValue(), equalTo(1.5)); assertThat(doc.rootDoc().getFields("point")[1].stringValue(), equalTo("1.4,1.5")); } @@ -365,9 +363,9 @@ public class LatLonMappingGeoPointTests { .bytes()); assertThat(doc.rootDoc().getField("point.lat"), notNullValue()); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getField("point.lat").binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.2))); + assertThat(doc.rootDoc().getField("point.lat").numericValue().doubleValue(), equalTo(1.2)); assertThat(doc.rootDoc().getField("point.lon"), notNullValue()); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getField("point.lon").binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.3))); + assertThat(doc.rootDoc().getField("point.lon").numericValue().doubleValue(), equalTo(1.3)); assertThat(doc.rootDoc().get("point"), equalTo("1.2,1.3")); } @@ -390,11 +388,11 @@ public class LatLonMappingGeoPointTests { assertThat(doc.rootDoc().getFields("point.lat").length, equalTo(2)); assertThat(doc.rootDoc().getFields("point.lon").length, equalTo(2)); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lat")[0].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.2))); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lon")[0].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.3))); + assertThat(doc.rootDoc().getFields("point.lat")[0].numericValue().doubleValue(), equalTo(1.2)); + assertThat(doc.rootDoc().getFields("point.lon")[0].numericValue().doubleValue(), equalTo(1.3)); assertThat(doc.rootDoc().getFields("point")[0].stringValue(), equalTo("1.2,1.3")); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lat")[1].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.4))); - assertThat(BytesRef.deepCopyOf(doc.rootDoc().getFields("point.lon")[1].binaryValue()).bytes, equalTo(Numbers.doubleToBytes(1.5))); + assertThat(doc.rootDoc().getFields("point.lat")[1].numericValue().doubleValue(), equalTo(1.4)); + assertThat(doc.rootDoc().getFields("point.lon")[1].numericValue().doubleValue(), equalTo(1.5)); assertThat(doc.rootDoc().getFields("point")[1].stringValue(), equalTo("1.4,1.5")); } } diff --git a/src/test/java/org/elasticsearch/test/unit/index/mapper/lucene/StoredNumericValuesTest.java b/src/test/java/org/elasticsearch/test/unit/index/mapper/lucene/StoredNumericValuesTest.java new file mode 100644 index 00000000000..861f10b08ea --- /dev/null +++ b/src/test/java/org/elasticsearch/test/unit/index/mapper/lucene/StoredNumericValuesTest.java @@ -0,0 +1,103 @@ +package org.elasticsearch.test.unit.index.mapper.lucene; + +import org.apache.lucene.document.*; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.store.RAMDirectory; +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.common.Numbers; +import org.elasticsearch.common.lucene.Lucene; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.index.fieldvisitor.CustomFieldsVisitor; +import org.elasticsearch.index.mapper.DocumentMapper; +import org.elasticsearch.index.mapper.ParsedDocument; +import org.elasticsearch.test.unit.index.mapper.MapperTests; +import org.testng.annotations.Test; + +import java.util.*; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +/** + * + */ +@Test +public class StoredNumericValuesTest { + + @Test + public void testBytesAndNumericRepresentation() throws Exception { + IndexWriter writer = new IndexWriter(new RAMDirectory(), new IndexWriterConfig(Lucene.VERSION, Lucene.STANDARD_ANALYZER)); + + String mapping = XContentFactory.jsonBuilder() + .startObject() + .startObject("type") + .startObject("properties") + .startObject("field1").field("type", "integer").field("store", "yes").endObject() + .startObject("field2").field("type", "float").field("store", "yes").endObject() + .startObject("field3").field("type", "long").field("store", "yes").endObject() + .endObject() + .endObject() + .endObject() + .string(); + DocumentMapper mapper = MapperTests.newParser().parse(mapping); + + ParsedDocument doc = mapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .field("field1", 1) + .field("field2", 1.1) + .startArray("field3").value(1).value(2).value(3).endArray() + .endObject() + .bytes()); + + writer.addDocument(doc.rootDoc(), doc.analyzer()); + + // Indexing a doc in the old way + FieldType fieldType = new FieldType(); + fieldType.setStored(true); + fieldType.setNumericType(FieldType.NumericType.INT); + Document doc2 = new Document(); + doc2.add(new StoredField("field1", new BytesRef(Numbers.intToBytes(1)))); + doc2.add(new StoredField("field2", new BytesRef(Numbers.floatToBytes(1.1f)))); + doc2.add(new StoredField("field3", new BytesRef(Numbers.longToBytes(1l)))); + doc2.add(new StoredField("field3", new BytesRef(Numbers.longToBytes(2l)))); + doc2.add(new StoredField("field3", new BytesRef(Numbers.longToBytes(3l)))); + writer.addDocument(doc2); + + DirectoryReader reader = DirectoryReader.open(writer, true); + IndexSearcher searcher = new IndexSearcher(reader); + + Set fields = new HashSet(Arrays.asList("field1", "field2", "field3")); + CustomFieldsVisitor fieldsVisitor = new CustomFieldsVisitor(fields, false); + searcher.doc(0, fieldsVisitor); + fieldsVisitor.postProcess(mapper); + assertThat(fieldsVisitor.fields().size(), equalTo(3)); + assertThat(fieldsVisitor.fields().get("field1").size(), equalTo(1)); + assertThat((Integer) fieldsVisitor.fields().get("field1").get(0), equalTo(1)); + assertThat(fieldsVisitor.fields().get("field2").size(), equalTo(1)); + assertThat((Float) fieldsVisitor.fields().get("field2").get(0), equalTo(1.1f)); + assertThat(fieldsVisitor.fields().get("field3").size(), equalTo(3)); + assertThat((Long) fieldsVisitor.fields().get("field3").get(0), equalTo(1l)); + assertThat((Long) fieldsVisitor.fields().get("field3").get(1), equalTo(2l)); + assertThat((Long) fieldsVisitor.fields().get("field3").get(2), equalTo(3l)); + + // Make sure the doc gets loaded as if it was stored in the new way + fieldsVisitor.reset(); + searcher.doc(1, fieldsVisitor); + fieldsVisitor.postProcess(mapper); + assertThat(fieldsVisitor.fields().size(), equalTo(3)); + assertThat(fieldsVisitor.fields().get("field1").size(), equalTo(1)); + assertThat((Integer) fieldsVisitor.fields().get("field1").get(0), equalTo(1)); + assertThat(fieldsVisitor.fields().get("field2").size(), equalTo(1)); + assertThat((Float) fieldsVisitor.fields().get("field2").get(0), equalTo(1.1f)); + assertThat(fieldsVisitor.fields().get("field3").size(), equalTo(3)); + assertThat((Long) fieldsVisitor.fields().get("field3").get(0), equalTo(1l)); + assertThat((Long) fieldsVisitor.fields().get("field3").get(1), equalTo(2l)); + assertThat((Long) fieldsVisitor.fields().get("field3").get(2), equalTo(3l)); + + reader.close(); + writer.close(); + } +}