[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

To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]