From 76511158b56ddf0217dd207df554fe2e0d71661a Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Wed, 23 Jul 2014 17:38:13 +0200 Subject: [PATCH] Fielddata: Fix the ordinals impl for sparse fields. Caused by #6908 --- .../index/fielddata/ordinals/MultiOrdinals.java | 7 +++++-- .../index/fielddata/ordinals/OrdinalsBuilder.java | 8 +++++++- .../fielddata/FSTPackedBytesStringFieldDataTests.java | 3 ++- .../index/fielddata/PagedBytesStringFieldDataTests.java | 3 ++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinals.java b/src/main/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinals.java index 2e4ac8bc73c..0d5ae1260d4 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinals.java +++ b/src/main/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinals.java @@ -75,6 +75,8 @@ public class MultiOrdinals extends Ordinals { } lastEndOffset = endOffset; } + endOffsets.freeze(); + ords.freeze(); assert endOffsets.size() == builder.maxDoc(); assert ords.size() == builder.getTotalNumOrds() : ords.size() + " != " + builder.getTotalNumOrds(); } @@ -109,8 +111,9 @@ public class MultiOrdinals extends Ordinals { @Override public int getOrd(int docId) { - final long offset = docId != 0 ? endOffsets.get(docId - 1) : 0; - return (int) ords.get(offset); + final long startOffset = docId != 0 ? endOffsets.get(docId - 1) : 0; + final long endOffset = endOffsets.get(docId); + return startOffset == endOffset ? -1 : (int) ords.get(startOffset); } @Override diff --git a/src/main/java/org/elasticsearch/index/fielddata/ordinals/OrdinalsBuilder.java b/src/main/java/org/elasticsearch/index/fielddata/ordinals/OrdinalsBuilder.java index 0c695e043df..f01deea3602 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/ordinals/OrdinalsBuilder.java +++ b/src/main/java/org/elasticsearch/index/fielddata/ordinals/OrdinalsBuilder.java @@ -40,6 +40,11 @@ import java.util.Comparator; */ public final class OrdinalsBuilder implements Closeable { + /** + * Whether to for the use of {@link MultiOrdinals} to store the ordinals for testing purposes. + */ + public static final String FORCE_MULTI_ORDINALS = "force_multi_ordinals"; + /** * Default acceptable overhead ratio. {@link OrdinalsBuilder} memory usage is mostly transient so it is likely a better trade-off to * trade memory for speed in order to resize less often. @@ -395,7 +400,8 @@ public final class OrdinalsBuilder implements Closeable { */ public Ordinals build(Settings settings) { final float acceptableOverheadRatio = settings.getAsFloat("acceptable_overhead_ratio", PackedInts.FASTEST); - if (numMultiValuedDocs > 0 || MultiOrdinals.significantlySmallerThanSinglePackedOrdinals(maxDoc, numDocsWithValue, getValueCount(), acceptableOverheadRatio)) { + final boolean forceMultiOrdinals = settings.getAsBoolean(FORCE_MULTI_ORDINALS, false); + if (forceMultiOrdinals || numMultiValuedDocs > 0 || MultiOrdinals.significantlySmallerThanSinglePackedOrdinals(maxDoc, numDocsWithValue, getValueCount(), acceptableOverheadRatio)) { // MultiOrdinals can be smaller than SinglePackedOrdinals for sparse fields return new MultiOrdinals(this, acceptableOverheadRatio); } else { diff --git a/src/test/java/org/elasticsearch/index/fielddata/FSTPackedBytesStringFieldDataTests.java b/src/test/java/org/elasticsearch/index/fielddata/FSTPackedBytesStringFieldDataTests.java index e9047c2814b..c881a5e4aa9 100644 --- a/src/test/java/org/elasticsearch/index/fielddata/FSTPackedBytesStringFieldDataTests.java +++ b/src/test/java/org/elasticsearch/index/fielddata/FSTPackedBytesStringFieldDataTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.fielddata; import org.elasticsearch.common.settings.ImmutableSettings; +import org.elasticsearch.index.fielddata.ordinals.OrdinalsBuilder; /** */ @@ -27,6 +28,6 @@ public class FSTPackedBytesStringFieldDataTests extends AbstractStringFieldDataT @Override protected FieldDataType getFieldDataType() { - return new FieldDataType("string", ImmutableSettings.builder().put("format", "fst")); + return new FieldDataType("string", ImmutableSettings.builder().put("format", "fst").put(OrdinalsBuilder.FORCE_MULTI_ORDINALS, randomBoolean())); } } diff --git a/src/test/java/org/elasticsearch/index/fielddata/PagedBytesStringFieldDataTests.java b/src/test/java/org/elasticsearch/index/fielddata/PagedBytesStringFieldDataTests.java index 7496ce1ea0c..56f0b5d4d84 100644 --- a/src/test/java/org/elasticsearch/index/fielddata/PagedBytesStringFieldDataTests.java +++ b/src/test/java/org/elasticsearch/index/fielddata/PagedBytesStringFieldDataTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.fielddata; import org.elasticsearch.common.settings.ImmutableSettings; +import org.elasticsearch.index.fielddata.ordinals.OrdinalsBuilder; /** */ @@ -27,6 +28,6 @@ public class PagedBytesStringFieldDataTests extends AbstractStringFieldDataTests @Override protected FieldDataType getFieldDataType() { - return new FieldDataType("string", ImmutableSettings.builder().put("format", "paged_bytes")); + return new FieldDataType("string", ImmutableSettings.builder().put("format", "paged_bytes").put(OrdinalsBuilder.FORCE_MULTI_ORDINALS, randomBoolean())); } }