From ca5063df99e777098d87da1443b8c7f063480d7b Mon Sep 17 00:00:00 2001 From: Michael McCandless Date: Thu, 23 Oct 2014 08:46:36 +0000 Subject: [PATCH] LUCENE-6019: add another [failing and then fixed] test; do not set FieldInfo.docValueType when it disgrees with low-schema git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1633770 13f79535-47bb-0310-9956-ffa450edef68 --- .../lucene/index/DefaultIndexingChain.java | 6 ++-- .../org/apache/lucene/index/FieldInfos.java | 10 ++++--- .../org/apache/lucene/index/IndexWriter.java | 4 +-- .../apache/lucene/index/SegmentDocValues.java | 24 ++++++++-------- .../lucene/index/TestDocValuesIndexing.java | 28 +++++++++++++++++++ 5 files changed, 51 insertions(+), 21 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java b/lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java index c4dff5b9961..ab778895a4e 100644 --- a/lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java +++ b/lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java @@ -387,12 +387,12 @@ final class DefaultIndexingChain extends DocConsumer { boolean hasDocValues = fp.fieldInfo.hasDocValues(); - // This will throw an exc if the caller tried to - // change the DV type for the field: - fp.fieldInfo.setDocValuesType(dvType); if (hasDocValues == false) { + // This will throw an exc if the caller tried to + // change the DV type for the field: fieldInfos.globalFieldNumbers.setDocValuesType(fp.fieldInfo.number, fp.fieldInfo.name, dvType); } + fp.fieldInfo.setDocValuesType(dvType); int docID = docState.docID; diff --git a/lucene/core/src/java/org/apache/lucene/index/FieldInfos.java b/lucene/core/src/java/org/apache/lucene/index/FieldInfos.java index e7a0184dc80..8c01184cd1f 100644 --- a/lucene/core/src/java/org/apache/lucene/index/FieldInfos.java +++ b/lucene/core/src/java/org/apache/lucene/index/FieldInfos.java @@ -314,14 +314,16 @@ public class FieldInfos implements Iterable { fi.update(storeTermVector, omitNorms, storePayloads, indexOptions); if (docValues != null) { - // only pay the synchronization cost if fi does not already have a DVType + // Only pay the synchronization cost if fi does not already have a DVType boolean updateGlobal = !fi.hasDocValues(); - fi.setDocValuesType(docValues); // this will also perform the consistency check. if (updateGlobal) { - // must also update docValuesType map so it's - // aware of this field's DocValueType + // Must also update docValuesType map so it's + // aware of this field's DocValueType. This will throw IllegalArgumentException if + // an illegal type change was attempted. globalFieldNumbers.setDocValuesType(fi.number, name, docValues); } + + fi.setDocValuesType(docValues); // this will also perform the consistency check. } } return fi; diff --git a/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java b/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java index 5b07ae9ff4f..f4191ad7c32 100644 --- a/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java +++ b/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java @@ -3931,8 +3931,8 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable { // we pass merge.getMergeReaders() instead of merge.readers to allow the // OneMerge to return a view over the actual segments to merge final SegmentMerger merger = new SegmentMerger(merge.getMergeReaders(), - merge.info.info, infoStream, dirWrapper, - checkAbort, globalFieldNumberMap, + merge.info.info, infoStream, dirWrapper, + checkAbort, globalFieldNumberMap, context); merge.checkAborted(directory); diff --git a/lucene/core/src/java/org/apache/lucene/index/SegmentDocValues.java b/lucene/core/src/java/org/apache/lucene/index/SegmentDocValues.java index ffb4fba91b6..66006597d23 100644 --- a/lucene/core/src/java/org/apache/lucene/index/SegmentDocValues.java +++ b/lucene/core/src/java/org/apache/lucene/index/SegmentDocValues.java @@ -1,17 +1,5 @@ package org.apache.lucene.index; -import java.io.IOException; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import org.apache.lucene.codecs.DocValuesFormat; -import org.apache.lucene.codecs.DocValuesProducer; -import org.apache.lucene.store.Directory; -import org.apache.lucene.store.IOContext; -import org.apache.lucene.util.IOUtils; -import org.apache.lucene.util.RefCount; - /* * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with @@ -29,6 +17,18 @@ import org.apache.lucene.util.RefCount; * limitations under the License. */ +import java.io.IOException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.apache.lucene.codecs.DocValuesFormat; +import org.apache.lucene.codecs.DocValuesProducer; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.util.IOUtils; +import org.apache.lucene.util.RefCount; + /** * Manages the {@link DocValuesProducer} held by {@link SegmentReader} and * keeps track of their reference counting. diff --git a/lucene/core/src/test/org/apache/lucene/index/TestDocValuesIndexing.java b/lucene/core/src/test/org/apache/lucene/index/TestDocValuesIndexing.java index 06dfd27a76d..bce64c9c46f 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestDocValuesIndexing.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestDocValuesIndexing.java @@ -511,6 +511,34 @@ public class TestDocValuesIndexing extends LuceneTestCase { dir.close(); } + public void testMixedTypesAfterReopenAppend3() throws IOException { + Directory dir = newDirectory(); + IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random()))) ; + Document doc = new Document(); + doc.add(new SortedSetDocValuesField("foo", new BytesRef("foo"))); + w.addDocument(doc); + w.close(); + + doc = new Document(); + w = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random()))); + doc.add(new StringField("foo", "bar", Field.Store.NO)); + doc.add(new BinaryDocValuesField("foo", new BytesRef("foo"))); + try { + // NOTE: this case follows a different code path inside + // DefaultIndexingChain/FieldInfos, because the field (foo) + // is first added without DocValues: + w.addDocument(doc); + fail("did not get expected exception"); + } catch (IllegalArgumentException iae) { + // expected + } + // Also add another document so there is a segment to write here: + w.addDocument(new Document()); + w.forceMerge(1); + w.close(); + dir.close(); + } + // Two documents with same field as different types, added // from separate threads: public void testMixedTypesDifferentThreads() throws Exception {