Sorry Marc, I had missed your message. This is what I meant indeed.
On Fri, Dec 30, 2022 at 4:36 PM Greg Miller <gsmil...@gmail.com> wrote:
>
> OK, great! Thanks Marc. I plan on merging the PR today.
>
> Cheers,
> -Greg
>
> On Thu, Dec 29, 2022 at 3:23 PM Marc D'Mello <marcd2...@gmail.com> wrote:
>>
>> Hi Greg,
>>
>> I'm also OK merging as is since this is a new feature and doesn't affect any
>> of the current functionality. I also think there are no glaring issues with
>> the API in its current state. However, I do think that merging the range and
>> rangeonrange functionality makes sense and I like Adrien's suggestion of
>> providing factory methods. I think if we merge in its current state we
>> should create a new issue to refactor the range and rangeonrange faceting
>> package into one and follow the RangeFieldQuery model more closely.
>>
>> On Thu, Dec 29, 2022 at 2:58 PM Greg Miller <gsmil...@gmail.com> wrote:
>>>
>>> Hey Marc-
>>>
>>> I don't want to speak for Adrien as he might have something different in
>>> mind, but I think that's more-or-less the idea. I'm not sure the factory
>>> methods belong on the LongRange/DoubleRange classes, or if separate classes
>>> should be created for this purpose (which is more how I thought of it)?
>>>
>>> To do this cleanly though, I'd really like us to try to consolidate all the
>>> "range related" faceting functionality into one java package and
>>> consolidate the API a bit. As part of this, I think we can be a little
>>> smarter about not duplicating the "range" classes themselves.
>>>
>>> All this said, given that I think your "range on range" faceting PR is
>>> ready to be merged as it currently exists, and has been through a number of
>>> iteration already, I'm OK if we want to merge that work as it stands and
>>> follow up with revisiting the API/naming/etc. as a future project. What do
>>> you think?
>>>
>>> Cheers,
>>> -Greg
>>>
>>> On Tue, Dec 13, 2022 at 7:23 PM Marc D'Mello <marcd2...@gmail.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> I'm a bit unsure about what is being suggested. Is the idea to rename
>>>> range#LongRange and rangeonrange#LongRange to LongFieldFacets and
>>>> LongRangeFacets respectively and stick the static getters in there? In
>>>> that case, I also think that the idea makes a lot of sense and that it
>>>> would match our current range query API much better.
>>>>
>>>> In addition, looking at document#LongRange, there are queries like
>>>> newContainsQuery() and newWithinQuery() that we can probably mimic to
>>>> avoid exposing RangeFieldQuery.QueryType to the user.
>>>>
>>>> On Tue, Dec 13, 2022 at 5:04 PM Greg Miller <gsmil...@gmail.com> wrote:
>>>>>
>>>>> Thanks for the suggestion Adrien. I like this idea! Marc- what do you
>>>>> think?
>>>>>
>>>>> We might need to rework the package structure under the facets module to
>>>>> make this clean, but that might not be a terrible thing anyway. The
>>>>> existing sub-packages will make it challenging to get the visibility
>>>>> right. I think it would be ideal to flatten the package so we can reduce
>>>>> visibility of the class definitions and only expose the factory methods.
>>>>>
>>>>> Cheers,
>>>>> -Greg
>>>>>
>>>>> On Tue, Dec 13, 2022 at 01:18 Adrien Grand <jpou...@gmail.com> wrote:
>>>>>>
>>>>>> I wonder if the facets actually require a different name, since they
>>>>>> look to me like a generalization of range facets for range fields,
>>>>>> while we previously only supported range facets on numeric fields. We
>>>>>> could keep calling them range facets?
>>>>>>
>>>>>> Maybe we could use the same model we used for queries by not exposing
>>>>>> query classes to users and providing factory methods, e.g. we could
>>>>>> have something like:
>>>>>>
>>>>>> public class LongFieldFacets {
>>>>>>
>>>>>> public static Facets getRangeFacetCounts(String field,
>>>>>> FacetsCollector hits, LongRange... ranges) {
>>>>>> return new LongRangeFacetCounts(...);
>>>>>> }
>>>>>>
>>>>>> }
>>>>>>
>>>>>> public class LongRangeFacets {
>>>>>>
>>>>>> // same function name
>>>>>> public static Facets getRangeFacetCounts(String field,
>>>>>> FacetsCollector hits, RangeFieldQuery.QueryType queryType,
>>>>>> LongRange... ranges) {
>>>>>> return new LongRangeOnRangeFacetCounts(...);
>>>>>> }
>>>>>>
>>>>>> }
>>>>>>
>>>>>> We'd still need to give a name for these classes, but the name would
>>>>>> be less important since these class names would be only for ourselves.
>>>>>> Users would never see them and refer to this new functionality as
>>>>>> range facets on range fields?
>>>>>>
>>>>>> On Mon, Dec 12, 2022 at 10:11 PM Gus Heck <gus.h...@gmail.com> wrote:
>>>>>> >
>>>>>> > In that case, maybe "Range Logic Faceting" ?
>>>>>> >
>>>>>> > Relation seems too broad and too overloaded elsewhere, makes me think
>>>>>> > of RDBMS, related-ness, joins and such via word associations.
>>>>>> >
>>>>>> > On Mon, Dec 12, 2022 at 3:27 PM Greg Miller <gsmil...@gmail.com> wrote:
>>>>>> >>
>>>>>> >> Thank for the suggestion! I like the descriptiveness of it. My only
>>>>>> >> hesitation is that is supports more than range intersection based on
>>>>>> >> the provided QueryType instance (e.g., within, contains). I _imagine_
>>>>>> >> that intersection will be most common, but I don’t really know of
>>>>>> >> course. I thought about generalizing your suggestion to something
>>>>>> >> like “Range Relation Faceting,” but fear that would be confusing.
>>>>>> >>
>>>>>> >> Thanks again!
>>>>>> >>
>>>>>> >> Cheers,
>>>>>> >> -Greg
>>>>>> >>
>>>>>> >> On Mon, Dec 12, 2022 at 10:19 Gus Heck <gus.h...@gmail.com> wrote:
>>>>>> >>>
>>>>>> >>> Maybe "Range Intersect Faceting"?
>>>>>> >>>
>>>>>> >>> On Mon, Dec 12, 2022 at 1:11 PM Greg Miller <gsmil...@gmail.com>
>>>>>> >>> wrote:
>>>>>> >>>>
>>>>>> >>>> Folks-
>>>>>> >>>>
>>>>>> >>>> Naming is hard! (But you all know that already).
>>>>>> >>>>
>>>>>> >>>> Marc D'Mello and I have been working on a new faceting
>>>>>> >>>> implementation that's meant to complement Lucene's existing
>>>>>> >>>> range-relation queries (e.g., LongRange#newIntersectsQuery,
>>>>>> >>>> DoubleRange#newContainsQuery,
>>>>>> >>>> LongRangeDocValuesField#newSlowIntersectsQuery, etc.). Well, I
>>>>>> >>>> should say Marc is working on the change and I'm just providing
>>>>>> >>>> nit-picky feedback on his PR, which is here:
>>>>>> >>>> https://github.com/apache/lucene/pull/11901. The general idea of
>>>>>> >>>> this feature is to allow users to get facet counts for these sorts
>>>>>> >>>> of range-relation filters before they're applied. For example, if a
>>>>>> >>>> user is indexing ranges with their documents, they may have a set
>>>>>> >>>> of query-ranges they want to facet on, based on some range
>>>>>> >>>> relationship (e.g., intersection, contains, etc.).
>>>>>> >>>>
>>>>>> >>>> As a concrete example, imagine that documents contain a price range
>>>>>> >>>> (maybe a document represents some e-commerce product but the price
>>>>>> >>>> varies based on some configuration options), and a user wants to
>>>>>> >>>> build a price range filter that applies filtering based on
>>>>>> >>>> whether-or-not the two ranges intersect (i.e.,
>>>>>> >>>> DoubleRange#newIntersectsQuery to apply a price range filter). This
>>>>>> >>>> user wants faceting capabilities over the different price ranges
>>>>>> >>>> they want to make available, so they need a way to facet over a
>>>>>> >>>> list of provided query-ranges, based on the "intersect"
>>>>>> >>>> relationship with the doc-encoded ranges. That's what Marc's
>>>>>> >>>> "RangeOnRange" faceting is trying to accomplish.
>>>>>> >>>>
>>>>>> >>>> In my opinion, the PR is really close to being ready (thanks again
>>>>>> >>>> Marc!), but I'm wondering if we can come up with a more descriptive
>>>>>> >>>> name. As it currently stands, the feature is termed "RangeOnRange
>>>>>> >>>> Faceting," which feels just a bit wonky to me. That said, I can't
>>>>>> >>>> really come up with anything better.
>>>>>> >>>>
>>>>>> >>>> ** Does anyone have suggestions on a better name? **
>>>>>> >>>>
>>>>>> >>>> Any / all suggestions appreciated! (And of course, any other input
>>>>>> >>>> on the PR is welcome if anyone is interested).
>>>>>> >>>>
>>>>>> >>>> Cheers,
>>>>>> >>>> -Greg
>>>>>> >>>
>>>>>> >>>
>>>>>> >>>
>>>>>> >>> --
>>>>>> >>> http://www.needhamsoftware.com (work)
>>>>>> >>> http://www.the111shift.com (play)
>>>>>> >
>>>>>> >
>>>>>> >
>>>>>> > --
>>>>>> > http://www.needhamsoftware.com (work)
>>>>>> > http://www.the111shift.com (play)
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Adrien
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
>>>>>> For additional commands, e-mail: dev-h...@lucene.apache.org
>>>>>>
--
Adrien
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
