[ 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