[ https://issues.apache.org/jira/browse/SOLR-8344?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16165102#comment-16165102 ]
David Smiley commented on SOLR-8344: ------------------------------------ * SolrDocumentFetcher: minor: {{allStoreds}} variables & getter should simply be {{allStored}} (no trailing 's' which reads weird) * RetrieveFieldsOptimizer.storedFields add comment saying "null" means all available * RetrieveFieldsOptimizer.calcStoredFieldsForReturn: in the hasPatternMatching condition check: eagerly initialize storedFields to a new HashSet instead of lazy. If it turns out that no stored fields match, we don't want to act like we want all stored fields, which is what I think will happen with your logic. To clarify calcStoredFieldsForReturn further, I suggest making storedFields final, and return null early for wantsAllFields. (IMO final fields can help clarify non-trivial logic) * RetrieveFieldsOptimizer.optimize: couldn't the loop here be replaced with a {{dvFields.addAll(storedFields); storedFields.clear();}} ? * minor: It's debatable to me that RetrieveFieldsOptimizer is its own class. It seems to me that a few static methods on DocsStreamer would be fine? (package access for testing). No big deal though. * do we really need to make this setting toggle-able with a new parameter? I suppose you left this in for performance testing but I think it can be removed now. * should we consider the presence of useDocValuesAsStored=false in one of the fields that are both stored and docValues as a signal that we should not do this optimization? I suppose so. I think I see an existing bug (not introduced here) in the logic you moved -- calcDocValueFieldsForReturn. If {{fl=foo*,dvField}} and dvField has useDocValuesAsStored=false then the code won't return dvField even though it's been explicitly mentioned. I haven't tried this; I'm just reading the code carefully. > Decide default when requested fields are both column and row stored. > -------------------------------------------------------------------- > > Key: SOLR-8344 > URL: https://issues.apache.org/jira/browse/SOLR-8344 > Project: Solr > Issue Type: New Feature > Reporter: Ishan Chattopadhyaya > Attachments: SOLR-8344.patch, SOLR-8344.patch > > > This issue was discussed in the comments at SOLR-8220. Splitting it out to a > separate issue so that we can have a focused discussion on whether/how to do > this. > If a given set of requested fields are all stored and have docValues (column > stored), we can retrieve the values from either place. What should the > default be? -- This message was sent by Atlassian JIRA (v6.4.14#64029) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org