You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2016/08/09 02:51:54 UTC

[2/2] lucene-solr:branch_6x: SOLR-9377: Fix jdocs and test to match expected behavior, workaround SOLR-9396

SOLR-9377: Fix jdocs and test to match expected behavior, workaround SOLR-9396

(cherry picked from commit 1164c17e0e1c81fae4aa1103506536f82f70cf3c)


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/e08d656a
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/e08d656a
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/e08d656a

Branch: refs/heads/branch_6x
Commit: e08d656a15a51ef2e38ceb983e786cc37eed834f
Parents: 91d7b53
Author: Chris Hostetter <ho...@apache.org>
Authored: Mon Aug 8 19:48:39 2016 -0700
Committer: Chris Hostetter <ho...@apache.org>
Committed: Mon Aug 8 19:48:57 2016 -0700

----------------------------------------------------------------------
 .../transform/SubQueryAugmenterFactory.java     | 17 ++++
 .../apache/solr/cloud/TestRandomFlRTGCloud.java | 84 +++++++++++++-------
 2 files changed, 74 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e08d656a/solr/core/src/java/org/apache/solr/response/transform/SubQueryAugmenterFactory.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/response/transform/SubQueryAugmenterFactory.java b/solr/core/src/java/org/apache/solr/response/transform/SubQueryAugmenterFactory.java
index 40cc313..cbe6998 100644
--- a/solr/core/src/java/org/apache/solr/response/transform/SubQueryAugmenterFactory.java
+++ b/solr/core/src/java/org/apache/solr/response/transform/SubQueryAugmenterFactory.java
@@ -76,6 +76,15 @@ import org.apache.solr.search.TermsQParserPlugin;
  * its' native parameters like <code>collection, shards</code> for subquery, eg<br>
  *  <code>q=*:*&amp;fl=*,foo:[subquery]&amp;foo.q=cloud&amp;foo.collection=departments</code>
  *
