tomglk commented on PR #1883: URL: https://github.com/apache/solr/pull/1883#issuecomment-1710811919
Hi @cpoerschke , thank you for continuing the work here! The use of `useDocValuesForStored` to switch between the behaviours looks good to me, it seems concise. As for my understanding you plan to remove the `LegacyFieldValueFeature` **and** `useDocValuesForStored` in 9.X or 10, correct? Looking at the code, I think that people would have to copy quite a bit of code if they want to still use the legacy behaviour if useDocValuesForStored is gone. But I think that's fine. As long as all the FieldValueFeatures are separated from each other and we do not have [the prefetching](https://issues.apache.org/jira/browse/SOLR-15450), the DocValues approach is most efficient, I think. I [refactored the tests a bit](https://github.com/apache/solr/commit/ae6d160176ea3c41854641ee1336d752ebe4db6c), to make it easier to see what `test_storedDvIsTrendy_FieldValueFeatureScorer_className_differs` wants to test and added some helper functions for `testThatCorrectFieldValueFeatureIsUsedForDocValueTypes` to (subjectively) improve readability. What do you think about those changes? Other than that all good for me. :) -- 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...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
