[ 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