From 086ec7c4931e7cec1c6ae31040830fde6a90f3dd Mon Sep 17 00:00:00 2001
From: Yonik Seeley <yonik@apache.org>
Date: Wed, 16 Oct 2013 20:45:48 +0000
Subject: [PATCH] SOLR-5330: make copy of term bytes before calling next

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1532900 13f79535-47bb-0310-9956-ffa450edef68
---
 solr/CHANGES.txt                              |  4 +++
 .../PerSegmentSingleValuedFaceting.java       |  8 ++---
 .../org/apache/solr/TestRandomFaceting.java   |  9 ++++--
 .../java/org/apache/solr/SolrTestCaseJ4.java  | 30 +++++++++++++++++--
 4 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 4f03204442c..7274086e14b 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -189,6 +189,10 @@ Bug Fixes
 * SOLR-5349: CloudSolrServer - ZK timeout arguments passed to ZkStateReader are flipped.
   (Ricardo Merizalde via shalin)
 
+* SOLR-5330: facet.method=fcs on single values fields could sometimes result
+  in incorrect facet labels. (Michael Froh, yonik)
+
+
 Other Changes
 ----------------------
 
diff --git a/solr/core/src/java/org/apache/solr/request/PerSegmentSingleValuedFaceting.java b/solr/core/src/java/org/apache/solr/request/PerSegmentSingleValuedFaceting.java
index 0c1948a27d0..7573a1b6d09 100644
--- a/solr/core/src/java/org/apache/solr/request/PerSegmentSingleValuedFaceting.java
+++ b/solr/core/src/java/org/apache/solr/request/PerSegmentSingleValuedFaceting.java
@@ -174,10 +174,10 @@ class PerSegmentSingleValuedFaceting {
     while (queue.size() > 0) {
       SegFacet seg = queue.top();
 
-      // make a shallow copy
-      val.bytes = seg.tempBR.bytes;
-      val.offset = seg.tempBR.offset;
-      val.length = seg.tempBR.length;
+      // we will normally end up advancing the term enum for this segment
+      // while still using "val", so we need to make a copy since the BytesRef
+      // may be shared across calls.
+      val.copyBytes(seg.tempBR);
 
       int count = 0;
 
diff --git a/solr/core/src/test/org/apache/solr/TestRandomFaceting.java b/solr/core/src/test/org/apache/solr/TestRandomFaceting.java
index 8009d7b8384..2f06b470870 100644
--- a/solr/core/src/test/org/apache/solr/TestRandomFaceting.java
+++ b/solr/core/src/test/org/apache/solr/TestRandomFaceting.java
@@ -59,8 +59,11 @@ public class TestRandomFaceting extends SolrTestCaseJ4 {
     types.add(new FldType("small_f",ONE_ONE, new FVal(-4,5)));
     types.add(new FldType("small_d",ONE_ONE, new FVal(-4,5)));
     types.add(new FldType("foo_i",ZERO_ONE, new IRange(-2,indexSize)));
-    types.add(new FldType("small_s",ZERO_ONE, new SVal('a',(char)('c'+indexSize/3),1,1)));
-    types.add(new FldType("small2_s",ZERO_ONE, new SVal('a',(char)('c'+indexSize/3),1,1)));
+    types.add(new FldType("rare_s1",new IValsPercent(95,0,5,1), new SVal('a','b',1,5)));
+    types.add(new FldType("str_s1",ZERO_ONE, new SVal('a','z',1,2)));
+    types.add(new FldType("long_s1",ZERO_ONE, new SVal('a','b',1,5)));
+    types.add(new FldType("small_s1",ZERO_ONE, new SVal('a',(char)('c'+indexSize/3),1,1)));
+    types.add(new FldType("small2_s1",ZERO_ONE, new SVal('a',(char)('c'+indexSize/3),1,1)));
     types.add(new FldType("small2_ss",ZERO_TWO, new SVal('a',(char)('c'+indexSize/3),1,1)));
     types.add(new FldType("small3_ss",new IRange(0,25), new SVal('A','z',1,1)));
     types.add(new FldType("small_i",ZERO_ONE, new IRange(-2,5+indexSize/3)));
@@ -70,7 +73,7 @@ public class TestRandomFaceting extends SolrTestCaseJ4 {
 
     types.add(new FldType("missing_i",new IRange(0,0), new IRange(0,100)));
     types.add(new FldType("missing_is",new IRange(0,0), new IRange(0,100)));
-    types.add(new FldType("missing_s",new IRange(0,0), new SVal('a','b',1,1)));
+    types.add(new FldType("missing_s1",new IRange(0,0), new SVal('a','b',1,1)));
     types.add(new FldType("missing_ss",new IRange(0,0), new SVal('a','b',1,1)));
 
     // TODO: doubles, multi-floats, ints with precisionStep>0, booleans
diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
index 38bf9ff2921..3e4c0736cf4 100644
--- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
+++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
@@ -1162,6 +1162,32 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
     }
   }
 
+  public static class IValsPercent extends IVals {
+    final int[] percentAndValue;
+    public IValsPercent(int... percentAndValue) {
+      this.percentAndValue = percentAndValue;
+    }
+
+    @Override
+    public int getInt() {
+      int r = between(0,99);
+      int cumulative = 0;
+      for (int i=0; i<percentAndValue.length; i+=2) {
+        cumulative += percentAndValue[i];
+        if (r < cumulative) {
+          return percentAndValue[i+1];
+        }
+      }
+
+      return percentAndValue[percentAndValue.length-1];
+    }
+
+    @Override
+    public Comparable get() {
+      return getInt();
+    }
+  }
+
   public static class FVal extends Vals {
     final float min;
     final float max;
@@ -1280,14 +1306,14 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
 
   protected class FldType {
     public String fname;
-    public IRange numValues;
+    public IVals numValues;
     public Vals vals;
 
     public FldType(String fname, Vals vals) {
       this(fname, ZERO_ONE, vals);
     }
 
-    public FldType(String fname, IRange numValues, Vals vals) {
+    public FldType(String fname, IVals numValues, Vals vals) {
       this.fname = fname;
       this.numValues = numValues;
       this.vals = vals;