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/04/22 22:44:25 UTC

[GitHub] [lucenenet] rclabo opened a new pull request #475: Improved Grouping Search API and added 8 more grouping tests

rclabo opened a new pull request #475:
URL: https://github.com/apache/lucenenet/pull/475


   


-- 
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] NightOwl888 commented on a change in pull request #475: Improved Grouping Search API and added 8 more grouping tests

Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on a change in pull request #475:
URL: https://github.com/apache/lucenenet/pull/475#discussion_r625190254



##########
File path: src/Lucene.Net.Grouping/GroupingSearch.cs
##########
@@ -161,86 +196,261 @@ public virtual ITopGroups<object> Search(IndexSearcher searcher, Filter filter,
         }
 
         /// <summary>
-        /// Executes a grouped search. Both the first pass and second pass are executed on the specified searcher.
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+        {
+            return GroupByField<BytesRef>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+
+        /// <summary>
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
         /// </summary>
-        /// <typeparam name="TGroupValue">The expected return type of the search.</typeparam>
         /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
         /// <param name="filter">The filter to execute with the grouping</param>
         /// <param name="query">The query to execute with the grouping</param>
         /// <param name="groupOffset">The group offset</param>
         /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
         /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
         /// <exception cref="IOException">If any I/O related errors occur</exception>
-        public virtual ITopGroups<TGroupValue> Search<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
         {
-            if (groupField != null || groupFunction != null)
+            if (groupField is null)
             {
-                return GroupByFieldOrFunction<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("Must use constructor to set pass a non null value for groupField.");
             }
-            else if (groupEndDocs != null)
+
+            if (groupFunction != null)
             {
-                return GroupByDocBlock<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("The groupFunction must be null.");
             }
-            else
+            return GroupByField<BytesRef>(searcher, filter, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue
+        {
+            return GroupByFunction<TMutableValue>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="filter">The filter to execute with the grouping</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue
+        {
+            if (groupFunction is null)
             {
                 throw IllegalStateException.Create("Either groupField, groupFunction or groupEndDocs must be set."); // This can't happen...
             }
+
+            if (groupField != null)
+            {
+                throw new Exception("The valueSource must be null.");
+            }
+            return GroupByFunction<TMutableValue>(searcher, filter, query, groupOffset, groupLimit);
         }
 
-        protected virtual ITopGroups<TGroupValue> GroupByFieldOrFunction<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        //LUCENENET: Method replaced by two methods GroupByField and GroupByFunction so that a generic type constraint
+        //           can be added to the GroupByFunction case.
+        //protected virtual ITopGroups GroupByFieldOrFunction(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        //{
+        //}
+
+
+        //LUCENENET Specific. One of two methods that replace GroupByFieldOrFunction. Used support SearchByField.
+        //          This method is essentually a Field specific version of the GroupByFieldOrFunction.
+        protected virtual ITopGroups<TGroupValue> GroupByField<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
         {
             int topN = groupOffset + groupLimit;
             IAbstractFirstPassGroupingCollector<TGroupValue> firstPassCollector;
             IAbstractAllGroupsCollector<TGroupValue> allGroupsCollector;
             AbstractAllGroupHeadsCollector allGroupHeadsCollector;
-            if (groupFunction != null)
+
+            if (groupField is null)
+            {
+                throw IllegalStateException.Create("groupField must be set via the constructor.");
+            }
+
+            firstPassCollector = (IAbstractFirstPassGroupingCollector<TGroupValue>)new TermFirstPassGroupingCollector(groupField, groupSort, topN);
+            if (allGroups)
+            {
+                allGroupsCollector = (IAbstractAllGroupsCollector<TGroupValue>)new TermAllGroupsCollector(groupField, initialSize);
+            }
+            else
+            {
+                allGroupsCollector = null;
+            }
+            if (allGroupHeads)
             {
-                firstPassCollector = (IAbstractFirstPassGroupingCollector<TGroupValue>)new FunctionFirstPassGroupingCollector(groupFunction, valueSourceContext, groupSort, topN);
+                allGroupHeadsCollector = TermAllGroupHeadsCollector.Create(groupField, sortWithinGroup, initialSize);
+            }
+            else
+            {
+                allGroupHeadsCollector = null;
+            }
+
+            ICollector firstRound;
+            if (allGroupHeads || allGroups)
+            {
+                List<ICollector> collectors = new List<ICollector>();
+                collectors.Add(firstPassCollector);
+
                 if (allGroups)
                 {
-                    allGroupsCollector = (IAbstractAllGroupsCollector<TGroupValue>)new FunctionAllGroupsCollector(groupFunction, valueSourceContext);
-                }
-                else
-                {
-                    allGroupsCollector = null;
+                    collectors.Add(allGroupsCollector);
                 }
                 if (allGroupHeads)
                 {
-                    allGroupHeadsCollector = new FunctionAllGroupHeadsCollector(groupFunction, valueSourceContext, sortWithinGroup);
-                }
-                else
-                {
-                    allGroupHeadsCollector = null;
+                    collectors.Add(allGroupHeadsCollector);
                 }
+                firstRound = MultiCollector.Wrap(collectors.ToArray(/* new Collector[collectors.size()] */));
             }
             else
             {
-                firstPassCollector = (IAbstractFirstPassGroupingCollector<TGroupValue>)new TermFirstPassGroupingCollector(groupField, groupSort, topN);
-                if (allGroups)
-                {
-                    allGroupsCollector = (IAbstractAllGroupsCollector<TGroupValue>)new TermAllGroupsCollector(groupField, initialSize);
-                }
-                else
-                {
-                    allGroupsCollector = null;
-                }
-                if (allGroupHeads)
+                firstRound = firstPassCollector;
+            }
+
+            CachingCollector cachedCollector = null;
+            if (maxCacheRAMMB != null || maxDocsToCache != null)
+            {
+                if (maxCacheRAMMB != null)
                 {
-                    allGroupHeadsCollector = TermAllGroupHeadsCollector.Create(groupField, sortWithinGroup, initialSize);
+                    cachedCollector = CachingCollector.Create(firstRound, cacheScores, maxCacheRAMMB.Value);
                 }
                 else
                 {
-                    allGroupHeadsCollector = null;
+                    cachedCollector = CachingCollector.Create(firstRound, cacheScores, maxDocsToCache.Value);
                 }
+                searcher.Search(query, filter, cachedCollector);
+            }
+            else
+            {
+                searcher.Search(query, filter, firstRound);
+            }
+
+            if (allGroups)
+            {
+                matchingGroups = (ICollection)allGroupsCollector.Groups;

Review comment:
       Yes, that sounds good.




-- 
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] NightOwl888 commented on a change in pull request #475: Improved Grouping Search API and added 8 more grouping tests

Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on a change in pull request #475:
URL: https://github.com/apache/lucenenet/pull/475#discussion_r624783456



##########
File path: src/Lucene.Net.Grouping/GroupingSearch.cs
##########
@@ -161,86 +196,261 @@ public virtual ITopGroups<object> Search(IndexSearcher searcher, Filter filter,
         }
 
         /// <summary>
-        /// Executes a grouped search. Both the first pass and second pass are executed on the specified searcher.
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+        {
+            return GroupByField<BytesRef>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+
+        /// <summary>
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
         /// </summary>
-        /// <typeparam name="TGroupValue">The expected return type of the search.</typeparam>
         /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
         /// <param name="filter">The filter to execute with the grouping</param>
         /// <param name="query">The query to execute with the grouping</param>
         /// <param name="groupOffset">The group offset</param>
         /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
         /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
         /// <exception cref="IOException">If any I/O related errors occur</exception>
-        public virtual ITopGroups<TGroupValue> Search<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
         {
-            if (groupField != null || groupFunction != null)
+            if (groupField is null)
             {
-                return GroupByFieldOrFunction<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("Must use constructor to set pass a non null value for groupField.");
             }
-            else if (groupEndDocs != null)
+
+            if (groupFunction != null)
             {
-                return GroupByDocBlock<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("The groupFunction must be null.");
             }
-            else
+            return GroupByField<BytesRef>(searcher, filter, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue
+        {
+            return GroupByFunction<TMutableValue>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="filter">The filter to execute with the grouping</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue

Review comment:
       Seems like the generic type parameter should still be named `TGroupValue` to make it clear that is what it is grouping on. The user can still see that the type must be `MutableValue` or a subclass via Intellisense.

##########
File path: src/Lucene.Net.Grouping/GroupingSearch.cs
##########
@@ -161,86 +196,261 @@ public virtual ITopGroups<object> Search(IndexSearcher searcher, Filter filter,
         }
 
         /// <summary>
-        /// Executes a grouped search. Both the first pass and second pass are executed on the specified searcher.
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+        {
+            return GroupByField<BytesRef>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+
+        /// <summary>
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
         /// </summary>
-        /// <typeparam name="TGroupValue">The expected return type of the search.</typeparam>
         /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
         /// <param name="filter">The filter to execute with the grouping</param>
         /// <param name="query">The query to execute with the grouping</param>
         /// <param name="groupOffset">The group offset</param>
         /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
         /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
         /// <exception cref="IOException">If any I/O related errors occur</exception>
-        public virtual ITopGroups<TGroupValue> Search<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
         {
-            if (groupField != null || groupFunction != null)
+            if (groupField is null)
             {
-                return GroupByFieldOrFunction<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("Must use constructor to set pass a non null value for groupField.");
             }
-            else if (groupEndDocs != null)
+
+            if (groupFunction != null)
             {
-                return GroupByDocBlock<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("The groupFunction must be null.");
             }
-            else
+            return GroupByField<BytesRef>(searcher, filter, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue
+        {
+            return GroupByFunction<TMutableValue>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="filter">The filter to execute with the grouping</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue
+        {
+            if (groupFunction is null)
             {
                 throw IllegalStateException.Create("Either groupField, groupFunction or groupEndDocs must be set."); // This can't happen...
             }
+
+            if (groupField != null)
+            {
+                throw new Exception("The valueSource must be null.");
+            }
+            return GroupByFunction<TMutableValue>(searcher, filter, query, groupOffset, groupLimit);
         }
 
-        protected virtual ITopGroups<TGroupValue> GroupByFieldOrFunction<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        //LUCENENET: Method replaced by two methods GroupByField and GroupByFunction so that a generic type constraint
+        //           can be added to the GroupByFunction case.
+        //protected virtual ITopGroups GroupByFieldOrFunction(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        //{
+        //}
+
+
+        //LUCENENET Specific. One of two methods that replace GroupByFieldOrFunction. Used support SearchByField.
+        //          This method is essentually a Field specific version of the GroupByFieldOrFunction.
+        protected virtual ITopGroups<TGroupValue> GroupByField<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
         {
             int topN = groupOffset + groupLimit;
             IAbstractFirstPassGroupingCollector<TGroupValue> firstPassCollector;
             IAbstractAllGroupsCollector<TGroupValue> allGroupsCollector;
             AbstractAllGroupHeadsCollector allGroupHeadsCollector;
-            if (groupFunction != null)
+
+            if (groupField is null)
+            {
+                throw IllegalStateException.Create("groupField must be set via the constructor.");
+            }
+
+            firstPassCollector = (IAbstractFirstPassGroupingCollector<TGroupValue>)new TermFirstPassGroupingCollector(groupField, groupSort, topN);
+            if (allGroups)
+            {
+                allGroupsCollector = (IAbstractAllGroupsCollector<TGroupValue>)new TermAllGroupsCollector(groupField, initialSize);
+            }
+            else
+            {
+                allGroupsCollector = null;
+            }
+            if (allGroupHeads)
             {
-                firstPassCollector = (IAbstractFirstPassGroupingCollector<TGroupValue>)new FunctionFirstPassGroupingCollector(groupFunction, valueSourceContext, groupSort, topN);
+                allGroupHeadsCollector = TermAllGroupHeadsCollector.Create(groupField, sortWithinGroup, initialSize);
+            }
+            else
+            {
+                allGroupHeadsCollector = null;
+            }
+
+            ICollector firstRound;
+            if (allGroupHeads || allGroups)
+            {
+                List<ICollector> collectors = new List<ICollector>();
+                collectors.Add(firstPassCollector);
+
                 if (allGroups)
                 {
-                    allGroupsCollector = (IAbstractAllGroupsCollector<TGroupValue>)new FunctionAllGroupsCollector(groupFunction, valueSourceContext);
-                }
-                else
-                {
-                    allGroupsCollector = null;
+                    collectors.Add(allGroupsCollector);
                 }
                 if (allGroupHeads)
                 {
-                    allGroupHeadsCollector = new FunctionAllGroupHeadsCollector(groupFunction, valueSourceContext, sortWithinGroup);
-                }
-                else
-                {
-                    allGroupHeadsCollector = null;
+                    collectors.Add(allGroupHeadsCollector);
                 }
+                firstRound = MultiCollector.Wrap(collectors.ToArray(/* new Collector[collectors.size()] */));
             }
             else
             {
-                firstPassCollector = (IAbstractFirstPassGroupingCollector<TGroupValue>)new TermFirstPassGroupingCollector(groupField, groupSort, topN);
-                if (allGroups)
-                {
-                    allGroupsCollector = (IAbstractAllGroupsCollector<TGroupValue>)new TermAllGroupsCollector(groupField, initialSize);
-                }
-                else
-                {
-                    allGroupsCollector = null;
-                }
-                if (allGroupHeads)
+                firstRound = firstPassCollector;
+            }
+
+            CachingCollector cachedCollector = null;
+            if (maxCacheRAMMB != null || maxDocsToCache != null)
+            {
+                if (maxCacheRAMMB != null)
                 {
-                    allGroupHeadsCollector = TermAllGroupHeadsCollector.Create(groupField, sortWithinGroup, initialSize);
+                    cachedCollector = CachingCollector.Create(firstRound, cacheScores, maxCacheRAMMB.Value);
                 }
                 else
                 {
-                    allGroupHeadsCollector = null;
+                    cachedCollector = CachingCollector.Create(firstRound, cacheScores, maxDocsToCache.Value);
                 }
+                searcher.Search(query, filter, cachedCollector);
+            }
+            else
+            {
+                searcher.Search(query, filter, firstRound);
+            }
+
+            if (allGroups)
+            {
+                matchingGroups = (ICollection)allGroupsCollector.Groups;

Review comment:
       Is there a way we can make this cast use a generic collection?
   
   I am hoping to eliminate the use of non-generic collections in Lucene.NET. There are 2 reasons for this:
   
   1. Custom user collections probably won't bother implementing non-generic interfaces, so casts like this aren't guaranteed to succeed.
   2. J2N doesn't currently support comparing/equating non-generic collections.




-- 
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 a change in pull request #475: Improved Grouping Search API and added 8 more grouping tests

Posted by GitBox <gi...@apache.org>.
rclabo commented on a change in pull request #475:
URL: https://github.com/apache/lucenenet/pull/475#discussion_r625193569



##########
File path: src/Lucene.Net.Grouping/GroupingSearch.cs
##########
@@ -161,86 +196,261 @@ public virtual ITopGroups<object> Search(IndexSearcher searcher, Filter filter,
         }
 
         /// <summary>
-        /// Executes a grouped search. Both the first pass and second pass are executed on the specified searcher.
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+        {
+            return GroupByField<BytesRef>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+
+        /// <summary>
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
         /// </summary>
-        /// <typeparam name="TGroupValue">The expected return type of the search.</typeparam>
         /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
         /// <param name="filter">The filter to execute with the grouping</param>
         /// <param name="query">The query to execute with the grouping</param>
         /// <param name="groupOffset">The group offset</param>
         /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
         /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
         /// <exception cref="IOException">If any I/O related errors occur</exception>
-        public virtual ITopGroups<TGroupValue> Search<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
         {
-            if (groupField != null || groupFunction != null)
+            if (groupField is null)
             {
-                return GroupByFieldOrFunction<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("Must use constructor to set pass a non null value for groupField.");
             }
-            else if (groupEndDocs != null)
+
+            if (groupFunction != null)
             {
-                return GroupByDocBlock<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("The groupFunction must be null.");
             }
-            else
+            return GroupByField<BytesRef>(searcher, filter, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue
+        {
+            return GroupByFunction<TMutableValue>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="filter">The filter to execute with the grouping</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue

Review comment:
       I yield.  Yep, will clean it up.




-- 
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] NightOwl888 commented on a change in pull request #475: Improved Grouping Search API and added 8 more grouping tests

Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on a change in pull request #475:
URL: https://github.com/apache/lucenenet/pull/475#discussion_r625189817



##########
File path: src/Lucene.Net.Grouping/GroupingSearch.cs
##########
@@ -161,86 +196,261 @@ public virtual ITopGroups<object> Search(IndexSearcher searcher, Filter filter,
         }
 
         /// <summary>
-        /// Executes a grouped search. Both the first pass and second pass are executed on the specified searcher.
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+        {
+            return GroupByField<BytesRef>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+
+        /// <summary>
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
         /// </summary>
-        /// <typeparam name="TGroupValue">The expected return type of the search.</typeparam>
         /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
         /// <param name="filter">The filter to execute with the grouping</param>
         /// <param name="query">The query to execute with the grouping</param>
         /// <param name="groupOffset">The group offset</param>
         /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
         /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
         /// <exception cref="IOException">If any I/O related errors occur</exception>
-        public virtual ITopGroups<TGroupValue> Search<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
         {
-            if (groupField != null || groupFunction != null)
+            if (groupField is null)
             {
-                return GroupByFieldOrFunction<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("Must use constructor to set pass a non null value for groupField.");
             }
-            else if (groupEndDocs != null)
+
+            if (groupFunction != null)
             {
-                return GroupByDocBlock<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("The groupFunction must be null.");
             }
-            else
+            return GroupByField<BytesRef>(searcher, filter, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue
+        {
+            return GroupByFunction<TMutableValue>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="filter">The filter to execute with the grouping</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue

Review comment:
       What type it is is clear, however, its purpose (that this is the type it is grouping on) is not. While we could make this clear by adding a [`<typeparam>`](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/xmldoc/typeparam) value in the documentation, I still think that naming the generic parameter after its purpose rather than its type is important.




-- 
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 a change in pull request #475: Improved Grouping Search API and added 8 more grouping tests

Posted by GitBox <gi...@apache.org>.
rclabo commented on a change in pull request #475:
URL: https://github.com/apache/lucenenet/pull/475#discussion_r625143263



##########
File path: src/Lucene.Net.Grouping/GroupingSearch.cs
##########
@@ -161,86 +196,261 @@ public virtual ITopGroups<object> Search(IndexSearcher searcher, Filter filter,
         }
 
         /// <summary>
-        /// Executes a grouped search. Both the first pass and second pass are executed on the specified searcher.
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+        {
+            return GroupByField<BytesRef>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+
+        /// <summary>
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
         /// </summary>
-        /// <typeparam name="TGroupValue">The expected return type of the search.</typeparam>
         /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
         /// <param name="filter">The filter to execute with the grouping</param>
         /// <param name="query">The query to execute with the grouping</param>
         /// <param name="groupOffset">The group offset</param>
         /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
         /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
         /// <exception cref="IOException">If any I/O related errors occur</exception>
-        public virtual ITopGroups<TGroupValue> Search<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
         {
-            if (groupField != null || groupFunction != null)
+            if (groupField is null)
             {
-                return GroupByFieldOrFunction<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("Must use constructor to set pass a non null value for groupField.");
             }
-            else if (groupEndDocs != null)
+
+            if (groupFunction != null)
             {
-                return GroupByDocBlock<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("The groupFunction must be null.");
             }
-            else
+            return GroupByField<BytesRef>(searcher, filter, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue
+        {
+            return GroupByFunction<TMutableValue>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="filter">The filter to execute with the grouping</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue
+        {
+            if (groupFunction is null)
             {
                 throw IllegalStateException.Create("Either groupField, groupFunction or groupEndDocs must be set."); // This can't happen...
             }
+
+            if (groupField != null)
+            {
+                throw new Exception("The valueSource must be null.");
+            }
+            return GroupByFunction<TMutableValue>(searcher, filter, query, groupOffset, groupLimit);
         }
 
-        protected virtual ITopGroups<TGroupValue> GroupByFieldOrFunction<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        //LUCENENET: Method replaced by two methods GroupByField and GroupByFunction so that a generic type constraint
+        //           can be added to the GroupByFunction case.
+        //protected virtual ITopGroups GroupByFieldOrFunction(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        //{
+        //}
+
+
+        //LUCENENET Specific. One of two methods that replace GroupByFieldOrFunction. Used support SearchByField.
+        //          This method is essentually a Field specific version of the GroupByFieldOrFunction.
+        protected virtual ITopGroups<TGroupValue> GroupByField<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
         {
             int topN = groupOffset + groupLimit;
             IAbstractFirstPassGroupingCollector<TGroupValue> firstPassCollector;
             IAbstractAllGroupsCollector<TGroupValue> allGroupsCollector;
             AbstractAllGroupHeadsCollector allGroupHeadsCollector;
-            if (groupFunction != null)
+
+            if (groupField is null)
+            {
+                throw IllegalStateException.Create("groupField must be set via the constructor.");
+            }
+
+            firstPassCollector = (IAbstractFirstPassGroupingCollector<TGroupValue>)new TermFirstPassGroupingCollector(groupField, groupSort, topN);
+            if (allGroups)
+            {
+                allGroupsCollector = (IAbstractAllGroupsCollector<TGroupValue>)new TermAllGroupsCollector(groupField, initialSize);
+            }
+            else
+            {
+                allGroupsCollector = null;
+            }
+            if (allGroupHeads)
             {
-                firstPassCollector = (IAbstractFirstPassGroupingCollector<TGroupValue>)new FunctionFirstPassGroupingCollector(groupFunction, valueSourceContext, groupSort, topN);
+                allGroupHeadsCollector = TermAllGroupHeadsCollector.Create(groupField, sortWithinGroup, initialSize);
+            }
+            else
+            {
+                allGroupHeadsCollector = null;
+            }
+
+            ICollector firstRound;
+            if (allGroupHeads || allGroups)
+            {
+                List<ICollector> collectors = new List<ICollector>();
+                collectors.Add(firstPassCollector);
+
                 if (allGroups)
                 {
-                    allGroupsCollector = (IAbstractAllGroupsCollector<TGroupValue>)new FunctionAllGroupsCollector(groupFunction, valueSourceContext);
-                }
-                else
-                {
-                    allGroupsCollector = null;
+                    collectors.Add(allGroupsCollector);
                 }
                 if (allGroupHeads)
                 {
-                    allGroupHeadsCollector = new FunctionAllGroupHeadsCollector(groupFunction, valueSourceContext, sortWithinGroup);
-                }
-                else
-                {
-                    allGroupHeadsCollector = null;
+                    collectors.Add(allGroupHeadsCollector);
                 }
+                firstRound = MultiCollector.Wrap(collectors.ToArray(/* new Collector[collectors.size()] */));
             }
             else
             {
-                firstPassCollector = (IAbstractFirstPassGroupingCollector<TGroupValue>)new TermFirstPassGroupingCollector(groupField, groupSort, topN);
-                if (allGroups)
-                {
-                    allGroupsCollector = (IAbstractAllGroupsCollector<TGroupValue>)new TermAllGroupsCollector(groupField, initialSize);
-                }
-                else
-                {
-                    allGroupsCollector = null;
-                }
-                if (allGroupHeads)
+                firstRound = firstPassCollector;
+            }
+
+            CachingCollector cachedCollector = null;
+            if (maxCacheRAMMB != null || maxDocsToCache != null)
+            {
+                if (maxCacheRAMMB != null)
                 {
-                    allGroupHeadsCollector = TermAllGroupHeadsCollector.Create(groupField, sortWithinGroup, initialSize);
+                    cachedCollector = CachingCollector.Create(firstRound, cacheScores, maxCacheRAMMB.Value);
                 }
                 else
                 {
-                    allGroupHeadsCollector = null;
+                    cachedCollector = CachingCollector.Create(firstRound, cacheScores, maxDocsToCache.Value);
                 }
+                searcher.Search(query, filter, cachedCollector);
+            }
+            else
+            {
+                searcher.Search(query, filter, firstRound);
+            }
+
+            if (allGroups)
+            {
+                matchingGroups = (ICollection)allGroupsCollector.Groups;

Review comment:
       I think so.  I could also probably convert the  matchingGroups member var from a `ICollection` to a `ICollection<TGroupValue>` if that's preferred for the reasons you cited. It would mean that [ICollection GetAllMatchingGroups()](https://github.com/apache/lucenenet/blob/master/src/Lucene.Net.Grouping/GroupingSearch.cs#L707) would need to be removed but it appears to only be used by one test which can easily be upgraded to use the generic version of the same method.  So I can make those changes if that sounds good. 




-- 
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] NightOwl888 merged pull request #475: Improved Grouping Search API and added 8 more grouping tests

Posted by GitBox <gi...@apache.org>.
NightOwl888 merged pull request #475:
URL: https://github.com/apache/lucenenet/pull/475


   


-- 
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 a change in pull request #475: Improved Grouping Search API and added 8 more grouping tests

Posted by GitBox <gi...@apache.org>.
rclabo commented on a change in pull request #475:
URL: https://github.com/apache/lucenenet/pull/475#discussion_r625131864



##########
File path: src/Lucene.Net.Grouping/GroupingSearch.cs
##########
@@ -161,86 +196,261 @@ public virtual ITopGroups<object> Search(IndexSearcher searcher, Filter filter,
         }
 
         /// <summary>
-        /// Executes a grouped search. Both the first pass and second pass are executed on the specified searcher.
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+        {
+            return GroupByField<BytesRef>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+
+        /// <summary>
+        /// Executes a grouped search base on the field specified via the constructor. Both the first pass
+        /// and second pass are executed on the specified searcher.
         /// </summary>
-        /// <typeparam name="TGroupValue">The expected return type of the search.</typeparam>
         /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
         /// <param name="filter">The filter to execute with the grouping</param>
         /// <param name="query">The query to execute with the grouping</param>
         /// <param name="groupOffset">The group offset</param>
         /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
         /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
         /// <exception cref="IOException">If any I/O related errors occur</exception>
-        public virtual ITopGroups<TGroupValue> Search<TGroupValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+        // LUCENENET additional method signature. Makes searching by field easier due to concrete return type
+        public virtual ITopGroups<BytesRef> SearchByField(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
         {
-            if (groupField != null || groupFunction != null)
+            if (groupField is null)
             {
-                return GroupByFieldOrFunction<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("Must use constructor to set pass a non null value for groupField.");
             }
-            else if (groupEndDocs != null)
+
+            if (groupFunction != null)
             {
-                return GroupByDocBlock<TGroupValue>(searcher, filter, query, groupOffset, groupLimit);
+                throw IllegalStateException.Create("The groupFunction must be null.");
             }
-            else
+            return GroupByField<BytesRef>(searcher, filter, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue
+        {
+            return GroupByFunction<TMutableValue>(searcher, null, query, groupOffset, groupLimit);
+        }
+
+        /// <summary>
+        /// Executes a grouped search base on the function specified by a <see cref="ValueSource"/> passed via the constructor.
+        /// Both the first pass and second pass are executed on the specified searcher.
+        /// </summary>
+        /// <param name="searcher">The <see cref="IndexSearcher"/> instance to execute the grouped search on.</param>
+        /// <param name="filter">The filter to execute with the grouping</param>
+        /// <param name="query">The query to execute with the grouping</param>
+        /// <param name="groupOffset">The group offset</param>
+        /// <param name="groupLimit">The number of groups to return from the specified group offset</param>
+        /// <returns>the grouped result as a <see cref="ITopGroups{Object}"/> instance</returns>
+        /// <exception cref="IOException">If any I/O related errors occur</exception>
+        // LUCENENET additional method signature. Makes searching by function easier due to ability to specify type of MutableValue returned.
+        public virtual ITopGroups<TMutableValue> SearchByFunction<TMutableValue>(IndexSearcher searcher, Filter filter, Query query, int groupOffset, int groupLimit)
+            where TMutableValue : MutableValue

Review comment:
       I think TMutableValue is more clear since the T passed must inherit from MutableValue.  But if you feel strongly about it I can change it.




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