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

Reply via email to