You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by GitBox <gi...@apache.org> on 2021/02/21 21:50:40 UTC

[GitHub] [lucenenet] NightOwl888 opened a new issue #431: Query Casting

NightOwl888 opened a new issue #431:
URL: https://github.com/apache/lucenenet/issues/431


   Several APIs return type `Query` with the expectation that the consumer will cast it to the appropriate type of query when utilizing it. Some known examples are:
   
   - `MoreLikeThis.Like()`
   - `Query.Rewrite()`
   - `IndexSearcher.Rewrite()`
   
   
   While there is nothing really broken with this, it is less than ideal. There is at least one way we can make these APIs better for the consumer: by providing generic overloads with a constraint. 
   
   ```c#
   public T Like<T>(TextReader r, string fieldName) where T : Query
   {
       Return (T)Like(r, fieldName);
   }
   ```
   
   This improves things a bit by enforcing correct casting at compile time instead of getting casting exceptions at runtime.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [lucenenet] rclabo commented on issue #431: Query/Filter Casting

Posted by GitBox <gi...@apache.org>.
rclabo commented on issue #431:
URL: https://github.com/apache/lucenenet/issues/431#issuecomment-889089489


   I too think that having these return `Query` is probably fine.  Often a person won't need to cast the result and will just work with it as query. In cases where the caller does need to cast it, I'd rather cast it myself then deal with a generic day in and day out for these methods since they add API complexity that often isn't needed.  Also, using a generic rather than a cast, will still result in runtime casting errors if the generic type supplied isn't correct.  So for example if I use a generic to specify to return type of  `TermQuery` when the method call would otherwise return a `BooleanQuery` as a `Query` object it'll throw at run time not compile time, no different then me doing a cast after the call.  
   
   In summary, most of the time I probably wouldn't need to cast these but if I needed to I'd rather do the cast myself then use a generic to specify the return type so the method can do the cast internally.  


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [lucenenet] NightOwl888 closed issue #431: Query/Filter Casting

Posted by GitBox <gi...@apache.org>.
NightOwl888 closed issue #431:
URL: https://github.com/apache/lucenenet/issues/431


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [lucenenet] jeme edited a comment on issue #431: Query/Filter Casting

Posted by GitBox <gi...@apache.org>.
jeme edited a comment on issue #431:
URL: https://github.com/apache/lucenenet/issues/431#issuecomment-888821631


   After a little investigation, is this even worth pursuing?. I am lacking some examples to illustrate why you would want this.
   
   ### MoreLikeThis.Like()
   
   Actually returns a BooleanQuery, but from a interface perspective, all you should really care about is that it produces a Query object that when used for searching will return documents that a similar to a specific document. (I am not quite sure of the usage of the overload you mentioned here) - Essentially, what type of Query would fulfill that requirement is an implementation detail and shouldn't concern the developer.
   
   ### Query.Rewrite() AND IndexSearcher.Rewrite()
   
   As the later really just delegates to the former, ill take these together.
   
   Rewrite is a way of trying to optimize the Query, again you should not care about what type of Query is returned, just that it performs the same basic function as the original Query but in a better way, what that means should again be an underlying implementation detail a developer shouldn't be concerned with.
   
   I guess the BooleanQuery is the most "straight to the point" example of this, if a BooleanQuery only contains a single clause, there is no need for the BooleanQuery it self, so this is eliminated in a rewrite process by pulling out the single clause, rewriting the Query object in that and then returning that Query object.
   
   ### Filter.GetDocIdSet()
   
   I am not no familiar with this method, so I can't comment here specifically yet, would need more time, but it's again certainly not a straight forward task. But there doesn't seem to be many cases where a specific type is expected in the code, instead in some places where it is type checked it causes a branching instead. So is the abstraction here again not specifically done intentionally?.
   
   
   
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [lucenenet] jeme commented on issue #431: Query/Filter Casting

