lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mikemcc...@apache.org
Subject lucene-solr:branch_6x: LUCENE-7501: fix back-compat bug; add test
Date Wed, 02 Nov 2016 09:36:13 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/branch_6x 830a3fdfb -> 35f3cd9bd


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/35f3cd9b
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/35f3cd9b
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/35f3cd9b

Branch: refs/heads/branch_6x
Commit: 35f3cd9bd95607983fa4704c66f61bf59cd40860
Parents: 830a3fd
Author: Mike McCandless <mikemccand@apache.org>
Authored: Wed Nov 2 05:36:00 2016 -0400
Committer: Mike McCandless <mikemccand@apache.org>
Committed: Wed Nov 2 05:36:00 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/35f3cd9b/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 915b102..4f8ca66 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/35f3cd9b/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/35f3cd9b/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/35f3cd9b/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 9dee2d1..6785f3a 100644
--- a/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
+++ b/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
@@ -769,8 +769,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");
           }
         }
 
@@ -1860,12 +1862,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);
                                    }
                                  }
 
@@ -1874,10 +1876,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);
@@ -1897,24 +1899,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
+ "\"");
                                    }
                                  }                                   
 
@@ -1925,11 +1932,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/35f3cd9b/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 efb8ea6..10285e8 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
@@ -70,6 +70,12 @@ public class BKDReader 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();
 
@@ -138,6 +144,7 @@ public class BKDReader 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;
@@ -239,7 +246,18 @@ public class BKDReader 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];
@@ -460,14 +478,23 @@ public class BKDReader 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:
@@ -489,7 +516,18 @@ public class BKDReader 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);
   }


Mime
View raw message