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

Hoss Man commented on SOLR-9279:
--------------------------------

I didn't look at the pull request in depth, but in general i applaud the idea 
and the general approach from skimming the issue description and 
CompareNumericFunction.java

3 things that did jump out at me when skimming the patch as a whole: 
* i see edits to ValueSourceParser.java but no edits to QueryEqualityTest.java 
... that gives me 99% confidence that this patch breaks QueryEqualityTest
* I see tests of using these new functions when wrapped in {{if(...)}} but no 
(obvious to me) tests of these new functions being used directly for their 
return value -- ex: {{fl=id,gte(price,0)}} -- and demonstrating what the 
expected result should be
** in this example, i would expect the result type to be a Boolean (ie: {{<bool 
name="gte(price,0)">true</bool>}}
** Since i don't see {{FunctionValues.objectVal}} overridden anywhere in the 
patch, i'm assuming this doesn't work as I expect
* I don't see {{FunctionValues.exists}} overridden anywhere in this patch, 
which IIRC means these functions are always going to "exists=true", which does 
not seem like a good hardcoded behavior for a ValueSource thta wraps other 
ValueSources.
** some thought/javadocs should be given to how exactly these functions should 
behave if/when one or more of the ValueSources they wrap do not exist for a 
given document -- and some tests demonstrating the expected behavior in these 
situations seem crucial.
** see LUCENE-5961, and the core premise expressed in the first comment on that 
issue, which seems just as relevant to me for this issue.
** I would suggest implementing {{exists}} using 
{{MultiFunction.allExists(...)}}, that way callers can decide for themselves 
how it should behave, by wrapping the inner ValueSources in DefFunction, and/or 
by wrapping their CompareNumericFunction in a DefFunction as they see fit.

> Add greater than, less than, etc in Solr function queries
> ---------------------------------------------------------
>
>                 Key: SOLR-9279
>                 URL: https://issues.apache.org/jira/browse/SOLR-9279
>             Project: Solr
>          Issue Type: New Feature
>      Security Level: Public(Default Security Level. Issues are Public) 
>          Components: search
>            Reporter: Doug Turnbull
>             Fix For: master (7.0)
>
>
> If you use the "if" function query, you'll often expect to be able to use 
> greater than/less than functions. For example, you might want to boost books 
> written in the past 7 years. Unfortunately, there's no "greater than" 
> function query that will return non-zero when the lhs > rhs. Instead to get 
> this, you need to create really awkward function queries like I do here 
> (http://opensourceconnections.com/blog/2014/11/26/stepwise-date-boosting-in-solr/):
> if(min(0,sub(ms(mydatefield),sub(ms(NOW),315569259747))),0.8,1)
> The pull request attached to this Jira adds the following function queries
> (https://github.com/apache/lucene-solr/pull/49)
> -gt(lhs, rhs) (returns 1 if lhs > rhs, 0 otherwise)
> -lt(lhs, rhs) (returns 1 if lhs < rhs, 0 otherwise)
> -gte
> -lte
> -eq
> So instead of 
> if(min(0,sub(ms(mydatefield),sub(ms(NOW),315569259747))),0.8,1)
> one could now write
> if(lt(ms(mydatefield),315569259747,0.8,1)
> (if mydatefield < 315569259747 then 0.8 else 1)
> A bit more readable and less puzzling



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