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

Adrien Grand commented on LUCENE-5325:
--------------------------------------

I am +1 in general, I think it's great that we are getting a cleaner values 
source API! I left some comments about the current patch below:

 - Should we rename advanceTo to advanceExact to be more consistent with the 
doc values API? These methods seem to have the same contract.
 - It seems to be that {{fromIntField}} could just delegate to 
{{fromLongField}} all the time?
 - Can we use {{LongUnaryOperator.identity()}} rather than {{(v) -> v}}
 - {{// TODO make setScorer throw IOException?}} +1
 - {{LongValuesSource.fromDoubleField}} looks a bit trappy to me since it 
implicitly casts. Should we instead expect callers to call 
{{DoubleValuesSource.fromDoubleField}} and then do the casting explicitly using 
{{toLongValuesSource}}?
 - Maybe we should not have the {{DoubleValues getValues(LeafReaderContext)}} 
method (without a Scorer). I'm wondering it might be better to require callers 
to pass null themselves when they know scores are not needed.

About the way we have access to scores, I'm wondering that things might be a 
bit cleaner if we avoided mixing the values source and Scorer APIs by replacing 
{{LongValues getValues(LeafReaderContext ctx, Scorer scorer)}} with 
{{LongValues getValues(LeafReaderContext ctx, DoubleValues scores)}} in 
addition to something like a {{DoubleValues fromScorer(Scorer scorer)}} method 
on {{DoubleValuesSource}}?

In general I think we should also better document the contract of these APIs, 
like what the expectations are for the scorer/scores when {{needsScores}} 
returns {{false}} and what {{toDoubleValues}} and {{toLongValues}} do since not 
all doubles can be represented as a long and vice-versa.

> Move ValueSource and FunctionValues under core/
> -----------------------------------------------
>
>                 Key: LUCENE-5325
>                 URL: https://issues.apache.org/jira/browse/LUCENE-5325
>             Project: Lucene - Core
>          Issue Type: Improvement
>          Components: core/search
>            Reporter: Shai Erera
>         Attachments: LUCENE-5325.patch, LUCENE-5325.patch, LUCENE-5325.patch, 
> LUCENE-5325.patch, LUCENE-5325.patch
>
>
> Spinoff from LUCENE-5298: ValueSource and FunctionValues are abstract APIs 
> which exist under the queries/ module. That causes any module which wants to 
> depend on these APIs (but not necessarily on any of their actual 
> implementations!), to depend on the queries/ module. If we move these APIs 
> under core/, we can eliminate these dependencies and add some mock impls for 
> testing purposes.
> Quoting Robert from LUCENE-5298:
> {quote}
> we should eliminate the suggest/ dependencies on expressions and queries, the 
> expressions/ on queries, the grouping/ dependency on queries, the spatial/ 
> dependency on queries, its a mess.
> {quote}
> To add to that list, facet/ should not depend on queries too.



--
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