You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mi...@apache.org on 2016/12/09 23:05:22 UTC
lucene-solr:master: LUCENE-7581: don't allow updating a doc values
field if it's used in the index sort
Repository: lucene-solr
Updated Branches:
refs/heads/master 22d04a7c1 -> 4efbde4e7
LUCENE-7581: don't allow updating a doc values field if it's used in the index sort
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/4efbde4e
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/4efbde4e
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/4efbde4e
Branch: refs/heads/master
Commit: 4efbde4e76277f364952866c071bb953ca2be070
Parents: 22d04a7
Author: Mike McCandless <mi...@apache.org>
Authored: Fri Dec 9 18:05:13 2016 -0500
Committer: Mike McCandless <mi...@apache.org>
Committed: Fri Dec 9 18:05:13 2016 -0500
----------------------------------------------------------------------
lucene/CHANGES.txt | 4 +++
.../org/apache/lucene/index/IndexWriter.java | 6 ++++
.../apache/lucene/index/IndexWriterConfig.java | 3 ++
.../lucene/index/LiveIndexWriterConfig.java | 13 +++++++++
.../apache/lucene/index/TestIndexSorting.java | 30 ++++++++++++++++++--
5 files changed, 53 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4efbde4e/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index b9deb7e..da6e3d2 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -90,6 +90,10 @@ Bug Fixes
that does not extend ReflectiveAccessException in Java 9.
(Uwe Schindler)
+* LUCENE-7581: Lucene now prevents updating a doc values field that is used
+ in the index sort, since this would lead to corruption. (Jim
+ Ferenczi via Mike McCandless)
+
Improvements
* LUCENE-6824: TermAutomatonQuery now rewrites to TermQuery,
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4efbde4e/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
----------------------------------------------------------------------
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 9868785..3ee87b1 100644
--- a/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
+++ b/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
@@ -1619,6 +1619,9 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable {
if (!globalFieldNumberMap.contains(field, DocValuesType.NUMERIC)) {
throw new IllegalArgumentException("can only update existing numeric-docvalues fields!");
}
+ if (config.getIndexSortFields().contains(field)) {
+ throw new IllegalArgumentException("cannot update docvalues field involved in the index sort, field=" + field + ", sort=" + config.getIndexSort());
+ }
try {
long seqNo = docWriter.updateDocValues(new NumericDocValuesUpdate(term, field, value));
if (seqNo < 0) {
@@ -1713,6 +1716,9 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable {
if (!globalFieldNumberMap.contains(f.name(), dvType)) {
throw new IllegalArgumentException("can only update existing docvalues fields! field=" + f.name() + ", type=" + dvType);
}
+ if (config.getIndexSortFields().contains(f.name())) {
+ throw new IllegalArgumentException("cannot update docvalues field involved in the index sort, field=" + f.name() + ", sort=" + config.getIndexSort());
+ }
switch (dvType) {
case NUMERIC:
dvUpdates[i] = new NumericDocValuesUpdate(term, f.name(), (Long) f.numericValue());
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4efbde4e/lucene/core/src/java/org/apache/lucene/index/IndexWriterConfig.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/IndexWriterConfig.java b/lucene/core/src/java/org/apache/lucene/index/IndexWriterConfig.java
index 4f642ee..ce4f0a8 100644
--- a/lucene/core/src/java/org/apache/lucene/index/IndexWriterConfig.java
+++ b/lucene/core/src/java/org/apache/lucene/index/IndexWriterConfig.java
@@ -18,7 +18,9 @@ package org.apache.lucene.index;
import java.io.PrintStream;
+import java.util.Arrays;
import java.util.EnumSet;
+import java.util.stream.Collectors;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.standard.StandardAnalyzer;
@@ -474,6 +476,7 @@ public final class IndexWriterConfig extends LiveIndexWriterConfig {
}
}
this.indexSort = sort;
+ this.indexSortFields = Arrays.stream(sort.getSort()).map((s) -> s.getField()).collect(Collectors.toSet());
return this;
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4efbde4e/lucene/core/src/java/org/apache/lucene/index/LiveIndexWriterConfig.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/LiveIndexWriterConfig.java b/lucene/core/src/java/org/apache/lucene/index/LiveIndexWriterConfig.java
index cec70c0..d9e1bc7 100644
--- a/lucene/core/src/java/org/apache/lucene/index/LiveIndexWriterConfig.java
+++ b/lucene/core/src/java/org/apache/lucene/index/LiveIndexWriterConfig.java
@@ -17,6 +17,9 @@
package org.apache.lucene.index;
+import java.util.Collections;
+import java.util.Set;
+
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.codecs.Codec;
import org.apache.lucene.index.DocumentsWriterPerThread.IndexingChain;
@@ -98,6 +101,9 @@ public class LiveIndexWriterConfig {
/** The sort order to use to write merged segments. */
protected Sort indexSort = null;
+ /** The field names involved in the index sort */
+ protected Set<String> indexSortFields = Collections.emptySet();
+
// used by IndexWriterConfig
LiveIndexWriterConfig(Analyzer analyzer) {
this.analyzer = analyzer;
@@ -457,6 +463,13 @@ public class LiveIndexWriterConfig {
return indexSort;
}
+ /**
+ * Returns the field names involved in the index sort
+ */
+ public Set<String> getIndexSortFields() {
+ return indexSortFields;
+ }
+
@Override
public String toString() {
StringBuilder sb = new StringBuilder();
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4efbde4e/lucene/core/src/test/org/apache/lucene/index/TestIndexSorting.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestIndexSorting.java b/lucene/core/src/test/org/apache/lucene/index/TestIndexSorting.java
index 5ebf8f4..08a85ef 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestIndexSorting.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestIndexSorting.java
@@ -1700,6 +1700,29 @@ public class TestIndexSorting extends LuceneTestCase {
dir.close();
}
+
+ // docvalues fields involved in the index sort cannot be updated
+ public void testBadDVUpdate() throws Exception {
+ Directory dir = newDirectory();
+ IndexWriterConfig iwc = new IndexWriterConfig(new MockAnalyzer(random()));
+ Sort indexSort = new Sort(new SortField("foo", SortField.Type.LONG));
+ iwc.setIndexSort(indexSort);
+ IndexWriter w = new IndexWriter(dir, iwc);
+ Document doc = new Document();
+ doc.add(new StringField("id", new BytesRef("0"), Store.NO));
+ doc.add(new NumericDocValuesField("foo", random().nextInt()));
+ w.addDocument(doc);
+ w.commit();
+ IllegalArgumentException exc = expectThrows(IllegalArgumentException.class,
+ () -> w.updateDocValues(new Term("id", "0"), new NumericDocValuesField("foo", -1)));
+ assertEquals(exc.getMessage(), "cannot update docvalues field involved in the index sort, field=foo, sort=<long: \"foo\">");
+ exc = expectThrows(IllegalArgumentException.class,
+ () -> w.updateNumericDocValue(new Term("id", "0"), "foo", -1));
+ assertEquals(exc.getMessage(), "cannot update docvalues field involved in the index sort, field=foo, sort=<long: \"foo\">");
+ w.close();
+ dir.close();
+ }
+
static class DVUpdateRunnable implements Runnable {
private final int numDocs;
@@ -1727,7 +1750,7 @@ public class TestIndexSorting extends LuceneTestCase {
final long value = random.nextInt(20);
synchronized (values) {
- w.updateDocValues(new Term("id", Integer.toString(id)), new NumericDocValuesField("foo", value));
+ w.updateDocValues(new Term("id", Integer.toString(id)), new NumericDocValuesField("bar", value));
values.put(id, value);
}
@@ -1762,7 +1785,8 @@ public class TestIndexSorting extends LuceneTestCase {
for (int i = 0; i < numDocs; ++i) {
Document doc = new Document();
doc.add(new StringField("id", Integer.toString(i), Store.NO));
- doc.add(new NumericDocValuesField("foo", -1));
+ doc.add(new NumericDocValuesField("foo", random().nextInt()));
+ doc.add(new NumericDocValuesField("bar", -1));
w.addDocument(doc);
values.put(i, -1L);
}
@@ -1786,7 +1810,7 @@ public class TestIndexSorting extends LuceneTestCase {
for (int i = 0; i < numDocs; ++i) {
final TopDocs topDocs = searcher.search(new TermQuery(new Term("id", Integer.toString(i))), 1);
assertEquals(1, topDocs.totalHits);
- NumericDocValues dvs = MultiDocValues.getNumericValues(reader, "foo");
+ NumericDocValues dvs = MultiDocValues.getNumericValues(reader, "bar");
int hitDoc = topDocs.scoreDocs[0].doc;
assertEquals(hitDoc, dvs.advance(hitDoc));
assertEquals(values.get(i).longValue(), dvs.longValue());
RE: lucene-solr:master: LUCENE-7581: don't allow updating a doc values field if it's used in the index sort
Posted by Uwe Schindler <uw...@thetaphi.de>.
Hi,
> > + this.indexSortFields = Arrays.stream(sort.getSort()).map((s) ->
> > s.getField()).collect(Collectors.toSet());
>
> I'd use method references instead of Lambdas:
>
> Instead of: map((s) -> s.getField())
> Use: map(s::getField)
Correction: map(SortField::getField)
> This spares a synthetic lambda$xxx method that clutters stack trace and is
> more readable.
>
> Uwe
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: lucene-solr:master: LUCENE-7581: don't allow updating a doc
values field if it's used in the index sort
Posted by Michael McCandless <lu...@mikemccandless.com>.
Thanks Uwe, I'll fix.
Mike McCandless
http://blog.mikemccandless.com
On Fri, Dec 9, 2016 at 6:25 PM, Uwe Schindler <uw...@thetaphi.de> wrote:
> Hi,
>
>> + this.indexSortFields = Arrays.stream(sort.getSort()).map((s) ->
>> s.getField()).collect(Collectors.toSet());
>
> I'd use method references instead of Lambdas:
>
> Instead of: map((s) -> s.getField())
> Use: map(s::getField)
>
> This spares a synthetic lambda$xxx method that clutters stack trace and is more readable.
>
> Uwe
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
RE: lucene-solr:master: LUCENE-7581: don't allow updating a doc values field if it's used in the index sort
Posted by Uwe Schindler <uw...@thetaphi.de>.
Hi,
> + this.indexSortFields = Arrays.stream(sort.getSort()).map((s) ->
> s.getField()).collect(Collectors.toSet());
I'd use method references instead of Lambdas:
Instead of: map((s) -> s.getField())
Use: map(s::getField)
This spares a synthetic lambda$xxx method that clutters stack trace and is more readable.
Uwe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org