From fc32875ae9470eaa0d92aa94c4b49d47eea11165 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Thu, 24 Apr 2014 22:20:38 +0200 Subject: [PATCH] Make ordinals start at 0. Our ordinals currently start at 1, like FieldCache did in older Lucene versions. However, Lucene 4.2 changed it in order to make ordinals start at 0, using -1 as the ordinal for the missing value. We should switch to the same numbering as Lucene for consistency. This also allows to remove some abstraction on top of Lucene doc values. Close #5871 --- .../BytesRefOrdValComparator.java | 15 +++---- .../fielddata/ordinals/EmptyOrdinals.java | 4 +- .../InternalGlobalOrdinalsBuilder.java | 13 +++--- .../fielddata/ordinals/MultiOrdinals.java | 16 +++---- .../index/fielddata/ordinals/Ordinals.java | 6 ++- .../fielddata/ordinals/OrdinalsBuilder.java | 22 ++++----- .../ordinals/SinglePackedOrdinals.java | 14 +++--- .../plain/DoubleArrayIndexFieldData.java | 8 +++- .../plain/FSTBytesAtomicFieldData.java | 5 +-- .../plain/FSTBytesIndexFieldData.java | 1 - .../plain/FloatArrayIndexFieldData.java | 9 ++-- .../GeoPointCompressedAtomicFieldData.java | 1 - .../GeoPointCompressedIndexFieldData.java | 13 ++++-- .../GeoPointDoubleArrayAtomicFieldData.java | 1 - .../GeoPointDoubleArrayIndexFieldData.java | 11 +++-- .../plain/PackedArrayAtomicFieldData.java | 4 +- .../plain/PackedArrayIndexFieldData.java | 8 ++-- .../plain/PagedBytesAtomicFieldData.java | 1 - .../plain/PagedBytesIndexFieldData.java | 1 - .../plain/ParentChildIndexFieldData.java | 3 -- .../plain/SortedSetDVAtomicFieldData.java | 41 +++++------------ .../TermsStringOrdinalsFacetExecutor.java | 14 +++--- .../AbstractStringFieldDataTests.java | 32 ++++++------- .../index/fielddata/FilterFieldDataTest.java | 36 +++++++-------- .../ordinals/MultiOrdinalsTests.java | 45 +++++++++---------- 25 files changed, 152 insertions(+), 172 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefOrdValComparator.java b/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefOrdValComparator.java index bd1c172b933..3d5ea440904 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefOrdValComparator.java +++ b/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefOrdValComparator.java @@ -242,7 +242,7 @@ public final class BytesRefOrdValComparator extends NestedWrappableComparator 0; + assert ord >= 0; ords[slot] = ord << 2; if (values[slot] == null || values[slot] == missingValue) { values[slot] = new BytesRef(); @@ -261,11 +261,9 @@ public final class BytesRefOrdValComparator extends NestedWrappableComparator= 0 : ord; - assert (ord == 0) == (value == null) : "ord=" + ord + ", value=" + value; - final long previousOrd = ord >>> 2; + final long previousOrd = ord >> 2; final long nextOrd = previousOrd + 1; - final BytesRef previous = previousOrd == 0 ? null : termsIndex.getValueByOrd(previousOrd); + final BytesRef previous = previousOrd == Ordinals.MISSING_ORDINAL ? null : termsIndex.getValueByOrd(previousOrd); if ((ord & 3) == 0) { // there was an existing ord with the inserted value assert compareValues(previous, value) == 0; } else { @@ -281,7 +279,6 @@ public final class BytesRefOrdValComparator extends NestedWrappableComparator= 0) { // value exists in the current segment @@ -398,13 +395,13 @@ public final class BytesRefOrdValComparator extends NestedWrappableComparator= 0; - assert result <= readerOrds.getMaxOrd(); + assert result >= Ordinals.MISSING_ORDINAL; + assert result < readerOrds.getMaxOrd(); return result; // Enable this when the api can tell us that the ords per doc are ordered /*if (reversed) { diff --git a/src/main/java/org/elasticsearch/index/fielddata/ordinals/EmptyOrdinals.java b/src/main/java/org/elasticsearch/index/fielddata/ordinals/EmptyOrdinals.java index c02e6c787e3..7bd2feaf860 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/ordinals/EmptyOrdinals.java +++ b/src/main/java/org/elasticsearch/index/fielddata/ordinals/EmptyOrdinals.java @@ -56,7 +56,7 @@ public enum EmptyOrdinals implements Ordinals { @Override public long getOrd(int docId) { - return 0; + return Ordinals.MISSING_ORDINAL; } @Override @@ -71,7 +71,7 @@ public enum EmptyOrdinals implements Ordinals { @Override public long currentOrd() { - return 0; + return Ordinals.MISSING_ORDINAL; } } } diff --git a/src/main/java/org/elasticsearch/index/fielddata/ordinals/InternalGlobalOrdinalsBuilder.java b/src/main/java/org/elasticsearch/index/fielddata/ordinals/InternalGlobalOrdinalsBuilder.java index fee9846934e..c5fbb902395 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/ordinals/InternalGlobalOrdinalsBuilder.java +++ b/src/main/java/org/elasticsearch/index/fielddata/ordinals/InternalGlobalOrdinalsBuilder.java @@ -64,9 +64,7 @@ public class InternalGlobalOrdinalsBuilder extends AbstractIndexComponent implem // it makes sense to force COMPACT for them final float acceptableOverheadRatio = settings.getAsFloat("acceptable_overhead_ratio", PackedInts.FAST); final AppendingPackedLongBuffer globalOrdToFirstSegment = new AppendingPackedLongBuffer(PackedInts.COMPACT); - globalOrdToFirstSegment.add(0); final MonotonicAppendingLongBuffer globalOrdToFirstSegmentDelta = new MonotonicAppendingLongBuffer(PackedInts.COMPACT); - globalOrdToFirstSegmentDelta.add(0); FieldDataType fieldDataType = indexFieldData.getFieldDataType(); int defaultThreshold = settings.getAsInt(ORDINAL_MAPPING_THRESHOLD_INDEX_SETTING_KEY, ORDINAL_MAPPING_THRESHOLD_DEFAULT); @@ -79,13 +77,13 @@ public class InternalGlobalOrdinalsBuilder extends AbstractIndexComponent implem final AtomicFieldData.WithOrdinals[] withOrdinals = new AtomicFieldData.WithOrdinals[indexReader.leaves().size()]; TermIterator termIterator = new TermIterator(indexFieldData, indexReader.leaves(), withOrdinals); for (BytesRef term = termIterator.next(); term != null; term = termIterator.next()) { - currentGlobalOrdinal++; globalOrdToFirstSegment.add(termIterator.firstReaderIndex()); long globalOrdinalDelta = currentGlobalOrdinal - termIterator.firstLocalOrdinal(); globalOrdToFirstSegmentDelta.add(globalOrdinalDelta); for (TermIterator.LeafSource leafSource : termIterator.competitiveLeafs()) { ordinalMappingBuilder.onOrdinal(leafSource.context.ord, leafSource.tenum.ord(), currentGlobalOrdinal); } + currentGlobalOrdinal++; } // ram used for the globalOrd to segmentOrd and segmentOrd to firstReaderIndex lookups @@ -95,7 +93,7 @@ public class InternalGlobalOrdinalsBuilder extends AbstractIndexComponent implem globalOrdToFirstSegmentDelta.freeze(); memorySizeInBytesCounter += globalOrdToFirstSegmentDelta.ramBytesUsed(); - final long maxOrd = currentGlobalOrdinal + 1; + final long maxOrd = currentGlobalOrdinal; OrdinalMappingSource[] segmentOrdToGlobalOrdLookups = ordinalMappingBuilder.build(maxOrd); // add ram used for the main segmentOrd to globalOrd lookups memorySizeInBytesCounter += ordinalMappingBuilder.getMemorySizeInBytes(); @@ -163,7 +161,11 @@ public class InternalGlobalOrdinalsBuilder extends AbstractIndexComponent implem @Override public final long getOrd(int docId) { long segmentOrd = segmentOrdinals.getOrd(docId); - return currentGlobalOrd = getGlobalOrd(segmentOrd); + if (segmentOrd == Ordinals.MISSING_ORDINAL) { + return currentGlobalOrd = Ordinals.MISSING_ORDINAL; + } else { + return currentGlobalOrd = getGlobalOrd(segmentOrd); + } } @Override @@ -189,7 +191,6 @@ public class InternalGlobalOrdinalsBuilder extends AbstractIndexComponent implem segmentOrdToGlobalOrdDeltas = new MonotonicAppendingLongBuffer[numSegments]; for (int i = 0; i < segmentOrdToGlobalOrdDeltas.length; i++) { segmentOrdToGlobalOrdDeltas[i] = new MonotonicAppendingLongBuffer(acceptableOverheadRatio); - segmentOrdToGlobalOrdDeltas[i].add(0); } this.numSegments = numSegments; this.acceptableOverheadRatio = acceptableOverheadRatio; 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 c617fb2fa43..25093eda2e2 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinals.java +++ b/src/main/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinals.java @@ -51,13 +51,13 @@ public class MultiOrdinals implements Ordinals { } private final boolean multiValued; - private final long numOrds; + private final long maxOrd; private final MonotonicAppendingLongBuffer endOffsets; private final AppendingPackedLongBuffer ords; public MultiOrdinals(OrdinalsBuilder builder, float acceptableOverheadRatio) { multiValued = builder.getNumMultiValuesDocs() > 0; - numOrds = builder.getNumOrds(); + maxOrd = builder.getMaxOrd(); endOffsets = new MonotonicAppendingLongBuffer(OFFSET_INIT_PAGE_COUNT, OFFSETS_PAGE_SIZE, acceptableOverheadRatio); ords = new AppendingPackedLongBuffer(OFFSET_INIT_PAGE_COUNT, OFFSETS_PAGE_SIZE, acceptableOverheadRatio); long lastEndOffset = 0; @@ -66,7 +66,7 @@ public class MultiOrdinals implements Ordinals { final long endOffset = lastEndOffset + docOrds.length; endOffsets.add(endOffset); for (int j = 0; j < docOrds.length; ++j) { - ords.add(docOrds.longs[docOrds.offset + j] - 1); + ords.add(docOrds.longs[docOrds.offset + j]); } lastEndOffset = endOffset; } @@ -86,7 +86,7 @@ public class MultiOrdinals implements Ordinals { @Override public long getMaxOrd() { - return numOrds + 1; + return maxOrd; } @Override @@ -98,7 +98,6 @@ public class MultiOrdinals implements Ordinals { private final MonotonicAppendingLongBuffer endOffsets; private final AppendingPackedLongBuffer ords; - private final LongsRef longsScratch; private long offset; private long limit; private long currentOrd; @@ -107,7 +106,6 @@ public class MultiOrdinals implements Ordinals { super(ordinals); this.endOffsets = ordinals.endOffsets; this.ords = ordinals.ords; - this.longsScratch = new LongsRef(16); } @Override @@ -115,16 +113,16 @@ public class MultiOrdinals implements Ordinals { final long startOffset = docId > 0 ? endOffsets.get(docId - 1) : 0; final long endOffset = endOffsets.get(docId); if (startOffset == endOffset) { - return currentOrd = 0L; // ord for missing values + return currentOrd = Ordinals.MISSING_ORDINAL; // ord for missing values } else { - return currentOrd = 1L + ords.get(startOffset); + return currentOrd = ords.get(startOffset); } } @Override public long nextOrd() { assert offset < limit; - return currentOrd = 1L + ords.get(offset++); + return currentOrd = ords.get(offset++); } @Override diff --git a/src/main/java/org/elasticsearch/index/fielddata/ordinals/Ordinals.java b/src/main/java/org/elasticsearch/index/fielddata/ordinals/Ordinals.java index d000a8562e3..962ae90c1c3 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/ordinals/Ordinals.java +++ b/src/main/java/org/elasticsearch/index/fielddata/ordinals/Ordinals.java @@ -19,14 +19,16 @@ package org.elasticsearch.index.fielddata.ordinals; +import org.apache.lucene.index.SortedSetDocValues; + /** * A thread safe ordinals abstraction. Ordinals can only be positive integers. */ public interface Ordinals { - static final long MISSING_ORDINAL = 0; - static final long MIN_ORDINAL = 1; + static final long MISSING_ORDINAL = SortedSetDocValues.NO_MORE_ORDS; + static final long MIN_ORDINAL = 0; /** * The memory size this ordinals take. 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 b89d6509717..e308fc84218 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/ordinals/OrdinalsBuilder.java +++ b/src/main/java/org/elasticsearch/index/fielddata/ordinals/OrdinalsBuilder.java @@ -138,7 +138,7 @@ public final class OrdinalsBuilder implements Closeable { // First level (0) of ordinals and pointers to the next level private final GrowableWriter firstOrdinals; private PagedGrowableWriter firstNextLevelSlices; - // Ordinals and pointers for other levels, starting at 1 + // Ordinals and pointers for other levels +1 private final PagedGrowableWriter[] ordinals; private final PagedGrowableWriter[] nextLevelSlices; private final int[] sizes; @@ -181,7 +181,7 @@ public final class OrdinalsBuilder implements Closeable { if (position == 0L) { // on the first level // 0 or 1 ordinal if (firstOrdinals.get(docID) == 0L) { - firstOrdinals.set(docID, ordinal); + firstOrdinals.set(docID, ordinal + 1); return 1; } else { final long newSlice = newSlice(1); @@ -190,7 +190,7 @@ public final class OrdinalsBuilder implements Closeable { } firstNextLevelSlices.set(docID, newSlice); final long offset = startOffset(1, newSlice); - ordinals[1].set(offset, ordinal); + ordinals[1].set(offset, ordinal + 1); positions.set(docID, position(1, offset)); // current position is on the 1st level and not allocated yet return 2; } @@ -212,7 +212,7 @@ public final class OrdinalsBuilder implements Closeable { // just go to the next slot ++offset; } - ordinals[level].set(offset, ordinal); + ordinals[level].set(offset, ordinal + 1); final long newPosition = position(level, offset); positions.set(docID, newPosition); return numOrdinals(level, offset); @@ -226,7 +226,7 @@ public final class OrdinalsBuilder implements Closeable { return; } ords.longs = ArrayUtil.grow(ords.longs, ords.offset + ords.length + 1); - ords.longs[ords.offset + ords.length++] = firstOrd; + ords.longs[ords.offset + ords.length++] = firstOrd - 1; if (firstNextLevelSlices == null) { return; } @@ -244,7 +244,7 @@ public final class OrdinalsBuilder implements Closeable { if (ord == 0L) { return; } - ords.longs[ords.offset + ords.length++] = ord; + ords.longs[ords.offset + ords.length++] = ord - 1; } if (nextLevelSlices[level] == null) { return; @@ -259,7 +259,7 @@ public final class OrdinalsBuilder implements Closeable { } private final int maxDoc; - private long currentOrd = 0; + private long currentOrd = Ordinals.MIN_ORDINAL - 1; private int numDocsWithValue = 0; private int numMultiValuedDocs = 0; private int totalNumOrds = 0; @@ -295,7 +295,7 @@ public final class OrdinalsBuilder implements Closeable { } /** - * Return a {@link PackedInts.Reader} instance mapping every doc ID to its first ordinal if it exists and 0 otherwise. + * Return a {@link PackedInts.Reader} instance mapping every doc ID to its first ordinal + 1 if it exists and 0 otherwise. */ public PackedInts.Reader getFirstOrdinals() { return ordinals.firstOrdinals; @@ -369,8 +369,8 @@ public final class OrdinalsBuilder implements Closeable { /** * Returns the number of distinct ordinals in this builder. */ - public long getNumOrds() { - return currentOrd; + public long getMaxOrd() { + return currentOrd + 1; } /** @@ -395,7 +395,7 @@ 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, getNumOrds(), acceptableOverheadRatio)) { + if (numMultiValuedDocs > 0 || MultiOrdinals.significantlySmallerThanSinglePackedOrdinals(maxDoc, numDocsWithValue, getMaxOrd(), acceptableOverheadRatio)) { // MultiOrdinals can be smaller than SinglePackedOrdinals for sparse fields return new MultiOrdinals(this, acceptableOverheadRatio); } else { diff --git a/src/main/java/org/elasticsearch/index/fielddata/ordinals/SinglePackedOrdinals.java b/src/main/java/org/elasticsearch/index/fielddata/ordinals/SinglePackedOrdinals.java index f67536eff27..7cf4b9be934 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/ordinals/SinglePackedOrdinals.java +++ b/src/main/java/org/elasticsearch/index/fielddata/ordinals/SinglePackedOrdinals.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.fielddata.ordinals; -import org.apache.lucene.util.LongsRef; import org.apache.lucene.util.RamUsageEstimator; import org.apache.lucene.util.packed.PackedInts; @@ -35,9 +34,9 @@ public class SinglePackedOrdinals implements Ordinals { public SinglePackedOrdinals(OrdinalsBuilder builder, float acceptableOverheadRatio) { assert builder.getNumMultiValuesDocs() == 0; - this.maxOrd = builder.getNumOrds() + 1; + this.maxOrd = builder.getMaxOrd(); // We don't reuse the builder as-is because it might have been built with a higher overhead ratio - final PackedInts.Mutable reader = PackedInts.getMutable(builder.maxDoc(), PackedInts.bitsRequired(getMaxOrd() - 1), acceptableOverheadRatio); + final PackedInts.Mutable reader = PackedInts.getMutable(builder.maxDoc(), PackedInts.bitsRequired(getMaxOrd()), acceptableOverheadRatio); PackedInts.copy(builder.getFirstOrdinals(), 0, reader, 0, builder.maxDoc(), 8 * 1024); this.reader = reader; } @@ -69,7 +68,6 @@ public class SinglePackedOrdinals implements Ordinals { private final PackedInts.Reader reader; - private final LongsRef longsScratch = new LongsRef(1); private long currentOrdinal; public Docs(SinglePackedOrdinals parent, PackedInts.Reader reader) { @@ -79,20 +77,20 @@ public class SinglePackedOrdinals implements Ordinals { @Override public long getOrd(int docId) { - return currentOrdinal = reader.get(docId); + return currentOrdinal = reader.get(docId) - 1; } @Override public long nextOrd() { - assert currentOrdinal > 0; + assert currentOrdinal >= Ordinals.MIN_ORDINAL; return currentOrdinal; } @Override public int setDocument(int docId) { - currentOrdinal = reader.get(docId); + currentOrdinal = reader.get(docId) - 1; // either this is > 1 or 0 - in any case it prevents a branch! - return (int)Math.min(currentOrdinal, 1); + return 1 + (int) Math.min(currentOrdinal, 0); } @Override diff --git a/src/main/java/org/elasticsearch/index/fielddata/plain/DoubleArrayIndexFieldData.java b/src/main/java/org/elasticsearch/index/fielddata/plain/DoubleArrayIndexFieldData.java index 32b16e53728..7051ca7e357 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/plain/DoubleArrayIndexFieldData.java +++ b/src/main/java/org/elasticsearch/index/fielddata/plain/DoubleArrayIndexFieldData.java @@ -88,7 +88,6 @@ public class DoubleArrayIndexFieldData extends AbstractIndexFieldData fstEnum = new BytesRefFSTEnum<>(fst); IntArray hashes = BigArrays.NON_RECYCLING_INSTANCE.newIntArray(ordinals.getMaxOrd()); - // we don't store an ord 0 in the FST since we could have an empty string in there and FST don't support - // empty strings twice. ie. them merge fails for long output. - hashes.set(0, new BytesRef().hashCode()); try { - for (long i = 1, maxOrd = ordinals.getMaxOrd(); i < maxOrd; ++i) { + for (long i = 0, maxOrd = ordinals.getMaxOrd(); i < maxOrd; ++i) { hashes.set(i, fstEnum.next().input.hashCode()); } assert fstEnum.next() == null; diff --git a/src/main/java/org/elasticsearch/index/fielddata/plain/FSTBytesIndexFieldData.java b/src/main/java/org/elasticsearch/index/fielddata/plain/FSTBytesIndexFieldData.java index c60faaa3c07..e1275c5375f 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/plain/FSTBytesIndexFieldData.java +++ b/src/main/java/org/elasticsearch/index/fielddata/plain/FSTBytesIndexFieldData.java @@ -93,7 +93,6 @@ public class FSTBytesIndexFieldData extends AbstractBytesIndexFieldData 0; fstBuilder.add(Util.toIntsRef(term, scratch), (long) termOrd); docsEnum = termsEnum.docs(null, docsEnum, DocsEnum.FLAG_NONE); for (int docId = docsEnum.nextDoc(); docId != DocsEnum.NO_MORE_DOCS; docId = docsEnum.nextDoc()) { diff --git a/src/main/java/org/elasticsearch/index/fielddata/plain/FloatArrayIndexFieldData.java b/src/main/java/org/elasticsearch/index/fielddata/plain/FloatArrayIndexFieldData.java index 519aa356285..ea996dde166 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/plain/FloatArrayIndexFieldData.java +++ b/src/main/java/org/elasticsearch/index/fielddata/plain/FloatArrayIndexFieldData.java @@ -86,8 +86,6 @@ public class FloatArrayIndexFieldData extends AbstractIndexFieldData 0; return encoding.decode(lat.get(ord), lon.get(ord), scratch); } diff --git a/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointCompressedIndexFieldData.java b/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointCompressedIndexFieldData.java index 18d4ef71977..9097172e304 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointCompressedIndexFieldData.java +++ b/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointCompressedIndexFieldData.java @@ -103,9 +103,8 @@ public class GeoPointCompressedIndexFieldData extends AbstractGeoPointIndexField try (OrdinalsBuilder builder = new OrdinalsBuilder(terms.size(), reader.maxDoc(), acceptableTransientOverheadRatio)) { final GeoPointEnum iter = new GeoPointEnum(builder.buildFromTerms(terms.iterator(null))); GeoPoint point; - long ord = 0; while ((point = iter.next()) != null) { - ++ord; + final long ord = builder.currentOrdinal(); if (lat.size() <= ord) { final long newSize = BigArrays.overSize(ord + 1); lat = lat.resize(newSize); @@ -127,10 +126,16 @@ public class GeoPointCompressedIndexFieldData extends AbstractGeoPointIndexField int maxDoc = reader.maxDoc(); PagedMutable sLat = new PagedMutable(reader.maxDoc(), pageSize, encoding.numBitsPerCoordinate(), PackedInts.COMPACT); PagedMutable sLon = new PagedMutable(reader.maxDoc(), pageSize, encoding.numBitsPerCoordinate(), PackedInts.COMPACT); + final long missing = encoding.encodeCoordinate(0); for (int i = 0; i < maxDoc; i++) { final long nativeOrdinal = ordinals.getOrd(i); - sLat.set(i, lat.get(nativeOrdinal)); - sLon.set(i, lon.get(nativeOrdinal)); + if (nativeOrdinal != Ordinals.MISSING_ORDINAL) { + sLat.set(i, lat.get(nativeOrdinal)); + sLon.set(i, lon.get(nativeOrdinal)); + } else { + sLat.set(i, missing); + sLon.set(i, missing); + } } FixedBitSet set = builder.buildDocsWithValuesSet(); if (set == null) { diff --git a/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointDoubleArrayAtomicFieldData.java b/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointDoubleArrayAtomicFieldData.java index c73383e1184..1b1e34dedb7 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointDoubleArrayAtomicFieldData.java +++ b/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointDoubleArrayAtomicFieldData.java @@ -94,7 +94,6 @@ public abstract class GeoPointDoubleArrayAtomicFieldData extends AtomicGeoPointF @Override public GeoPoint nextValue() { final long ord = ordinals.nextOrd(); - assert ord > 0; return scratch.reset(lat.get(ord), lon.get(ord)); } diff --git a/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointDoubleArrayIndexFieldData.java b/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointDoubleArrayIndexFieldData.java index 9083724cb6a..76071aac990 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointDoubleArrayIndexFieldData.java +++ b/src/main/java/org/elasticsearch/index/fielddata/plain/GeoPointDoubleArrayIndexFieldData.java @@ -72,8 +72,6 @@ public class GeoPointDoubleArrayIndexFieldData extends AbstractGeoPointIndexFiel } final BigDoubleArrayList lat = new BigDoubleArrayList(); final BigDoubleArrayList lon = new BigDoubleArrayList(); - lat.add(0); // first "t" indicates null value - lon.add(0); // first "t" indicates null value final float acceptableTransientOverheadRatio = fieldDataType.getSettings().getAsFloat("acceptable_transient_overhead_ratio", OrdinalsBuilder.DEFAULT_ACCEPTABLE_OVERHEAD_RATIO); boolean success = false; try (OrdinalsBuilder builder = new OrdinalsBuilder(terms.size(), reader.maxDoc(), acceptableTransientOverheadRatio)) { @@ -92,8 +90,13 @@ public class GeoPointDoubleArrayIndexFieldData extends AbstractGeoPointIndexFiel BigDoubleArrayList sLon = new BigDoubleArrayList(reader.maxDoc()); for (int i = 0; i < maxDoc; i++) { long nativeOrdinal = ordinals.getOrd(i); - sLat.add(lat.get(nativeOrdinal)); - sLon.add(lon.get(nativeOrdinal)); + if (nativeOrdinal == Ordinals.MISSING_ORDINAL) { + sLat.add(0); + sLon.add(0); + } else { + sLat.add(lat.get(nativeOrdinal)); + sLon.add(lon.get(nativeOrdinal)); + } } FixedBitSet set = builder.buildDocsWithValuesSet(); if (set == null) { diff --git a/src/main/java/org/elasticsearch/index/fielddata/plain/PackedArrayAtomicFieldData.java b/src/main/java/org/elasticsearch/index/fielddata/plain/PackedArrayAtomicFieldData.java index 7d2f4fff20e..a35969a2609 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/plain/PackedArrayAtomicFieldData.java +++ b/src/main/java/org/elasticsearch/index/fielddata/plain/PackedArrayAtomicFieldData.java @@ -135,7 +135,7 @@ public abstract class PackedArrayAtomicFieldData extends AbstractAtomicNumericFi @Override public long getValueByOrd(long ord) { assert ord != Ordinals.MISSING_ORDINAL; - return values.get(ord - 1); + return values.get(ord); } } @@ -151,7 +151,7 @@ public abstract class PackedArrayAtomicFieldData extends AbstractAtomicNumericFi @Override public double getValueByOrd(long ord) { assert ord != Ordinals.MISSING_ORDINAL; - return values.get(ord - 1); + return values.get(ord); } diff --git a/src/main/java/org/elasticsearch/index/fielddata/plain/PackedArrayIndexFieldData.java b/src/main/java/org/elasticsearch/index/fielddata/plain/PackedArrayIndexFieldData.java index 06eaab01cf8..6e92a2684dc 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/plain/PackedArrayIndexFieldData.java +++ b/src/main/java/org/elasticsearch/index/fielddata/plain/PackedArrayIndexFieldData.java @@ -186,7 +186,7 @@ public class PackedArrayIndexFieldData extends AbstractIndexFieldData Ordinals.MIN_ORDINAL) { aggregators.add(current); } else { @@ -232,8 +232,8 @@ public class TermsStringOrdinalsFacetExecutor extends FacetExecutor { @Override public void postCollection() { if (current != null) { - missing += current.counts.get(0); - total += current.total - current.counts.get(0); + missing += current.missing; + total += current.total; // if we have values for this one, add it if (current.values.ordinals().getMaxOrd() > Ordinals.MIN_ORDINAL) { aggregators.add(current); @@ -253,7 +253,8 @@ public class TermsStringOrdinalsFacetExecutor extends FacetExecutor { final BytesValues.WithOrdinals values; final IntArray counts; - long position = 0; + int missing = 0; + long position = Ordinals.MIN_ORDINAL - 1; BytesRef current; int total; @@ -270,8 +271,7 @@ public class TermsStringOrdinalsFacetExecutor extends FacetExecutor { } final void incrementMissing(int numMissing) { - counts.increment(0, numMissing); - total += numMissing; + missing += numMissing; } public boolean nextPosition() { diff --git a/src/test/java/org/elasticsearch/index/fielddata/AbstractStringFieldDataTests.java b/src/test/java/org/elasticsearch/index/fielddata/AbstractStringFieldDataTests.java index 31d458766e1..9da9ddc0c66 100644 --- a/src/test/java/org/elasticsearch/index/fielddata/AbstractStringFieldDataTests.java +++ b/src/test/java/org/elasticsearch/index/fielddata/AbstractStringFieldDataTests.java @@ -439,15 +439,15 @@ public abstract class AbstractStringFieldDataTests extends AbstractFieldDataImpl Ordinals.Docs ordinals = afd.getBytesValues(randomBoolean()).ordinals(); assertThat(ordinals.setDocument(0), equalTo(2)); long ord = ordinals.nextOrd(); - assertThat(ord, equalTo(4l)); + assertThat(ord, equalTo(3l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("02")); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(6l)); + assertThat(ord, equalTo(5l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("04")); assertThat(ordinals.setDocument(1), equalTo(0)); assertThat(ordinals.setDocument(2), equalTo(1)); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(5l)); + assertThat(ord, equalTo(4l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("03")); // Second segment @@ -456,34 +456,34 @@ public abstract class AbstractStringFieldDataTests extends AbstractFieldDataImpl ordinals = afd.getBytesValues(randomBoolean()).ordinals(); assertThat(ordinals.setDocument(0), equalTo(3)); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(6l)); + assertThat(ord, equalTo(5l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("04")); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(7l)); + assertThat(ord, equalTo(6l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("05")); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(8l)); + assertThat(ord, equalTo(7l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("06")); assertThat(ordinals.setDocument(1), equalTo(3)); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(8l)); + assertThat(ord, equalTo(7l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("06")); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(9l)); + assertThat(ord, equalTo(8l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("07")); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(10l)); + assertThat(ord, equalTo(9l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("08")); assertThat(ordinals.setDocument(2), equalTo(0)); assertThat(ordinals.setDocument(3), equalTo(3)); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(10l)); + assertThat(ord, equalTo(9l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("08")); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(11l)); + assertThat(ord, equalTo(10l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("09")); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(12l)); + assertThat(ord, equalTo(11l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("10")); // Third segment @@ -492,13 +492,13 @@ public abstract class AbstractStringFieldDataTests extends AbstractFieldDataImpl ordinals = afd.getBytesValues(randomBoolean()).ordinals(); assertThat(ordinals.setDocument(0), equalTo(3)); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(1l)); + assertThat(ord, equalTo(0l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("!08")); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(2l)); + assertThat(ord, equalTo(1l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("!09")); ord = ordinals.nextOrd(); - assertThat(ord, equalTo(3l)); + assertThat(ord, equalTo(2l)); assertThat(values.getValueByOrd(ord).utf8ToString(), equalTo("!10")); } @@ -529,7 +529,7 @@ public abstract class AbstractStringFieldDataTests extends AbstractFieldDataImpl } assertThat(size, equalTo(2)); - termsEnum.seekExact(9); + termsEnum.seekExact(8); assertThat(termsEnum.term().utf8ToString(), equalTo("07")); size = 0; while (termsEnum.next() != null) { diff --git a/src/test/java/org/elasticsearch/index/fielddata/FilterFieldDataTest.java b/src/test/java/org/elasticsearch/index/fielddata/FilterFieldDataTest.java index bf1c356ba91..7cfafd93425 100644 --- a/src/test/java/org/elasticsearch/index/fielddata/FilterFieldDataTest.java +++ b/src/test/java/org/elasticsearch/index/fielddata/FilterFieldDataTest.java @@ -74,9 +74,9 @@ public class FilterFieldDataTest extends AbstractFieldDataTests { AtomicFieldData.WithOrdinals loadDirect = (WithOrdinals) fieldData.loadDirect(context); BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean()); Docs ordinals = bytesValues.ordinals(); - assertThat(3L, equalTo(ordinals.getMaxOrd())); - assertThat(bytesValues.getValueByOrd(1).utf8ToString(), equalTo("10")); - assertThat(bytesValues.getValueByOrd(2).utf8ToString(), equalTo("100")); + assertThat(2L, equalTo(ordinals.getMaxOrd())); + assertThat(bytesValues.getValueByOrd(0).utf8ToString(), equalTo("10")); + assertThat(bytesValues.getValueByOrd(1).utf8ToString(), equalTo("100")); } { ifdService.clear(); @@ -86,8 +86,8 @@ public class FilterFieldDataTest extends AbstractFieldDataTests { AtomicFieldData.WithOrdinals loadDirect = (WithOrdinals) fieldData.loadDirect(context); BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean()); Docs ordinals = bytesValues.ordinals(); - assertThat(2L, equalTo(ordinals.getMaxOrd())); - assertThat(bytesValues.getValueByOrd(1).utf8ToString(), equalTo("5")); + assertThat(1L, equalTo(ordinals.getMaxOrd())); + assertThat(bytesValues.getValueByOrd(0).utf8ToString(), equalTo("5")); } { @@ -98,9 +98,9 @@ public class FilterFieldDataTest extends AbstractFieldDataTests { AtomicFieldData.WithOrdinals loadDirect = (WithOrdinals) fieldData.loadDirect(context); BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean()); Docs ordinals = bytesValues.ordinals(); - assertThat(3L, equalTo(ordinals.getMaxOrd())); - assertThat(bytesValues.getValueByOrd(1).utf8ToString(), equalTo("10")); - assertThat(bytesValues.getValueByOrd(2).utf8ToString(), equalTo("100")); + assertThat(2L, equalTo(ordinals.getMaxOrd())); + assertThat(bytesValues.getValueByOrd(0).utf8ToString(), equalTo("10")); + assertThat(bytesValues.getValueByOrd(1).utf8ToString(), equalTo("100")); } { @@ -111,9 +111,9 @@ public class FilterFieldDataTest extends AbstractFieldDataTests { AtomicFieldData.WithOrdinals loadDirect = (WithOrdinals) fieldData.loadDirect(context); BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean()); Docs ordinals = bytesValues.ordinals(); - assertThat(3L, equalTo(ordinals.getMaxOrd())); - assertThat(bytesValues.getValueByOrd(1).utf8ToString(), equalTo("10")); - assertThat(bytesValues.getValueByOrd(2).utf8ToString(), equalTo("100")); + assertThat(2L, equalTo(ordinals.getMaxOrd())); + assertThat(bytesValues.getValueByOrd(0).utf8ToString(), equalTo("10")); + assertThat(bytesValues.getValueByOrd(1).utf8ToString(), equalTo("100")); } { @@ -127,8 +127,8 @@ public class FilterFieldDataTest extends AbstractFieldDataTests { AtomicFieldData.WithOrdinals loadDirect = (WithOrdinals) fieldData.loadDirect(context); BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean()); Docs ordinals = bytesValues.ordinals(); - assertThat(2L, equalTo(ordinals.getMaxOrd())); - assertThat(bytesValues.getValueByOrd(1).utf8ToString(), equalTo("100")); + assertThat(1L, equalTo(ordinals.getMaxOrd())); + assertThat(bytesValues.getValueByOrd(0).utf8ToString(), equalTo("100")); } } @@ -171,8 +171,8 @@ public class FilterFieldDataTest extends AbstractFieldDataTests { AtomicFieldData.WithOrdinals loadDirect = (WithOrdinals) fieldData.loadDirect(context); BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean()); Docs ordinals = bytesValues.ordinals(); - assertThat(2L, equalTo(ordinals.getMaxOrd())); - assertThat(bytesValues.getValueByOrd(1).utf8ToString(), equalTo("5")); + assertThat(1L, equalTo(ordinals.getMaxOrd())); + assertThat(bytesValues.getValueByOrd(0).utf8ToString(), equalTo("5")); } { ifdService.clear(); @@ -182,9 +182,9 @@ public class FilterFieldDataTest extends AbstractFieldDataTests { AtomicFieldData.WithOrdinals loadDirect = (WithOrdinals) fieldData.loadDirect(context); BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean()); Docs ordinals = bytesValues.ordinals(); - assertThat(3L, equalTo(ordinals.getMaxOrd())); - assertThat(bytesValues.getValueByOrd(1).utf8ToString(), equalTo("10")); - assertThat(bytesValues.getValueByOrd(2).utf8ToString(), equalTo("5")); + assertThat(2L, equalTo(ordinals.getMaxOrd())); + assertThat(bytesValues.getValueByOrd(0).utf8ToString(), equalTo("10")); + assertThat(bytesValues.getValueByOrd(1).utf8ToString(), equalTo("5")); } } diff --git a/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java b/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java index 38ba65fd7d6..7abda803c1c 100644 --- a/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java +++ b/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java @@ -51,7 +51,7 @@ public class MultiOrdinalsTests extends ElasticsearchTestCase { OrdinalsBuilder builder = new OrdinalsBuilder(numDocs); Set ordsAndIdSet = new HashSet<>(); for (int i = 0; i < numValues; i++) { - ordsAndIdSet.add(new OrdAndId(1 + random.nextInt(numOrdinals), random.nextInt(numDocs))); + ordsAndIdSet.add(new OrdAndId(random.nextInt(numOrdinals), random.nextInt(numDocs))); } List ordsAndIds = new ArrayList<>(ordsAndIdSet); Collections.sort(ordsAndIds, new Comparator() { @@ -181,31 +181,31 @@ public class MultiOrdinalsTests extends ElasticsearchTestCase { int maxDoc = 7; long maxOrds = 32; OrdinalsBuilder builder = new OrdinalsBuilder(maxDoc); - builder.nextOrdinal(); // 1 + builder.nextOrdinal(); // 0 builder.addDoc(1).addDoc(4).addDoc(5).addDoc(6); - builder.nextOrdinal(); // 2 + builder.nextOrdinal(); // 1 builder.addDoc(0).addDoc(5).addDoc(6); builder.nextOrdinal(); // 3 builder.addDoc(2).addDoc(4).addDoc(5).addDoc(6); - builder.nextOrdinal(); // 4 + builder.nextOrdinal(); // 3 builder.addDoc(0).addDoc(4).addDoc(5).addDoc(6); + builder.nextOrdinal(); // 4 + builder.addDoc(4).addDoc(5).addDoc(6); builder.nextOrdinal(); // 5 builder.addDoc(4).addDoc(5).addDoc(6); - long ord = builder.nextOrdinal(); // 6 - builder.addDoc(4).addDoc(5).addDoc(6); - for (long i = ord; i < maxOrds; i++) { + while (builder.getMaxOrd() < maxOrds) { builder.nextOrdinal(); builder.addDoc(5).addDoc(6); } long[][] ordinalPlan = new long[][]{ - {2, 4}, - {1}, - {3}, + {1, 3}, + {0}, + {2}, {}, - {1, 3, 4, 5, 6}, - {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32}, - {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32} + {0, 2, 3, 4, 5}, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31}, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31} }; Ordinals ordinals = creationMultiOrdinals(builder); @@ -251,13 +251,13 @@ public class MultiOrdinalsTests extends ElasticsearchTestCase { } long[][] ordinalPlan = new long[][]{ - {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, - {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, + {0,1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}, + {0}, + {0, 1, 2, 3, 4}, + {0, 1, 2, 3, 4, 5}, {1}, - {1, 2, 3, 4, 5}, - {1, 2, 3, 4, 5, 6}, - {2}, - {1, 2, 3, 4, 5, 6, 7, 8, 9, 10} + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9} }; Ordinals ordinals = new MultiOrdinals(builder, PackedInts.FASTEST); @@ -266,14 +266,13 @@ public class MultiOrdinalsTests extends ElasticsearchTestCase { } private void assertEquals(Ordinals.Docs docs, long[][] ordinalPlan) { - long numOrds = 0; + long maxOrd = 0; for (int doc = 0; doc < ordinalPlan.length; ++doc) { if (ordinalPlan[doc].length > 0) { - numOrds = Math.max(numOrds, ordinalPlan[doc][ordinalPlan[doc].length - 1]); + maxOrd = Math.max(maxOrd, 1 + ordinalPlan[doc][ordinalPlan[doc].length - 1]); } } - assertThat(docs.getMaxOrd(), equalTo(numOrds + Ordinals.MIN_ORDINAL)); // Includes null ord - assertThat(docs.getMaxOrd(), equalTo(numOrds + 1)); + assertThat(docs.getMaxOrd(), equalTo(maxOrd)); assertThat(docs.isMultiValued(), equalTo(true)); for (int doc = 0; doc < ordinalPlan.length; ++doc) { long[] ords = ordinalPlan[doc];