You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by softwaredoug <gi...@git.apache.org> on 2016/07/06 00:57:48 UTC

[GitHub] lucene-solr pull request #49: SOLRComparison function queries

GitHub user softwaredoug opened a pull request:

    https://github.com/apache/lucene-solr/pull/49

    SOLRComparison function queries

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/o19s/lucene-solr comparison-function-queries

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/lucene-solr/pull/49.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #49
    
----
commit 220556d434da77e128a0cb3390f383344f5da9ab
Author: Doug Turnbull <so...@gmail.com>
Date:   2016-07-05T20:55:03Z

    Add comparison functions for function queries

commit 57b09cce4b1fb8ae022d7a8037c6fe447c71d7f3
Author: Doug Turnbull <so...@gmail.com>
Date:   2016-07-06T00:43:33Z

    Adds eq operator for numerical equivelance

commit d5493eec5e830583e14881f6dd2c41a83fd8d1e0
Author: Doug Turnbull <so...@gmail.com>
Date:   2016-07-06T00:44:38Z

    Adds tests for comparison function

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69671763
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/CompareNumericFunction.java ---
    @@ -0,0 +1,131 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class CompareNumericFunction extends ValueSource {
    +
    +  public final ValueSource lhs;
    +  public final ValueSource rhs;
    +
    +  public CompareNumericFunction(ValueSource lhs, ValueSource rhs) {
    +    this.lhs = lhs;
    +    this.rhs = rhs;
    +  }
    +
    +  // Perform the comparison, returning true or false
    +  public abstract boolean compareNumeric(double lhs, double rhs);
    --- End diff --
    
    Instead of making this an abstract class, what do you think of making it concrete and take the label and a simple interface named something like LongBinaryPredicate  (a specialization of JDK java.util.BiPredicate), then creating some singleton instances?  I would reduce all these subclasses.  A smaller change that reduces the top-level classes you create here is to make the subclasses in fact inner classes or even anonymous inner classes to initialize some singleton instances of them.  I think I like that a little better in that there is no need to create a new interface (even if it is an inner one).  I welcome your input.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    (note the last few commits based on your comments)
    
    On unifying somewhat with MultiBoolValues, or creating a BiBoolValues, I went down that path a bit David and it seems to complicate things. A couple of notes:
    
    - The bool values functions takes as input other boolfunctions, whereas the comparison value source takes in scalar values.  You can see this in how or, and, xor work: they loop over several boolean value sources and perform and, or, xor etc. We just need to pluck out two scalar values and compare them
    - The name `func` seems descriptive of this general behavior, whereas `compare` is more descriptive of the operation being perfomed by the comparison value source
    
    I think the comparison functions are more readable now as they are, but I'd be curious to get your thoughts.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    I think after addressing the comments I just added, it's probably good to go.  I still don't love the name, especially since it extends BoolFunction it ought to now end with Function.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    Ok this needs another look at @dsmiley as I've addressed a bug where Long comparisons failed 
    
    - Adds test that demonstrates bug comparing Longs
    - Changed compare to take two Comparables instead of doubles
    - Perform an integer comparison when it appears the two input values are effectively integers
    - Otherwise does a floating point comparison
    
    Unfortunately, I think tihs interface can't take arbitrary value sources. I feel like I need to enforce the safest comparison (integer vs floating pt) to avoid bugs.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    Renamed to ComparisonBoolFunction


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    I believe I've done what you requested. There's a lot of value sources that inherit directly from FunctionValues, so without cataloging which ones are best treated as longs and which are best treated as floats, its going to be impossible to always do the safest comparison. The best we can do is test for Long or Int values


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69723224
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/CompareNumericFunction.java ---
    @@ -0,0 +1,131 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class CompareNumericFunction extends ValueSource {
    +
    +  public final ValueSource lhs;
    --- End diff --
    
    whoops, thanks


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69934693
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/ComparisonValueSource.java ---
    @@ -0,0 +1,104 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.queries.function.docvalues.BoolDocValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class ComparisonValueSource extends BoolFunction {
    +
    +  private final ValueSource lhs;
    +  private final ValueSource rhs;
    +  private final String name;
    +
    +  public ComparisonValueSource(ValueSource lhs, ValueSource rhs, String name) {
    +    this.lhs = lhs;
    +    this.rhs = rhs;
    +    this.name = name;
    +  }
    +
    +  // Perform the comparison, returning true or false
    +  public abstract boolean compare(double lhs, double rhs);
    +
    +  // Uniquely identify the operation (ie "gt", "lt" "gte", etc)
    +  public String name() {
    +    return this.name;
    +  }
    +
    +  // string comparison? Probably should be a seperate function
    +  // public abstract boolean compareString(String lhs, String rhs);
    +
    +  public FunctionValues getValues(Map context, LeafReaderContext readerContext) throws IOException {
    +    final FunctionValues lhsVal = this.lhs.getValues(context, readerContext);
    +    final FunctionValues rhsVal = this.rhs.getValues(context, readerContext);
    +    final String compLabel = this.name();
    +
    +    return new BoolDocValues(this) {
    +      @Override
    +      public boolean boolVal(int doc) {
    +        return compare(lhsVal.floatVal(doc), rhsVal.floatVal(doc));
    +      }
    +
    +      @Override
    +      public String toString(int doc) {
    +        return compLabel + "(" + lhsVal.toString(doc) + "," + rhsVal.toString(doc) + ")";
    +      }
    +    };
    +  }
    +
    +  @Override
    +  public boolean equals(Object o) {
    +    if (this.getClass() != o.getClass()) return false;
    +    if (!(o instanceof ComparisonValueSource)) return false;
    --- End diff --
    
    This line is not needed; the classes must be equal in the previous line.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    Dumb question, what's the next steps? Do I need to do anything else here or at the JIRA ticket?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    Renaming is a good idea. I can push that up. ComparisonFunction?
    
    On Thu, Jul 7, 2016 at 12:13 PM David Smiley <no...@github.com>
    wrote:
    
    > I think after addressing the comments I just added, it's probably good to
    > go. I still don't love the name, especially since it extends BoolFunction
    > it ought to now end with Function.
    >
    > \u2014
    > You are receiving this because you authored the thread.
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/lucene-solr/pull/49#issuecomment-231126560>,
    > or mute the thread
    > <https://github.com/notifications/unsubscribe/AAmZRMlf6o2flc_NCh8LYXm1Ha2_S7_Iks5qTSPtgaJpZM4JFqfq>
    > .
    >



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    Ok; this is fine.  It could be optimized later :-)  Did you see Hoss's comments on implementing exists() ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69934986
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/ComparisonValueSource.java ---
    @@ -0,0 +1,104 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.queries.function.docvalues.BoolDocValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class ComparisonValueSource extends BoolFunction {
    +
    +  private final ValueSource lhs;
    +  private final ValueSource rhs;
    +  private final String name;
    +
    +  public ComparisonValueSource(ValueSource lhs, ValueSource rhs, String name) {
    +    this.lhs = lhs;
    +    this.rhs = rhs;
    +    this.name = name;
    +  }
    +
    +  // Perform the comparison, returning true or false
    --- End diff --
    
    Use javadoc method comments, not //


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69724481
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/CompareNumericFunction.java ---
    @@ -0,0 +1,131 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class CompareNumericFunction extends ValueSource {
    --- End diff --
    
    I thought about BoolFunction, but it seemed unused. I certainly could with no change, it would only have documentation value.
    
    Would removing "value" imply this also works on strings? I'm not thoroughly versed here but when I was looking at IfFunction, I intentionally omitted non-numeric comparions (ie string comparisons). Hence the name. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69671790
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/CompareNumericFunction.java ---
    @@ -0,0 +1,131 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class CompareNumericFunction extends ValueSource {
    --- End diff --
    
    Extend Lucene "BoolFunction" instead?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69935281
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/ComparisonValueSource.java ---
    @@ -0,0 +1,104 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.queries.function.docvalues.BoolDocValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class ComparisonValueSource extends BoolFunction {
    +
    +  private final ValueSource lhs;
    +  private final ValueSource rhs;
    +  private final String name;
    +
    +  public ComparisonValueSource(ValueSource lhs, ValueSource rhs, String name) {
    +    this.lhs = lhs;
    +    this.rhs = rhs;
    +    this.name = name;
    +  }
    +
    +  // Perform the comparison, returning true or false
    +  public abstract boolean compare(double lhs, double rhs);
    +
    +  // Uniquely identify the operation (ie "gt", "lt" "gte", etc)
    +  public String name() {
    +    return this.name;
    +  }
    +
    +  // string comparison? Probably should be a seperate function
    +  // public abstract boolean compareString(String lhs, String rhs);
    +
    +  public FunctionValues getValues(Map context, LeafReaderContext readerContext) throws IOException {
    +    final FunctionValues lhsVal = this.lhs.getValues(context, readerContext);
    +    final FunctionValues rhsVal = this.rhs.getValues(context, readerContext);
    +    final String compLabel = this.name();
    +
    +    return new BoolDocValues(this) {
    +      @Override
    +      public boolean boolVal(int doc) {
    +        return compare(lhsVal.floatVal(doc), rhsVal.floatVal(doc));
    --- End diff --
    
    Can you instead pass lhsVal & rhsVal to let the compare() function choose if it wants to call floatVal, doubleVal, or longVal or whatever?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69724555
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/CompareNumericFunction.java ---
    @@ -0,0 +1,131 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class CompareNumericFunction extends ValueSource {
    --- End diff --
    
    oh wait I'm wrong on BoolFunction, it is used. I'll at least switch to that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    Actually yes, I would see that as a bug right now. I bet comparing two
    longs one off would fail a > or < comparison due to loss of precision as a
    double. I'm going to try to create a test that recreates that.
    
    On Fri, Jul 8, 2016 at 2:37 PM David Smiley <no...@github.com>
    wrote:
    
    > Just one thing -- have compare() take the FunctionValue so that a compare
    > impl can choose to call doubleVal vs longVal or whatever else. And the
    > impls you add to Solr can call doubleVal. Someone truly might want to
    > extend this to call something other than doubleVal; the set of values of
    > doubleVal is disjoint from longVal. Or maybe someone has got the data in
    > objectVal for some reason.
    >
    > After that please post a .patch file to JIRA.
    > https://wiki.apache.org/lucene-java/HowToContribute#Creating_a_patch
    > though those instructions should be modified to indicate how to generate a
    > diff from the point the current branch diverged from master.
    >
    > \u2014
    > You are receiving this because you commented.
    >
    >
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/lucene-solr/pull/49#issuecomment-231439077>,
    > or mute the thread
    > <https://github.com/notifications/unsubscribe/AAmZRIJmnJcZ3hLpIOLcti_z7ZyVWCxnks5qTpjYgaJpZM4JFqfq>
    > .
    >



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    I can do that, but in the Solr version I will end up overriding compare, then providing further abstract methods to do numerical comparisons. Otherwise I'm going to just end up with lots of boiler plate repeated in the ValueSourceParser


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug closed the pull request at:

    https://github.com/apache/lucene-solr/pull/49


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69671503
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/CompareNumericFunction.java ---
    @@ -0,0 +1,131 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class CompareNumericFunction extends ValueSource {
    +
    +  public final ValueSource lhs;
    --- End diff --
    
    eek; public?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69671875
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/CompareNumericFunction.java ---
    @@ -0,0 +1,131 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class CompareNumericFunction extends ValueSource {
    +
    +  public final ValueSource lhs;
    +  public final ValueSource rhs;
    +
    +  public CompareNumericFunction(ValueSource lhs, ValueSource rhs) {
    +    this.lhs = lhs;
    +    this.rhs = rhs;
    +  }
    +
    +  // Perform the comparison, returning true or false
    +  public abstract boolean compareNumeric(double lhs, double rhs);
    +
    +  // Uniquely identify the operation (ie "gt", "lt" "gte, etc)
    +  public abstract String getLabel();
    +
    +  // string comparison? Probably should be a seperate function
    +  // public abstract boolean compareString(String lhs, String rhs);
    +
    +  public FunctionValues getValues(Map context, LeafReaderContext readerContext) throws IOException {
    +    final FunctionValues lhsVal = this.lhs.getValues(context, readerContext);
    +    final FunctionValues rhsVal = this.rhs.getValues(context, readerContext);
    +    final String compLabel = this.getLabel();
    +
    +    return new FunctionValues() {
    --- End diff --
    
    Extend Lucene BoolDocValues instead.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    Sorry but I think it's terrible to invoke both doubleVal & longVal potentially twice per doc on the same FunctionValues.  I think what I suggested in earlier feedback is much closer -- let the subclass choose which longVal/doubleVal/whatever to call and to make whatever comparison needed.  On the Solr end... we could always work with the doubles, even though some 'long' values are out of range.  I believe some other FunctionValue impls are implemented similarly as well (?), despite not representing say Long.MAX_VALUE.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69726431
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/CompareNumericFunction.java ---
    @@ -0,0 +1,131 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class CompareNumericFunction extends ValueSource {
    +
    +  public final ValueSource lhs;
    +  public final ValueSource rhs;
    +
    +  public CompareNumericFunction(ValueSource lhs, ValueSource rhs) {
    +    this.lhs = lhs;
    +    this.rhs = rhs;
    +  }
    +
    +  // Perform the comparison, returning true or false
    +  public abstract boolean compareNumeric(double lhs, double rhs);
    --- End diff --
    
    I think I like this idea in general, it would get rid of a lot of cruft. I can pass a lot in as constructor args instead of creating lots of noisy classes.
    
    I'm not sure I want to use LongBinaryPredicate (do you mean [LongPredicate](https://docs.oracle.com/javase/8/docs/api/java/util/function/LongPredicate.html)?. Taking a glance it seems like the value in using it would be in the support for and/or/not operators in a Java-consistent way. It would be nice, though a bit out of scope for this PR, to extend this idea to the other boolean operations so that valuesource's were compatible with this API. 
    
    Let me push up a change that makes more anonymous instances and let me know what you think (I'll model in on how and, or, xor, etc are done)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69672138
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/CompareNumericFunction.java ---
    @@ -0,0 +1,131 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class CompareNumericFunction extends ValueSource {
    --- End diff --
    
    Maybe rename ComparisonPredicateValueSource?  Using "predicate" seems appropriate; Numeric seems redundant with anything ValueSource.  Not sure on Function suffix; there's sadly a mix in Lucene but I think it's better to prefer the subclass's name.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    I think at the Lucene level we should make this widely useful by simply exposing a boolean compare(FunctionValues lhs, FunctionValues rhs).  If we go the way of your latest commit, it would be less useful as some potential users would skip over it to avoid the performance overhead of calling double & longVal, or because they have more interesting requirements and want to call some other method on functionValues.  So lets not tie their hands.
    
    Yes my concern is definitely performance based.  These things are super-sensitive to it as it operates once per matching document, which could be a ton.
    
    What you could perhaps do at the Solr layer is check if both the lhs & rhs extend from LongDocValues and if so then use the Long version... otherwise use the Double version which I think can represent Integer completely.  But it'd be very nice not to do that double instanceof check at every comparison... so perhaps that would lead to actually implementing getValues().


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by softwaredoug <gi...@git.apache.org>.
Github user softwaredoug commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    I just think we have to tread carefully. Comparing two 64 bit timestamps could result in surprising bugs where an event seconds after another isn't greater than the earlier event due to a loss of precision casting to double. So I'd rather enforce the safest possible and most correct comparison and make the interface not that general.
    
    Is your concern performance based? Can we reduce the number of times we call doubleVal/longVal. Sorry I was not aware of the performance implications.
    
    We could also make the values themselves implement Comparable and let them be compared to other function values. But this seems complex and we'd still need to enforce the correct comparison. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr pull request #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/49#discussion_r69765065
  
    --- Diff: lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/CompareNumericFunction.java ---
    @@ -0,0 +1,131 @@
    +package org.apache.lucene.queries.function.valuesource;
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +import java.io.IOException;
    +import java.util.Map;
    +
    +import org.apache.lucene.index.LeafReaderContext;
    +import org.apache.lucene.queries.function.ValueSource;
    +import org.apache.lucene.queries.function.FunctionValues;
    +import org.apache.lucene.search.IndexSearcher;
    +
    +
    +/**
    + * Base class for comparison operators used within if statements
    + * To Solr's if function query a 0 is considered "false", all other values are "true"
    + */
    +public abstract class CompareNumericFunction extends ValueSource {
    +
    +  public final ValueSource lhs;
    +  public final ValueSource rhs;
    +
    +  public CompareNumericFunction(ValueSource lhs, ValueSource rhs) {
    +    this.lhs = lhs;
    +    this.rhs = rhs;
    +  }
    +
    +  // Perform the comparison, returning true or false
    +  public abstract boolean compareNumeric(double lhs, double rhs);
    --- End diff --
    
    > I'm not sure I want to use LongBinaryPredicate (do you mean LongPredicate?.
    
    I am referring to _no_ JDK interfaces other than saying it's _similar_ to BiPredicate, but specialized to the primitive type.  And I definitely didn't mean to suggest anything involving a refactor to Lucene's existing APIs.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    I forgot to use the magic works in the commit to close this.  Please close it Doug.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] lucene-solr issue #49: SOLR-9279 Adds comparison function queries

Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:

    https://github.com/apache/lucene-solr/pull/49
  
    Just one thing -- have compare() take the FunctionValue so that a compare impl can choose to call doubleVal vs longVal or whatever else.  And the impls you add to Solr can call doubleVal.  Someone truly might want to extend this to call something other than doubleVal; the set of values of doubleVal is disjoint from longVal.  Or maybe someone has got the data in objectVal for some reason.
    
    After that please post a .patch file to JIRA.  https://wiki.apache.org/lucene-java/HowToContribute#Creating_a_patch   though those instructions should be modified to indicate how to generate a diff from the point the current branch diverged from master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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