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/11/02 10:12:35 UTC
lucene-solr:master: LUCENE-7501: fix back-compat bug; add test
Repository: lucene-solr
Updated Branches:
refs/heads/master 5a66b3bc0 -> 69e654b37
LUCENE-7501: fix back-compat bug; add test
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/69e654b3
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/69e654b3
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/69e654b3
Branch: refs/heads/master
Commit: 69e654b3737a97fea7ffc9f57c8fad5e85f72452
Parents: 5a66b3b
Author: Mike McCandless <mi...@apache.org>
Authored: Wed Nov 2 05:36:00 2016 -0400
Committer: Mike McCandless <mi...@apache.org>
Committed: Wed Nov 2 05:48:19 2016 -0400
----------------------------------------------------------------------
.../index/TestBackwardsCompatibility.java | 1 +
.../lucene/index/TestManyPointsInOldIndex.java | 74 +++++++++++++++++++
.../org/apache/lucene/index/manypointsindex.zip | Bin 0 -> 3739 bytes
.../org/apache/lucene/index/CheckIndex.java | 31 +++++---
.../org/apache/lucene/util/bkd/BKDReader.java | 48 ++++++++++--
5 files changed, 137 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/69e654b3/lucene/backward-codecs/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java
----------------------------------------------------------------------
diff --git a/lucene/backward-codecs/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java b/lucene/backward-codecs/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java
index 2371b01..8bf7141 100644
--- a/lucene/backward-codecs/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java
+++ b/lucene/backward-codecs/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java
@@ -157,6 +157,7 @@ public class TestBackwardsCompatibility extends LuceneTestCase {
for(int i=0;i<50;i++) {
writer.addDocument(docs.nextDoc());
}
+ docs.close();
writer.close();
dir.close();
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/69e654b3/lucene/backward-codecs/src/test/org/apache/lucene/index/TestManyPointsInOldIndex.java
----------------------------------------------------------------------
diff --git a/lucene/backward-codecs/src/test/org/apache/lucene/index/TestManyPointsInOldIndex.java b/lucene/backward-codecs/src/test/org/apache/lucene/index/TestManyPointsInOldIndex.java
new file mode 100644
index 0000000..043979b
--- /dev/null
+++ b/lucene/backward-codecs/src/test/org/apache/lucene/index/TestManyPointsInOldIndex.java
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.index;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import org.apache.lucene.document.Document;
+import org.apache.lucene.document.IntPoint;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.IndexWriterConfig;
+import org.apache.lucene.store.BaseDirectoryWrapper;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.FSDirectory;
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.TestUtil;
+
+// LUCENE-7501
+public class TestManyPointsInOldIndex extends LuceneTestCase {
+
+// To regenerate the back index zip:
+//
+// Compile:
+// 1) temporarily remove 'extends LuceneTestCase' above (else java doesn't see our static void main)
+// 2) ant compile-test
+//
+// Run:
+// 1) java -cp ../build/backward-codecs/classes/test:../build/core/classes/java org.apache.lucene.index.TestManyPointsInOldIndex
+//
+// cd manypointsindex
+// zip manypointsindex.zip *
+
+ public static void main(String[] args) throws IOException {
+ Directory dir = FSDirectory.open(Paths.get("manypointsindex"));
+ IndexWriter w = new IndexWriter(dir, new IndexWriterConfig());
+ for(int i=0;i<1025;i++) {
+ Document doc = new Document();
+ doc.add(new IntPoint("intpoint", 1025-i));
+ w.addDocument(doc);
+ }
+ w.close();
+ dir.close();
+ }
+
+ public void testCheckOldIndex() throws IOException {
+ Path path = createTempDir("manypointsindex");
+ InputStream resource = getClass().getResourceAsStream("manypointsindex.zip");
+ assertNotNull("manypointsindex not found", resource);
+ TestUtil.unzip(resource, path);
+ BaseDirectoryWrapper dir = newFSDirectory(path);
+ // disable default checking...
+ dir.setCheckIndexOnClose(false);
+
+ // ... because we check ourselves here:
+ TestUtil.checkIndex(dir, false, true, null);
+ dir.close();
+ }
+}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/69e654b3/lucene/backward-codecs/src/test/org/apache/lucene/index/manypointsindex.zip
----------------------------------------------------------------------
diff --git a/lucene/backward-codecs/src/test/org/apache/lucene/index/manypointsindex.zip b/lucene/backward-codecs/src/test/org/apache/lucene/index/manypointsindex.zip
new file mode 100644
index 0000000..c7c0bf7
Binary files /dev/null and b/lucene/backward-codecs/src/test/org/apache/lucene/index/manypointsindex.zip differ
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/69e654b3/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java b/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
index f3d3562..7bc08f3 100644
--- a/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
+++ b/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
@@ -771,8 +771,10 @@ public final class CheckIndex implements Closeable {
throw new RuntimeException("Stored Field test failed");
} else if (segInfoStat.termVectorStatus.error != null) {
throw new RuntimeException("Term Vector test failed");
- } else if (segInfoStat.docValuesStatus.error != null) {
+ } else if (segInfoStat.docValuesStatus.error != null) {
throw new RuntimeException("DocValues test failed");
+ } else if (segInfoStat.pointsStatus.error != null) {
+ throw new RuntimeException("Points test failed");
}
}
@@ -1865,12 +1867,12 @@ public final class CheckIndex implements Closeable {
// Compare to last cell:
if (StringHelper.compare(bytesPerDim, packedValue, offset, lastMinPackedValue, offset) < 0) {
// This doc's point, in this dimension, is lower than the minimum value of the last cell checked:
- throw new RuntimeException("packed points value " + Arrays.toString(packedValue) + " for docID=" + docID + " is out-of-bounds of the last cell min=" + Arrays.toString(lastMinPackedValue) + " max=" + Arrays.toString(lastMaxPackedValue) + " dim=" + dim);
+ throw new RuntimeException("packed points value " + Arrays.toString(packedValue) + " for field=\"" + fieldInfo.name + "\", docID=" + docID + " is out-of-bounds of the last cell min=" + Arrays.toString(lastMinPackedValue) + " max=" + Arrays.toString(lastMaxPackedValue) + " dim=" + dim);
}
if (StringHelper.compare(bytesPerDim, packedValue, offset, lastMaxPackedValue, offset) > 0) {
// This doc's point, in this dimension, is greater than the maximum value of the last cell checked:
- throw new RuntimeException("packed points value " + Arrays.toString(packedValue) + " for docID=" + docID + " is out-of-bounds of the last cell min=" + Arrays.toString(lastMinPackedValue) + " max=" + Arrays.toString(lastMaxPackedValue) + " dim=" + dim);
+ throw new RuntimeException("packed points value " + Arrays.toString(packedValue) + " for field=\"" + fieldInfo.name + "\", docID=" + docID + " is out-of-bounds of the last cell min=" + Arrays.toString(lastMinPackedValue) + " max=" + Arrays.toString(lastMaxPackedValue) + " dim=" + dim);
}
}
@@ -1879,10 +1881,10 @@ public final class CheckIndex implements Closeable {
if (dimCount == 1) {
int cmp = StringHelper.compare(bytesPerDim, lastPackedValue, 0, packedValue, 0);
if (cmp > 0) {
- throw new RuntimeException("packed points value " + Arrays.toString(packedValue) + " for docID=" + docID + " is out-of-order vs the previous document's value " + Arrays.toString(lastPackedValue));
+ throw new RuntimeException("packed points value " + Arrays.toString(packedValue) + " for field=\"" + fieldInfo.name + "\", for docID=" + docID + " is out-of-order vs the previous document's value " + Arrays.toString(lastPackedValue));
} else if (cmp == 0) {
if (docID < lastDocID) {
- throw new RuntimeException("packed points value is the same, but docID=" + docID + " is out of order vs previous docID=" + lastDocID);
+ throw new RuntimeException("packed points value is the same, but docID=" + docID + " is out of order vs previous docID=" + lastDocID + ", field=\"" + fieldInfo.name + "\"");
}
}
System.arraycopy(packedValue, 0, lastPackedValue, 0, bytesPerDim);
@@ -1902,24 +1904,29 @@ public final class CheckIndex implements Closeable {
for(int dim=0;dim<dimCount;dim++) {
int offset = bytesPerDim * dim;
+ if (StringHelper.compare(bytesPerDim, minPackedValue, offset, maxPackedValue, offset) > 0) {
+ throw new RuntimeException("packed points cell minPackedValue " + Arrays.toString(minPackedValue) +
+ " is out-of-bounds of the cell's maxPackedValue " + Arrays.toString(maxPackedValue) + " dim=" + dim + " field=\"" + fieldInfo.name + "\"");
+ }
+
// Make sure this cell is not outside of the global min/max:
if (StringHelper.compare(bytesPerDim, minPackedValue, offset, globalMinPackedValue, offset) < 0) {
throw new RuntimeException("packed points cell minPackedValue " + Arrays.toString(minPackedValue) +
- " is out-of-bounds of the global minimum " + Arrays.toString(globalMinPackedValue) + " dim=" + dim);
+ " is out-of-bounds of the global minimum " + Arrays.toString(globalMinPackedValue) + " dim=" + dim + " field=\"" + fieldInfo.name + "\"");
}
if (StringHelper.compare(bytesPerDim, maxPackedValue, offset, globalMinPackedValue, offset) < 0) {
- throw new RuntimeException("packed points cell maxPackedValue " + Arrays.toString(minPackedValue) +
- " is out-of-bounds of the global minimum " + Arrays.toString(globalMinPackedValue) + " dim=" + dim);
+ throw new RuntimeException("packed points cell maxPackedValue " + Arrays.toString(maxPackedValue) +
+ " is out-of-bounds of the global minimum " + Arrays.toString(globalMinPackedValue) + " dim=" + dim + " field=\"" + fieldInfo.name + "\"");
}
if (StringHelper.compare(bytesPerDim, minPackedValue, offset, globalMaxPackedValue, offset) > 0) {
throw new RuntimeException("packed points cell minPackedValue " + Arrays.toString(minPackedValue) +
- " is out-of-bounds of the global maximum " + Arrays.toString(globalMaxPackedValue) + " dim=" + dim);
+ " is out-of-bounds of the global maximum " + Arrays.toString(globalMaxPackedValue) + " dim=" + dim + " field=\"" + fieldInfo.name + "\"");
}
if (StringHelper.compare(bytesPerDim, maxPackedValue, offset, globalMaxPackedValue, offset) > 0) {
throw new RuntimeException("packed points cell maxPackedValue " + Arrays.toString(maxPackedValue) +
- " is out-of-bounds of the global maximum " + Arrays.toString(globalMaxPackedValue) + " dim=" + dim);
+ " is out-of-bounds of the global maximum " + Arrays.toString(globalMaxPackedValue) + " dim=" + dim + " field=\"" + fieldInfo.name + "\"");
}
}
@@ -1930,11 +1937,11 @@ public final class CheckIndex implements Closeable {
private void checkPackedValue(String desc, byte[] packedValue, int docID) {
if (packedValue == null) {
- throw new RuntimeException(desc + " is null for docID=" + docID);
+ throw new RuntimeException(desc + " is null for docID=" + docID + " field=\"" + fieldInfo.name + "\"");
}
if (packedValue.length != packedBytesCount) {
- throw new RuntimeException(desc + " has incorrect length=" + packedValue.length + " vs expected=" + packedBytesCount + " for docID=" + docID);
+ throw new RuntimeException(desc + " has incorrect length=" + packedValue.length + " vs expected=" + packedBytesCount + " for docID=" + docID + " field=\"" + fieldInfo.name + "\"");
}
}
});
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/69e654b3/lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java b/lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
index 1ddb566..6bf7dfc 100644
--- a/lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
+++ b/lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
@@ -69,6 +69,12 @@ public class BKDReader extends PointValues implements Accountable {
in.readBytes(minPackedValue, 0, packedBytesLength);
in.readBytes(maxPackedValue, 0, packedBytesLength);
+ for(int dim=0;dim<numDims;dim++) {
+ if (StringHelper.compare(bytesPerDim, minPackedValue, dim*bytesPerDim, maxPackedValue, dim*bytesPerDim) > 0) {
+ throw new CorruptIndexException("minPackedValue " + new BytesRef(minPackedValue) + " is > maxPackedValue " + new BytesRef(maxPackedValue) + " for dim=" + dim, in);
+ }
+ }
+
pointCount = in.readVLong();
docCount = in.readVInt();
@@ -137,6 +143,7 @@ public class BKDReader extends PointValues implements Accountable {
this.numDims = numDims;
this.maxPointsInLeafNode = maxPointsInLeafNode;
this.bytesPerDim = bytesPerDim;
+ // no version check here because callers of this API (SimpleText) have no back compat:
bytesPerIndexEntry = numDims == 1 ? bytesPerDim : bytesPerDim + 1;
packedBytesLength = numDims * bytesPerDim;
this.leafNodeOffset = leafBlockFPs.length;
@@ -238,7 +245,18 @@ public class BKDReader extends PointValues implements Accountable {
// Non-leaf node:
int address = nodeID * bytesPerIndexEntry;
- int splitDim = numDims == 1 ? 0 : splitPackedValues[address++] & 0xff;
+ int splitDim;
+ if (numDims == 1) {
+ splitDim = 0;
+ if (version < BKDWriter.VERSION_IMPLICIT_SPLIT_DIM_1D) {
+ // skip over wastefully encoded 0 splitDim:
+ assert splitPackedValues[address] == 0;
+ address++;
+ }
+ } else {
+ splitDim = splitPackedValues[address++] & 0xff;
+ }
+
assert splitDim < numDims;
byte[] splitPackedValue = new byte[packedBytesLength];
@@ -459,14 +477,23 @@ public class BKDReader extends PointValues implements Accountable {
// Non-leaf node: recurse on the split left and right nodes
- // TODO: save the unused 1 byte prefix (it's always 0) in the 1d case here:
int address = nodeID * bytesPerIndexEntry;
- int splitDim = numDims == 1 ? 0 : splitPackedValues[address++] & 0xff;
+ int splitDim;
+ if (numDims == 1) {
+ splitDim = 0;
+ if (version < BKDWriter.VERSION_IMPLICIT_SPLIT_DIM_1D) {
+ // skip over wastefully encoded 0 splitDim:
+ assert splitPackedValues[address] == 0;
+ address++;
+ }
+ } else {
+ splitDim = splitPackedValues[address++] & 0xff;
+ }
+
assert splitDim < numDims;
// TODO: can we alloc & reuse this up front?
- // TODO: can we alloc & reuse this up front?
byte[] splitPackedValue = new byte[packedBytesLength];
// Recurse on left sub-tree:
@@ -488,7 +515,18 @@ public class BKDReader extends PointValues implements Accountable {
/** Copies the split value for this node into the provided byte array */
public void copySplitValue(int nodeID, byte[] splitPackedValue) {
int address = nodeID * bytesPerIndexEntry;
- int splitDim = numDims == 1 ? 0 : splitPackedValues[address++] & 0xff;
+ int splitDim;
+ if (numDims == 1) {
+ splitDim = 0;
+ if (version < BKDWriter.VERSION_IMPLICIT_SPLIT_DIM_1D) {
+ // skip over wastefully encoded 0 splitDim:
+ assert splitPackedValues[address] == 0;
+ address++;
+ }
+ } else {
+ splitDim = splitPackedValues[address++] & 0xff;
+ }
+
assert splitDim < numDims;
System.arraycopy(splitPackedValues, address, splitPackedValue, splitDim*bytesPerDim, bytesPerDim);
}