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

Robert Muir commented on LUCENE-6808:
-------------------------------------

I agree with Mike, I don't think we should create extra garbage here, the 
documentation says:
{code}
  /**
   * Return the actual value in the slot.
   *
   * @param slot the value
   * @return value in this slot
   */
  public abstract T value(int slot);
{code}

It says the value "in the slot" which isn't trappy at all. Its not a copy of 
the value or something else.

Also docs at the start of the class reiterate what Mike said, that its only 
called at the end of the search:

{code}
   <li> {@link #value} Return the sort value stored in
        the specified slot.  This is only called at the end
        of the search, in order to populate {@link
        FieldDoc#fields} when returning the top results.
{code}


> Term(Ord)ValComparator.value return BytesRefs that are re-used internally
> -------------------------------------------------------------------------
>
>                 Key: LUCENE-6808
>                 URL: https://issues.apache.org/jira/browse/LUCENE-6808
>             Project: Lucene - Core
>          Issue Type: Bug
>            Reporter: Hoss Man
>         Attachments: LUCENE-6808.patch, LUCENE-6808.patch
>
>
> While working on SOLR-6168, which involves some non-trivial usage of 
> {{FieldComparator}}, I discovered some weird bugs anytime I was using 
> TermOrdValComparator.  I ultimately tracked this down to the fact that the 
> {{BytesRef}} instances returned by {{TermOrdValComparator.value(int slot)}} 
> are backed by {{BytesRefBuilder}} instances that the Comparator hangs on to 
> and re-uses -- so the values a caller gets back from 
> {{FieldComparator.value(slot)}} might be changed out from under it before it 
> has a chance to use that value in something like 
> {{FieldComparator.compareValues(first,second)}}.
> The general approach when dealing with BytesRef instances (as i understand 
> it) is that the caller is responsible for making a copy if it wants to hang 
> on to it -- but in this case that would violate the generic API of 
> FieldComparator -- callers would have to pay attention to when a 
> {{FieldComparator}} is a 
> {{FieldComparator<BytesRef>}} and do casting to copy the BytesRef.
> It seems like the right solution is for {{TermOrdValComparator.value(slot)}} 
> (and {{TermValComparator.value(slot)}} which has a similar BytesRef usage) to 
> return {{BytesRef.deepCopyOf(values\[slot\])}}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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

Reply via email to