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

    https://github.com/apache/lucene-solr/pull/455#discussion_r228026284
  
    --- Diff: solr/core/src/java/org/apache/solr/update/AddUpdateCommand.java 
---
    @@ -262,6 +263,11 @@ private void flattenAnonymous(List<SolrInputDocument> 
unwrappedDocs, SolrInputDo
         flattenAnonymous(unwrappedDocs, currentDoc, false);
       }
     
    +  public String getRouteFieldVal() {
    --- End diff --
    
    I'm skeptical of this method.  It's name seems innocent enough looking at 
the code here.  But then also consider some collections have a "router.field" 
and this method is named in such a way that one would think this method returns 
that field's value... yet it does not.  Some callers put this into a variable 
named "id" or similar.  Given that, I propose you remove it but incorporate the 
logic into getHashableId which seems the proper place for it.  It _is_ the 
hashable Id... the hashable ID of a nested doc is it's root.
    
    But... I do also wonder if we need this at all.  Somewhere Solr already has 
code that looks at \_route\_ and acts on that if present.  Perhaps the code 
path for an atomic update isn't doing this yet but should do it?  Then we 
wouldn't need this change to AddUpdateCommand.


---

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

Reply via email to