gsmiller commented on pull request #149: URL: https://github.com/apache/lucene/pull/149#issuecomment-849160773
Thanks for diving into this @sqshq! Yeah, I think I agree with all this. I suppose the subtle difference here is that in taxonomy-based facet counting, `FacetsConfig` would never have been told about a custom index field (because presumably it's not a valid dimension). You could get in a weird scenario though where you're using some custom index field for all your taxonomy faceting (and have never indexed anything in `$facets`). In other words, you've specified one or more custom index fields to hold the doc values via `FacetsConfig` and your index doesn't contain the `$facets` field at all. In this (somewhat unusual) case, if calling code were to ask for a dimension that was never indexed (same scenario you're describing in SSDVFC), you'd still get an `IllegalArgumentException` in `TaxonomyFacets#verifyDim` (because it would try to look up the configured index field for the dimension, wouldn't find it, fall back to `$facets` and then also not find that). That said, I don't think fixing that as well necessarily needs to be part of this change. It would be nice though if that could also consistently return `null`. Well, +1 for making the change you proposed, particularly since it's consistent with the Javadoc as you point out as well. -- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
