[ 
https://issues.apache.org/jira/browse/SOLR-13851?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16953893#comment-16953893
 ] 

David Wayne Smiley commented on SOLR-13851:
-------------------------------------------

I agree with your analysis.  That "assert" should be an IllegalStateException.  
And the implementation of getFirstMatch is fine; just update it's documentation.

I dislike the names of these similar methods.... "getFirstMatch" and "lookupId" 
are basically doing the same thing yet the method names are so different.  And 
as you note, we always use the ID.  Perhaps these might be renamed to 
lookupDocIdByUniqueKey and lookupDocIdAsPairByUniqueKey and have them both 
simply take a BytesRef?  It's okay to me that my proposed names are long-ish.

> SolrIndexSearcher.getFirstMatch trips assertion if multiple matches
> -------------------------------------------------------------------
>
>                 Key: SOLR-13851
>                 URL: https://issues.apache.org/jira/browse/SOLR-13851
>             Project: Solr
>          Issue Type: Bug
>      Security Level: Public(Default Security Level. Issues are Public) 
>            Reporter: Chris M. Hostetter
>            Priority: Major
>
> the documentation for {{SolrIndexSearcher.getFirstMatch}} says...
> {quote}
> Returns the first document number containing the term <code>t</code> Returns 
> -1 if no document was found. This method is primarily intended for clients 
> that want to fetch documents using a unique identifier."
> @return the first document number containing the term
> {quote}
> But SOLR-12366 refactored {{SolrIndexSearcher.getFirstMatch}} to eliminate 
> it's previous implementation and replace it with a call to (a refactored 
> version of) {{SolrIndexSearcher.lookupId}} -- but the code in {{lookupId}} 
> was always designed *explicitly* for dealing with a uniqueKey field, and has 
> an assertion that once it finds a match _there will be no other matches in 
> the index_
> This means that even though {{getFirstMatch}} is _intended_ for fields that 
> are unique between documents, i it's used on a field that is not unique, it 
> can trip an assertion.
> At a minimum we need to either "fix" {{getFirstMatch}} to behave as 
> documented, or fix it's documetation.
> Given that the current behavior has now been in place since Solr 7.4, and 
> given that all existing uses in "core" solr code are for looking up docs by 
> uniqueKey, it's probably best to simply fix the documentation, but we should 
> also consider replacing hte assertion with an IllegalStateException, or 
> SolrException -- anything not dependent on having assertions enabled -- to 
> prevent silent bugs.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

Reply via email to