revisit payloads API in DocsAndPositionsEnum

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/branches/branch_4x@1372178 13f79535-47bb-0310-9956-ffa450edef68
andyfowler · Aug 12, 2012 · 28031b5 · 28031b5
1 parent ab9e653
commit 28031b5
Show file tree

Hide file tree

Showing 41 changed files with 133 additions and 283 deletions.
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
@@ -22,6 +22,11 @@ API Changes
   had positions or offsets, since this can be configured on a 
   per-field-per-document basis. (Robert Muir)
 
+* Removed DocsAndPositionsEnum.hasPayload() and simplified the
+  contract of getPayload(). It returns null if there is no payload,
+  otherwise returns the current payload. You can now call it multiple
+  times per position if you want. (Robert Muir)
+
 Bug Fixes
 
 * LUCENE-4297: BooleanScorer2 would multiply the coord() factor

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/MappingMultiDocsAndPositionsEnum.java b/lucene/core/src/java/org/apache/lucene/codecs/MappingMultiDocsAndPositionsEnum.java
@@ -126,6 +126,7 @@ public BytesRef getPayload() throws IOException {
     BytesRef payload = current.getPayload();
     if (mergeState.currentPayloadProcessor[upto] != null && payload != null) {
       // to not violate the D&P api, we must give the processor a private copy
+      // TODO: reuse a BytesRef if there is a PPP
       payload = BytesRef.deepCopyOf(payload);
       mergeState.currentPayloadProcessor[upto].processPayload(payload);
       if (payload.length == 0) {
@@ -135,12 +136,5 @@ public BytesRef getPayload() throws IOException {
     }
     return payload;
   }
-
-  @Override
-  public boolean hasPayload() {
-    // TODO: note this is actually bogus if there is a payloadProcessor,
-    // because it might remove it: but lets just remove this method completely
-    return current.hasPayload();
-  }
 }
 
diff --git a/lucene/core/src/java/org/apache/lucene/codecs/PostingsConsumer.java b/lucene/core/src/java/org/apache/lucene/codecs/PostingsConsumer.java
@@ -112,12 +112,7 @@ public TermStats merge(final MergeState mergeState, final DocsEnum postings, fin
         totTF += freq;
         for(int i=0;i<freq;i++) {
           final int position = postingsEnum.nextPosition();
-          final BytesRef payload;
-          if (postingsEnum.hasPayload()) {
-            payload = postingsEnum.getPayload();
-          } else {
-            payload = null;
-          }
+          final BytesRef payload = postingsEnum.getPayload();
           this.addPosition(position, payload, -1, -1);
         }
         this.finishDoc();
@@ -137,12 +132,7 @@ public TermStats merge(final MergeState mergeState, final DocsEnum postings, fin
         totTF += freq;
         for(int i=0;i<freq;i++) {
           final int position = postingsEnum.nextPosition();
-          final BytesRef payload;
-          if (postingsEnum.hasPayload()) {
-            payload = postingsEnum.getPayload();
-          } else {
-            payload = null;
-          }
+          final BytesRef payload = postingsEnum.getPayload();
           this.addPosition(position, payload, postingsEnum.startOffset(), postingsEnum.endOffset());
         }
         this.finishDoc();

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/TermVectorsWriter.java b/lucene/core/src/java/org/apache/lucene/codecs/TermVectorsWriter.java
@@ -271,8 +271,7 @@ protected final void addAllDocVectors(Fields vectors, MergeState mergeState) thr
             final int startOffset = docsAndPositionsEnum.startOffset();
             final int endOffset = docsAndPositionsEnum.endOffset();
 
-            BytesRef payload = docsAndPositionsEnum.hasPayload() ? 
-                docsAndPositionsEnum.getPayload() : null;
+            BytesRef payload = docsAndPositionsEnum.getPayload();
 
             if (payloadProcessor != null && payload != null) {
               // to not violate the D&P api, we must give the processor a private copy

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene3x/Lucene3xFields.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene3x/Lucene3xFields.java
@@ -1089,29 +1089,9 @@ public int endOffset() throws IOException {
       return -1;
     }
 
-    @Override
-    public boolean hasPayload() {
-      assert docID != NO_MORE_DOCS;
-      return pos.isPayloadAvailable();
-    }
-
-    private BytesRef payload;
-
     @Override
     public BytesRef getPayload() throws IOException {
-      final int len = pos.getPayloadLength();
-      if (payload == null) {
-        payload = new BytesRef();
-        payload.bytes = new byte[len];
-      } else {
-        if (payload.bytes.length < len) {
-          payload.grow(len);
-        }
-      }
-
-      payload.bytes = pos.getPayload(payload.bytes, 0);
-      payload.length = len;
-      return payload;
+      return pos.getPayload();
     }
   }
 }
diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene3x/Lucene3xTermVectorsReader.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene3x/Lucene3xTermVectorsReader.java
@@ -640,11 +640,6 @@ public BytesRef getPayload() {
       return null;
     }
 
-    @Override
-    public boolean hasPayload() {
-      return false;
-    }
-
     @Override
     public int nextPosition() {
       assert (positions != null && nextPos < positions.length) ||

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene3x/SegmentTermPositions.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene3x/SegmentTermPositions.java
@@ -23,6 +23,7 @@
 import org.apache.lucene.index.Term;
 import org.apache.lucene.index.FieldInfo.IndexOptions;
 import org.apache.lucene.store.IndexInput;
+import org.apache.lucene.util.BytesRef;
 
 /**
  * @lucene.experimental
@@ -36,6 +37,7 @@ final class SegmentTermPositions
   private int proxCount;
   private int position;
 
+  private BytesRef payload;
   // the current payload length
   private int payloadLength;
   // indicates whether the payload of the current position has
@@ -192,26 +194,24 @@ public int getPayloadLength() {
     return payloadLength;
   }
 
-  public byte[] getPayload(byte[] data, int offset) throws IOException {
-    if (!needToLoadPayload) {
-      throw new IOException("Either no payload exists at this term position or an attempt was made to load it more than once.");
+  public BytesRef getPayload() throws IOException {
+    if (payloadLength <= 0) {
+      return null; // no payload
     }
 
-    // read payloads lazily
-    byte[] retArray;
-    int retOffset;
-    if (data == null || data.length - offset < payloadLength) {
-      // the array is too small to store the payload data,
-      // so we allocate a new one
-      retArray = new byte[payloadLength];
-      retOffset = 0;
-    } else {
-      retArray = data;
-      retOffset = offset;
+    if (needToLoadPayload) {
+      // read payloads lazily
+      if (payload == null) {
+        payload = new BytesRef(payloadLength);
+      } else {
+        payload.grow(payloadLength);
+      }
+
+      proxStream.readBytes(payload.bytes, payload.offset, payloadLength);
+      payload.length = payloadLength;
+      needToLoadPayload = false;
     }
-    proxStream.readBytes(retArray, retOffset, payloadLength);
-    needToLoadPayload = false;
-    return retArray;
+    return payload;
   }
 
   public boolean isPayloadAvailable() {

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene40/Lucene40PostingsReader.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene40/Lucene40PostingsReader.java
@@ -873,12 +873,7 @@ public int endOffset() {
      *  payload was indexed. */
     @Override
     public BytesRef getPayload() throws IOException {
-      throw new IOException("No payloads exist for this field!");
-    }
-
-    @Override
-    public boolean hasPayload() {
-      return false;
+      return null;
     }
   }
 
@@ -1152,28 +1147,26 @@ public int endOffset() throws IOException {
     @Override
     public BytesRef getPayload() throws IOException {
       if (storePayloads) {
+        if (payloadLength <= 0) {
+          return null;
+        }
         assert lazyProxPointer == -1;
         assert posPendingCount < freq;
-        if (!payloadPending) {
-          throw new IOException("Either no payload exists at this term position or an attempt was made to load it more than once.");
-        }
-        if (payloadLength > payload.bytes.length) {
-          payload.grow(payloadLength);
-        }
+
+        if (payloadPending) {
+          if (payloadLength > payload.bytes.length) {
+            payload.grow(payloadLength);
+          }
 
-        proxIn.readBytes(payload.bytes, 0, payloadLength);
-        payload.length = payloadLength;
-        payloadPending = false;
+          proxIn.readBytes(payload.bytes, 0, payloadLength);
+          payload.length = payloadLength;
+          payloadPending = false;
+        }
 
         return payload;
       } else {
-        throw new IOException("No payloads exist for this field!");
+        return null;
       }
     }
-
-    @Override
-    public boolean hasPayload() {
-      return payloadPending && payloadLength > 0;
-    }
   }
 }
diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene40/Lucene40TermVectorsReader.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene40/Lucene40TermVectorsReader.java
@@ -671,7 +671,6 @@ public void reset(Bits liveDocs, int[] positions, int[] startOffsets, int[] endO
 
     @Override
     public BytesRef getPayload() {
-      // TODO: dumb that we have to duplicate hasPayload
       if (payloadOffsets == null) {
         return null;
       } else {
@@ -687,17 +686,6 @@ public BytesRef getPayload() {
       }
     }
 
-    @Override
-    public boolean hasPayload() {
-      if (payloadOffsets == null) {
-        return false;
-      } else {
-        int off = payloadOffsets[nextPos-1];
-        int end = nextPos == payloadOffsets.length ? payloadBytes.length : payloadOffsets[nextPos];
-        return end - off > 0;
-      }
-    }
-
     @Override
     public int nextPosition() {
       assert (positions != null && nextPos < positions.length) ||

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/memory/DirectPostingsFormat.java b/lucene/core/src/java/org/apache/lucene/codecs/memory/DirectPostingsFormat.java
@@ -348,9 +348,8 @@ public DirectField(SegmentReadState state, String field, Terms termsIn, int minS
                     scratch.add(docsAndPositionsEnum.endOffset());
                   }
                   if (hasPayloads) {
-                    final BytesRef payload;
-                    if (docsAndPositionsEnum.hasPayload()) {
-                      payload = docsAndPositionsEnum.getPayload();
+                    final BytesRef payload = docsAndPositionsEnum.getPayload();
+                    if (payload != null) {
                       scratch.add(payload.length);
                       ros.writeBytes(payload.bytes, payload.offset, payload.length);
                     } else {
@@ -421,9 +420,8 @@ public DirectField(SegmentReadState state, String field, Terms termsIn, int minS
                 for(int pos=0;pos<freq;pos++) {
                   positions[upto][posUpto] = docsAndPositionsEnum.nextPosition();
                   if (hasPayloads) {
-                    if (docsAndPositionsEnum.hasPayload()) {
-                      BytesRef payload = docsAndPositionsEnum.getPayload();
-                      assert payload != null;
+                    BytesRef payload = docsAndPositionsEnum.getPayload();
+                    if (payload != null) {
                       byte[] payloadBytes = new byte[payload.length];
                       System.arraycopy(payload.bytes, payload.offset, payloadBytes, 0, payload.length);
                       payloads[upto][pos] = payloadBytes;
@@ -1806,18 +1804,12 @@ public int advance(int target) {
       return docID;
     }
 
-    @Override
-    public boolean hasPayload() {
-      return payloadLength > 0;
-    }
-
     @Override
     public BytesRef getPayload() {
       if (payloadLength > 0) {
         payload.bytes = payloadBytes;
         payload.offset = lastPayloadOffset;
         payload.length = payloadLength;
-        payloadLength = 0;
         return payload;
       } else {
         return null;
@@ -2010,7 +2002,6 @@ public final static class HighFreqDocsAndPositionsEnum extends DocsAndPositionsE
     private int upto;
     private int docID = -1;
     private int posUpto;
-    private boolean gotPayload;
     private int[] curPositions;
 
     public HighFreqDocsAndPositionsEnum(Bits liveDocs, boolean hasOffsets) {
@@ -2080,7 +2071,6 @@ public int docID() {
     @Override
     public int nextPosition() {
       posUpto += posJump;
-      gotPayload = false;
       return curPositions[posUpto];
     }
 
@@ -2214,21 +2204,22 @@ public int advance(int target) {
       }
     }
 
-    @Override
-    public boolean hasPayload() {
-      return !gotPayload && payloads != null && payloads[upto][posUpto/(hasOffsets ? 3 : 1)] != null;
-    }
-
     private final BytesRef payload = new BytesRef();
 
     @Override
     public BytesRef getPayload() {
-      final byte[] payloadBytes = payloads[upto][posUpto/(hasOffsets ? 3:1)];
-      payload.bytes = payloadBytes;
-      payload.length = payloadBytes.length;
-      payload.offset = 0;
-      gotPayload = true;
-      return payload;
+      if (payloads == null) {
+        return null;
+      } else {
+        final byte[] payloadBytes = payloads[upto][posUpto/(hasOffsets ? 3:1)];
+        if (payloadBytes == null) {
+          return null;
+        }
+        payload.bytes = payloadBytes;
+        payload.length = payloadBytes.length;
+        payload.offset = 0;
+        return payload;
+      }
     }
   }
 }
diff --git a/lucene/core/src/java/org/apache/lucene/codecs/memory/MemoryPostingsFormat.java b/lucene/core/src/java/org/apache/lucene/codecs/memory/MemoryPostingsFormat.java
@@ -446,7 +446,6 @@ private final static class FSTDocsAndPositionsEnum extends DocsAndPositionsEnum
     private int numDocs;
     private int posPending;
     private int payloadLength;
-    private boolean payloadRetrieved;
     final boolean storeOffsets;
     int offsetLength;
     int startOffset;
@@ -484,7 +483,6 @@ public FSTDocsAndPositionsEnum reset(BytesRef bufferIn, Bits liveDocs, int numDo
       payloadLength = 0;
       this.numDocs = numDocs;
       posPending = 0;
-      payloadRetrieved = false;
       startOffset = storeOffsets ? 0 : -1; // always return -1 if no offsets are stored
       offsetLength = 0;
       return this;
@@ -577,10 +575,6 @@ public int nextPosition() {
         payload.offset = in.getPosition();
         in.skipBytes(payloadLength);
         payload.length = payloadLength;
-        // Necessary, in case caller changed the
-        // payload.bytes from prior call:
-        payload.bytes = buffer;
-        payloadRetrieved = false;
       }
 
       //System.out.println("      pos=" + pos + " payload=" + payload + " fp=" + in.getPosition());
@@ -599,13 +593,7 @@ public int endOffset() {
 
     @Override
     public BytesRef getPayload() {
-      payloadRetrieved = true;
-      return payload;
-    }
-
-    @Override
-    public boolean hasPayload() {
-      return !payloadRetrieved && payload.length > 0;
+      return payload.length > 0 ? payload : null;
     }
 
     @Override