Posted by GitBox <gi...@apache.org>.
jeme commented on issue #431:
URL: https://github.com/apache/lucenenet/issues/431#issuecomment-888821631


   After a little investigation, is this even worth pursuing?. I am lacking some examples to illustrate why you would want this.
   
   **MoreLikeThis.Like()**
   
   Actually returns a BooleanQuery, but from a interface perspective, all you should really care about is that it produces a Query object that when used for searching will return documents that a similar to a specific document. (I am not quite sure of the usage of the overload you mentioned here) - Essentially, what type of Query would fulfill that requirement is an implementation detail and shouldn't concern the developer.
   
   **Query.Rewrite() AND IndexSearcher.Rewrite()**
   
   As the later really just delegates to the former, ill take these together.
   
   Rewrite is a way of trying to optimize the Query, again you should not care about what type of Query is returned, just that it performs the same basic function as the original Query but in a better way, what that means should again be an underlying implementation detail a developer shouldn't be concerned with.
   
   I guess the BooleanQuery is the most "straight to the point" example of this, if a BooleanQuery only contains a single clause, there is no need for the BooleanQuery it self, so this is eliminated in a rewrite process by pulling out the single clause, rewriting the Query object in that and then returning that Query object.
   
   **Filter.GetDocIdSet()**
   
   I am not no familiar with this method, so I can't comment here specifically yet, would need more time, but it's again certainly not a straight forward task. But there doesn't seem to be many cases where a specific type is expected in the code, instead in some places where it is type checked it causes a branching instead. So is the abstraction here again not specifically done intentionally?.
   
   
   
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [lucenenet] NightOwl888 commented on issue #431: Query/Filter Casting

Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on issue #431:
URL: https://github.com/apache/lucenenet/issues/431#issuecomment-961188824


   I confess I was using the tests as an indicator of what a user might do, which may be an invalid assumption to make.
   
   ### MoreLikeThis.Like()
   
   I was specifically looking at the test here, but I can see that a user may not care about looking at the individual clauses under normal usage.
   
   ```c#
               BooleanQuery query = (BooleanQuery)mlt.Like(new StringReader("lucene release"), "text");
               IList<BooleanClause> clauses = query.Clauses;
   
               assertEquals("Expected " + originalValues.Count + " clauses.", originalValues.Count, clauses.Count);
   
               foreach (BooleanClause clause in clauses)
               {
                   TermQuery tq = (TermQuery)clause.Query;
                   float? termBoost = originalValues[tq.Term.Text];
                   assertNotNull("Expected term " + tq.Term.Text, termBoost);
   
                   float totalBoost = (float) (termBoost * boostFactor);
                   assertEquals("Expected boost of " + totalBoost + " for term '"
                       + tq.Term.Text + "' got " + tq.Boost, totalBoost, tq.Boost, 0.0001);
               }
   ```
   
   ### Query.Rewrite() AND IndexSearcher.Rewrite()
   
   This appears to be the same type of situation. The cast was only used so the test could check `SpanQuery.ExtractTerms()`. Most of the tests don't have any casting, and are probably more representative of what a user will do.
   
   ```c#
           [Test]
           public virtual void TestRewrite0()
           {
               SpanQuery q = new FieldMaskingSpanQuery(new SpanTermQuery(new Term("last", "sally")), "first");
               q.Boost = 8.7654321f;
               SpanQuery qr = (SpanQuery)searcher.Rewrite(q);
   
               QueryUtils.CheckEqual(q, qr);
   
               ISet<Term> terms = new JCG.HashSet<Term>();
               qr.ExtractTerms(terms);
               Assert.AreEqual(1, terms.Count);
           }
   ```
   
   ### Filter.GetDocIdSet()
   
   This also seems to be similar. Although it is a bit more difficult with this one, as this might be something a consumer actually does. That being said, casting is not required in over 90% of cases it is used.
   
   ```c#
           private Document GetParentDoc(IndexReader reader, Filter parents, int childDocID)
           {
               IList<AtomicReaderContext> leaves = reader.Leaves;
               int subIndex = ReaderUtil.SubIndex(childDocID, leaves);
               AtomicReaderContext leaf = leaves[subIndex];
               FixedBitSet bits = (FixedBitSet)parents.GetDocIdSet(leaf, null);
               return leaf.AtomicReader.Document(bits.NextSetBit(childDocID - leaf.DocBase));
           }
   ```
   
   So, I have to concur this is probably not worth the effort.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org