[ 
https://issues.apache.org/jira/browse/LUCENE-7500?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16044579#comment-16044579
 ] 

Michael McCandless commented on LUCENE-7500:
--------------------------------------------

+1, this is a nice simplification!

Maybe we can move most of the static methods from MultiFields to ReaderUtil or 
maybe a new MultiReaderUtil, and then can we remove MultiFields entirely?  Who 
still uses it (instantiates it as a Fields instance)? (But we can do that 
separately).

Can you add a comment here (PhraseHelper.java) explaining why the incoming 
field is ignored?

{noformat}
+    public Terms terms(String field) throws IOException {
+      return super.terms(fieldName);
+    }
{noformat}

This change in ParallelReader seems a little dangerous:

{noformat}
@@ -130,9 +131,12 @@
         if (!fieldToReader.containsKey(fieldInfo.name)) {
           builder.add(fieldInfo);
           fieldToReader.put(fieldInfo.name, reader);
-          if (fieldInfo.hasVectors()) {
-            tvFieldToReader.put(fieldInfo.name, reader);
-          }
+        }
+        if (fieldInfo.getIndexOptions() != IndexOptions.NONE) {
+          indexFieldToReader.putIfAbsent(fieldInfo.name, reader);
+        }
+        if (fieldInfo.hasVectors()) {
+          tvFieldToReader.putIfAbsent(fieldInfo.name, reader);
         }
       }
     }
{noformat}

because the two {{putIfAbsent}} are not inside the {{if 
(!fieldToReader.containsKey(fieldInfo.name))}}.  This could lead to cases where 
the FieldInfos returned by ParallelReader disagrees with what the 
{{terms(String)}} and term vectors APIs provide?


> Remove LeafReader.fields()
> --------------------------
>
>                 Key: LUCENE-7500
>                 URL: https://issues.apache.org/jira/browse/LUCENE-7500
>             Project: Lucene - Core
>          Issue Type: Improvement
>            Reporter: David Smiley
>            Assignee: David Smiley
>             Fix For: master (7.0)
>
>         Attachments: LUCENE_7500_avoid_leafReader_fields.patch, 
> LUCENE_7500_avoid_leafReader_fields.patch, 
> LUCENE_7500_Remove_LeafReader_fields.patch, 
> LUCENE_7500_Remove_LeafReader_fields.patch
>
>
> {{Fields}} seems like a pointless intermediary between the {{LeafReader}} and 
> {{Terms}}. Why not have {{LeafReader.getTerms(fieldName)}} instead? One loses 
> the ability to get the count and iterate over indexed fields, but it's not 
> clear what real use-cases are for that and such rare needs could figure that 
> out with FieldInfos.
> [~mikemccand] pointed out that we'd probably need to re-introduce a 
> {{TermVectors}} class since TV's are row-oriented not column-oriented.  IMO 
> they should be column-oriented but that'd be a separate issue.
> _(p.s. I'm lacking time to do this w/i the next couple months so if someone 
> else wants to tackle it then great)_



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to