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