gsmiller commented on PR #11780: URL: https://github.com/apache/lucene/pull/11780#issuecomment-1464091101
Sorry @stefanvodita, just now coming back to this after a break. OK, so current state-of-the-world: Right now, users have to instantiate reader state instances for each field they're using for SSDV faceting so they can provide those to SSDFacets. These reader state instances must be re-created when the index is refreshed, and users have to manage this on their own. Creating these state instances is expensive, so users need a way to keep track of them, but also refresh them when necessary. To be fair though, users have all the capabilities they need to do efficient SSDV faceting, it's just a lot to keep track of for them (in my opinion anyway). So I think the real question here is if we can do something that makes users' lives easier w.r.t. keeping track of these states and refreshing them when necessary. I think Mike's point-of-view is that maybe this isn't such a big deal and may not be necessary? If we _do_ want to explore ways to make this interaction easier for users, I think `FacetsConfig` is the only realistic "hook" in some sense. One issue is that there is actually no current place where we can know what SSDV fields in the index are being used for faceting. We can't rely on the "dim config" exposed by FacetsConfig since users are not required to register any specific dim config for a faceting field if they're happy with the defaults. I wonder if one option to explore is to have a `FacetsConfig` instance keep track of the SSDV fields being used for faceting when it builds docs? That's the one place we actually know that an SSDV field is being added for the purpose of faceting. If the `FacetsConfig` instance kept track of SSDV faceting fields in this way, we could add a helper method to `FacetsConfig` that takes an `IndexReader` as input and creates a new set of reader states. Then, we could consider a different ctor for SSDVFacets that takes a `FacetsConfig` instance instead of taking a state instance directly, and the `FacetsConfig` instance could provide the correct state instance. I think there's some complexity in here though since a single `FacetsConfig` instance could be used with different index readers while a refresh is happening, so it would probably need to keep track of a state instance for each SSDV field + IndexReader or something. I don't know. This might all just be too complicated to be worth i t, but it would be simpler for users if all they had to do was provide their `FacetsConfig` instance to SSDVFacets and then call some method on that `FacetsConfig` instance whenever they refresh their index. -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
