[jira] [Commented] (SOLR-8344) Decide default when requested fields are both column and row stored.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (SOLR-8344) Decide default when requested fields are both column and row stored.

JIRA jira@apache.org

    [ 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: [hidden email]
For additional commands, e-mail: [hidden email]