From 34d3282edee1ce63e3554b5008c86888f403ea21 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 19 Feb 2018 10:27:56 +0100 Subject: [PATCH] LUCENE-8134: Fix validation of index options. There were test failures in case inconsistent index options were introduced in a new segment because checks were not done in the right order. --- .../apache/lucene/index/DefaultIndexingChain.java | 2 +- .../src/java/org/apache/lucene/index/FieldInfos.java | 2 +- .../org/apache/lucene/index/TestIndexOptions.java | 12 ++++++++++-- 3 files changed, 12 insertions(+), 4 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 f4627aa05f8..e4e9773f272 100644 --- a/lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java +++ b/lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java @@ -637,11 +637,11 @@ final class DefaultIndexingChain extends DocConsumer { // Messy: must set this here because e.g. FreqProxTermsWriterPerField looks at the initial // IndexOptions to decide what arrays it must create). assert info.getIndexOptions() == IndexOptions.NONE; - info.setIndexOptions(indexOptions); // This is the first time we are seeing this field indexed, so we now // record the index options so that any future attempt to (illegally) // change the index options of this field, will throw an IllegalArgExc: fieldInfos.globalFieldNumbers.setIndexOptions(info.number, info.name, indexOptions); + info.setIndexOptions(indexOptions); } /** NOTE: not static: accesses at least docState, termsHash. */ 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 6036112b395..78e8e14711c 100644 --- a/lucene/core/src/java/org/apache/lucene/index/FieldInfos.java +++ b/lucene/core/src/java/org/apache/lucene/index/FieldInfos.java @@ -296,7 +296,7 @@ public class FieldInfos implements Iterable { } IndexOptions currentIndexOptions = this.indexOptions.get(name); if (indexOptions != IndexOptions.NONE && currentIndexOptions != null && currentIndexOptions != IndexOptions.NONE && indexOptions != currentIndexOptions) { - throw new IllegalArgumentException("cannot change index options from " + currentIndexOptions + " to " + indexOptions + " for field \"" + name + "\""); + throw new IllegalArgumentException("cannot change field \"" + name + "\" from index options=" + currentIndexOptions + " to inconsistent index options=" + indexOptions); } } diff --git a/lucene/core/src/test/org/apache/lucene/index/TestIndexOptions.java b/lucene/core/src/test/org/apache/lucene/index/TestIndexOptions.java index 4c6ccf2237b..a3ff52422c5 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestIndexOptions.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestIndexOptions.java @@ -33,21 +33,29 @@ public class TestIndexOptions extends LuceneTestCase { for (IndexOptions from : IndexOptions.values()) { for (IndexOptions to : IndexOptions.values()) { for (boolean preExisting : new boolean[] { false, true }) { - doTestChangeIndexOptionsViaAddDocument(preExisting, from, to); + for (boolean onNewSegment : new boolean[] { false, true }) { + doTestChangeIndexOptionsViaAddDocument(preExisting, onNewSegment, from, to); + } } } } } - private void doTestChangeIndexOptionsViaAddDocument(boolean preExistingField, IndexOptions from, IndexOptions to) throws IOException { + private void doTestChangeIndexOptionsViaAddDocument(boolean preExistingField, boolean onNewSegment, IndexOptions from, IndexOptions to) throws IOException { Directory dir = newDirectory(); IndexWriter w = new IndexWriter(dir, newIndexWriterConfig()); if (preExistingField) { w.addDocument(Collections.singleton(new IntPoint("foo", 1))); + if (onNewSegment) { + DirectoryReader.open(w).close(); + } } FieldType ft1 = new FieldType(TextField.TYPE_STORED); ft1.setIndexOptions(from); w.addDocument(Collections.singleton(new Field("foo", "bar", ft1))); + if (onNewSegment) { + DirectoryReader.open(w).close(); + } FieldType ft2 = new FieldType(TextField.TYPE_STORED); ft2.setIndexOptions(to); if (from == IndexOptions.NONE || to == IndexOptions.NONE || from == to) {