Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/416#discussion_r205465327
  
    --- Diff: 
solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java
 ---
    @@ -70,36 +86,62 @@ public DocTransformer create(String field, SolrParams 
params, SolrQueryRequest r
         }
     
         String parentFilter = params.get( "parentFilter" );
    -    if( parentFilter == null ) {
    -      throw new SolrException( ErrorCode.BAD_REQUEST, "Parent filter 
should be sent as parentFilter=filterCondition" );
    +    BitSetProducer parentsFilter = null;
    +    boolean buildHierarchy = params.getBool("hierarchy", false);
    +    if( parentFilter == null) {
    +      if(!buildHierarchy) {
    +        throw new SolrException( ErrorCode.BAD_REQUEST, "Parent filter 
should be sent as parentFilter=filterCondition" );
    +      }
    +      parentsFilter = new QueryBitSetProducer(rootFilter);
    +    } else {
    +      try {
    +        Query parentFilterQuery = QParser.getParser(parentFilter, 
req).getQuery();
    +        //TODO shouldn't we try to use the Solr filter cache, and then 
ideally implement
    +        //  BitSetProducer over that?
    +        // DocSet parentDocSet = 
req.getSearcher().getDocSet(parentFilterQuery);
    +        // then return BitSetProducer with custom BitSet impl accessing 
the docSet
    +        parentsFilter = new QueryBitSetProducer(parentFilterQuery);
    +      } catch (SyntaxError syntaxError) {
    +        throw new SolrException( ErrorCode.BAD_REQUEST, "Failed to create 
correct parent filter query" );
    +      }
         }
     
         String childFilter = params.get( "childFilter" );
         int limit = params.getInt( "limit", 10 );
     
    -    BitSetProducer parentsFilter = null;
    -    try {
    -      Query parentFilterQuery = QParser.getParser( parentFilter, 
req).getQuery();
    -      //TODO shouldn't we try to use the Solr filter cache, and then 
ideally implement
    -      //  BitSetProducer over that?
    -      // DocSet parentDocSet = 
req.getSearcher().getDocSet(parentFilterQuery);
    -      // then return BitSetProducer with custom BitSet impl accessing the 
docSet
    -      parentsFilter = new QueryBitSetProducer(parentFilterQuery);
    -    } catch (SyntaxError syntaxError) {
    -      throw new SolrException( ErrorCode.BAD_REQUEST, "Failed to create 
correct parent filter query" );
    -    }
    -
         Query childFilterQuery = null;
         if(childFilter != null) {
    --- End diff --
    
    The code flow from here to the end of the method looks very awkward to me.  
I think the top "if" condition should test for buildHierarchy that we the 
nested and non-nested cases are clearly separated.  Do you think that would be 
clear?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to