+ * <h3>When used in Real Time Get</h3>
+ * <p>
+ * When used in the context of a Real Time Get, the <i>values</i> from each document that are used 
+ * in the qubquery are the "real time" values (possibly from the transaction log), but the query 
+ * itself is still executed against the currently open searcher.  Note that this means if a 
+ * document is updated but not yet committed, an RTG request for that document that uses 
+ * <code>[subquery]</code> could include the older (committed) version of that document, 
+ * with differnet field values, in the subquery results.
+ * </p>
  */
 public class SubQueryAugmenterFactory extends TransformerFactory{
 
@@ -303,6 +312,14 @@ class SubQueryAugmenter extends DocTransformer {
   public String getName() {
     return name;
   }
+  
+  /**
+   * Returns false -- this transformer does use an IndexSearcher, but it does not (neccessarily) need 
+   * the searcher from the ResultContext of the document being returned.  Instead we use the current 
+   * "live" searcher for the specified core.
+   */
+  @Override
+  public boolean needsSolrIndexSearcher() { return false; }
 
   @Override
   public void transform(SolrDocument doc, int docid, float score) {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e08d656a/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java b/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java
index 484cc89..2e54679 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java
@@ -120,11 +120,7 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
       new GeoTransformerValidator("geo_2_srpt","my_geo_alias"),
       new ExplainValidator(),
       new ExplainValidator("explain_alias"),
-      //
-      // SOLR-9377: SubQueryValidator fails on uncommited docs because not using RT seacher for sub query
-      //
-      // new SubQueryValidator(),
-      //
+      new SubQueryValidator(),
       new NotIncludedValidator("score"),
       new NotIncludedValidator("score","score_alias:score")));
   
@@ -197,8 +193,7 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
     // items should only be added to this list if it's known that they do not work with RTG
     // and a specific Jira for fixing this is listed as a comment
     final List<String> knownBugs = Arrays.asList
-      ( SubQueryValidator.NAME, // SOLR-9377
-        "xml","json", // SOLR-9376
+      ( "xml","json", // SOLR-9376
         "child" // way to complicatd to vet with this test, see SOLR-9379 instead
       );
 
@@ -336,6 +331,9 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
                                        //
                                        "geo_1_srpt", GeoTransformerValidator.getValueForIndexing(random()),
                                        "geo_2_srpt", GeoTransformerValidator.getValueForIndexing(random()),
+                                       // for testing subqueries
+                                       "next_2_ids_ss", String.valueOf(docId + 1),
+                                       "next_2_ids_ss", String.valueOf(docId + 2),
                                        // for testing prefix globbing
                                        "axx_i", random().nextInt(),
                                        "ayy_i", random().nextInt(),
@@ -365,12 +363,8 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
     final Set<FlValidator> validators = new LinkedHashSet<>();
     validators.add(ID_VALIDATOR); // always include id so we can be confident which doc we're looking at
     addRandomFlValidators(random(), validators);
-    FlValidator.addFlParams(validators, params);
+    FlValidator.addParams(validators, params);
 
-    // HACK: [subquery] expects this to be top level params
-    params.add(SubQueryValidator.SUBQ_KEY + ".q",
-               "{!field f=" + SubQueryValidator.SUBQ_FIELD + " v=$row." + SubQueryValidator.SUBQ_FIELD + "}");
-    
     final List<String> idsToRequest = new ArrayList<>(docIds.length);
     final List<SolrInputDocument> docsToExpect = new ArrayList<>(docIds.length);
     for (int docId : docIds) {
@@ -421,7 +415,7 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
     // NOTE: RTG makes no garuntees about the order docs will be returned in when multi requested
     for (SolrDocument actual : docs) {
       try {
-        int actualId = Integer.parseInt(actual.getFirstValue("id").toString());
+        int actualId = assertParseInt("id", actual.getFirstValue("id"));
         final SolrInputDocument expected = knownDocs[actualId];
         assertNotNull("expected null doc but RTG returned: " + actual, expected);
         
@@ -485,10 +479,14 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
    */
   private interface FlValidator {
     
-    /** Given a list of FlValidators, adds one or more fl params that corrispond to the entire set */
-    public static void addFlParams(final Collection<FlValidator> validators, final ModifiableSolrParams params) {
+    /** 
+     * Given a list of FlValidators, adds one or more fl params that corrispond to the entire set, 
+     * as well as any other special case top level params required by the validators.
+     */
+    public static void addParams(final Collection<FlValidator> validators, final ModifiableSolrParams params) {
       final List<String> fls = new ArrayList<>(validators.size());
       for (FlValidator v : validators) {
+        params.add(v.getExtraRequestParams());
         fls.add(v.getFlParam());
       }
       params.add(buildCommaSepParams(random(), "fl", fls));
@@ -519,6 +517,11 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
      */
     public default String getDefaultTransformerFactoryName() { return null; }
     
+    /**
+     * Any special case params that must be added to the request for this validator
+     */
+    public default SolrParams getExtraRequestParams() { return params(); }
+    
     /** 
      * Must return a non null String that can be used in an fl param -- either by itself, 
      * or with other items separated by commas
@@ -747,34 +750,50 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
    * Trivial validator of a SubQueryAugmenter.  
    *
    * This validator ignores 90% of the features/complexity
-   * of SubQueryAugmenter, and instead just focuses on the basics of 
-   * "did we match at least one doc based on a field value of the requested doc?"
+   * of SubQueryAugmenter, and instead just focuses on the basics of:
+   * <ul>
+   *  <li>do a subquery for docs where SUBQ_FIELD contains the id of the top level doc</li>
+   *  <li>verify that any subquery match is expected based on indexing pattern</li>
+   * </ul>
    */
   private static class SubQueryValidator implements FlValidator {
+
+    // HACK to work around SOLR-9396...
+    // 
+    // we're using "id" (and only "id") in the subquery.q as a workarround limitation in
+    // "$rows.foo" parsing -- it only works reliably if "foo" is in fl, so we only use "$rows.id",
+    // which we know is in every request (and is a valid integer)
+    
     public final static String NAME = "subquery";
     public final static String SUBQ_KEY = "subq";
-    public final static String SUBQ_FIELD = "aaa_i";
-    /** always returns true */
-    public boolean requiresRealtimeSearcherReOpen() { return true; }
+    public final static String SUBQ_FIELD = "next_2_ids_i";
     public String getFlParam() { return SUBQ_KEY+":["+NAME+"]"; }
     public Collection<String> assertRTGResults(final Collection<FlValidator> validators,
                                                final SolrInputDocument expected,
                                                final SolrDocument actual) {
-      final Object origVal = expected.getFieldValue(SUBQ_FIELD);
+      final int compVal = assertParseInt("expected id", expected.getFieldValue("id"));
+      
       final Object actualVal = actual.getFieldValue(SUBQ_KEY);
       assertTrue("Expected a doclist: " + actualVal,
                  actualVal instanceof SolrDocumentList);
-      SolrDocumentList subList = (SolrDocumentList) actualVal;
-      assertTrue("sub query should have producted at least one result (this doc)",
-                 1 <= subList.getNumFound());
-      for (SolrDocument subDoc : subList) {
-        assertEquals("orig doc value doesn't match subquery doc value",
-                     origVal, subDoc.getFirstValue(SUBQ_FIELD));
+      assertTrue("should be at most 2 docs in doc list: " + actualVal,
+                 ((SolrDocumentList) actualVal).getNumFound() <= 2);
+      
+      for (SolrDocument subDoc : (SolrDocumentList) actualVal) {
+        final int subDocIdVal = assertParseInt("subquery id", subDoc.getFirstValue("id"));
+        assertTrue("subDocId="+subDocIdVal+" not in valid range for id="+compVal+" (expected "
+                   + (compVal-1) + " or " + (compVal-2) + ")",
+                   ((subDocIdVal < compVal) && ((compVal-2) <= subDocIdVal)));
+        
       }
     
       return Collections.<String>singleton(SUBQ_KEY);
     }
     public String getDefaultTransformerFactoryName() { return NAME; }
+    public SolrParams getExtraRequestParams() {
+      return params(SubQueryValidator.SUBQ_KEY + ".q",
+                    "{!field f=" + SubQueryValidator.SUBQ_FIELD + " v=$row.id}");
+    }
   }
   
   /** Trivial validator of a GeoTransformer */
@@ -945,4 +964,15 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
     }
     return result;
   }
+
+  /** helper method for asserting an object is a non-null String can be parsed as an int */
+  public static int assertParseInt(String msg, Object orig) {
+    assertNotNull(msg + ": is null", orig);
+    assertTrue(msg + ": is not a string: " + orig, orig instanceof String);
+    try {
+      return Integer.parseInt(orig.toString());
+    } catch (NumberFormatException nfe) {
+      throw new AssertionError(msg + ": can't be parsed as a number: " + orig, nfe);
+    }
+  }
 }