From 24d6b7846995542c5ccbb4ddcdaa844f78555205 Mon Sep 17 00:00:00 2001
From: Shalin Shekhar Mangar <shalin@apache.org>
Date: Thu, 7 Jul 2016 13:22:15 +0530
Subject: [PATCH] SOLR-8858: SolrIndexSearcher#doc() completely ignores field
 filters unless lazy field loading is enabled.

This closes #47.
---
 solr/CHANGES.txt                              |  3 +++
 .../apache/solr/search/SolrIndexSearcher.java | 20 ++++++++++++++-----
 .../transform/TestSubQueryTransformer.java    | 17 ++++++++--------
 3 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 97bf7f56b6c..95fa7962629 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -128,6 +128,9 @@ Bug Fixes
   is introduced (defaults to true) to send version ranges instead of individual versions for peer sync.
   (Pushkar Raste, shalin)
 
+* SOLR-8858: SolrIndexSearcher#doc() completely ignores field filters unless lazy field loading is enabled.
+  (Caleb Rackliffe, David Smiley, shalin)
+
 Optimizations
 ----------------------
 
diff --git a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
index 213f7583540..cc719f0be37 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
@@ -766,12 +766,22 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
     }
 
     final DirectoryReader reader = getIndexReader();
-    if (!enableLazyFieldLoading || fields == null) {
-      d = reader.document(i);
+    if (fields != null) {
+      if (enableLazyFieldLoading) {
+        final SetNonLazyFieldSelector visitor = new SetNonLazyFieldSelector(fields, reader, i);
+        reader.document(i, visitor);
+        d = visitor.doc;
+      } else if (documentCache == null) {
+        d = reader.document(i, fields);
+      } else {
+        // we do not pass the fields in this case because that would return an incomplete document which would
+        // be eventually cached. The alternative would be to read the stored fields twice; once with the fields
+        // and then without for caching leading to a performance hit
+        // see SOLR-8858 for related discussion
+        d = reader.document(i);
+      }
     } else {
-      final SetNonLazyFieldSelector visitor = new SetNonLazyFieldSelector(fields, reader, i);
-      reader.document(i, visitor);
-      d = visitor.doc;
+      d = reader.document(i);
     }
 
     if (documentCache != null) {
diff --git a/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformer.java b/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformer.java
index bd9ff393dcb..e9af1cff704 100644
--- a/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformer.java
+++ b/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformer.java
@@ -127,7 +127,7 @@ public class TestSubQueryTransformer extends SolrTestCaseJ4 {
      //System.out.println("p "+peopleMultiplier+" d "+deptMultiplier);
     assertQ("subq1.fl is limited to single field",
         req("q","name_s:(john nancy)", "indent","true",
-            "fl","name_s_dv,depts:[subquery]", 
+            "fl","dept_ss_dv,name_s_dv,depts:[subquery]", 
             "rows","" + (2 * peopleMultiplier),
             "depts.q","{!term f=dept_id_s v=$row.dept_ss_dv}", 
             "depts.fl","text_t",
@@ -150,8 +150,8 @@ public class TestSubQueryTransformer extends SolrTestCaseJ4 {
     }
   
   final String[] johnAndNancyParams = new String[]{"q","name_s:(john nancy)", "indent","true",
-      "fl","name_s_dv,depts:[subquery]",
-      "fl","depts_i:[subquery]",
+      "fl","dept_ss_dv,name_s_dv,depts:[subquery]",
+      "fl","dept_i_dv,depts_i:[subquery]",
       "rows","" + (2 * peopleMultiplier),
       "depts.q","{!term f=dept_id_s v=$row.dept_ss_dv}", 
       "depts.fl","text_t",
@@ -225,7 +225,7 @@ public class TestSubQueryTransformer extends SolrTestCaseJ4 {
     }
     
     String[] john = new String[]{"q","name_s:john", "indent","true",
-        "fl","name_s_dv,depts:[subquery]",
+        "fl","dept_ss_dv,name_s_dv,depts:[subquery]",
         "rows","" + (2 * peopleMultiplier),
         "depts.q","+{!term f=dept_id_s v=$row.dept_ss_dv}^=0 _val_:id_i", 
         "depts.fl","id",
@@ -277,10 +277,10 @@ public class TestSubQueryTransformer extends SolrTestCaseJ4 {
     assertQ("dave works at both dept with other folks",
   //  System.out.println(h.query( 
         req(new String[]{"q","name_s:dave", "indent","true",
-        "fl","name_s_dv,subq1:[subquery]", 
+        "fl","dept_ss_dv,name_s_dv,subq1:[subquery]", 
         "rows","" + peopleMultiplier,
         "subq1.q","{!terms f=dept_id_s v=$row.dept_ss_dv}", 
-        "subq1.fl","text_t,dept_id_s_dv,neighbours:[subquery]",
+        "subq1.fl","dept_id_i_dv,text_t,dept_id_s_dv,neighbours:[subquery]",
         "subq1.indent","true",
         "subq1.rows",""+(deptMultiplier*2),
         "subq1.neighbours.q",//flipping via numbers 
@@ -459,9 +459,8 @@ public class TestSubQueryTransformer extends SolrTestCaseJ4 {
     
     assertQ("dave works at both, whether we set a  default separator or both",
         req(new String[]{"q","name_s:dave", "indent","true",
-        "fl",(random().nextBoolean() ? "name_s_dv" : "*")+ //"dept_ss_dv,
-                    ",subq1:[subquery "
-                +((random1.nextBoolean() ? "" : "separator=,"))+"]", 
+        "fl", (random().nextBoolean() ? "name_s_dv,dept_ss_dv" : "*") + 
+              ",subq1:[subquery " +((random1.nextBoolean() ? "" : "separator=,"))+"]", 
         "rows","" + peopleMultiplier,
         "subq1.q","{!terms f=dept_id_s v=$row.dept_ss_dv "+((random1.nextBoolean() ? "" : "separator=,"))+"}", 
         "subq1.fl","text_t",