Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Around line 95 you have:

      if (this.omitTf != omitTf)

      { this.omitTf = true; // if one require omitTf at least once, it remains off for life }

      Both references of the omitTf booleans in the if statement refer to the same field. I am guessing its meant to be other.omitTf like the norms code above it.

        Activity

        Hide
        Michael Busch added a comment -

        Good eyes Mark!

        I was wondering why this didn't result in any unit test failures and it turns out that the only method that calls FieldInfo#update(FieldInfo) is this one in FieldInfos:

          synchronized public FieldInfo add(FieldInfo fieldInfo) {
            FieldInfo fi = fieldInfo(fieldInfo.name);
            if (fi == null) {
              return addInternal(fieldInfo.name, fieldInfo.isIndexed, fieldInfo.storeTermVector,
                                 fieldInfo.storePositionWithTermVector, fieldInfo.storeOffsetWithTermVector,
                                 fieldInfo.omitNorms, fieldInfo.storePayloads, fieldInfo.omitTf);
            } else {
              fi.update(fieldInfo);
            }
            return fi;
          }
        

        However, nowhere in Lucene's core is this add(FieldInfo) method called. Also all contrib modules build successfully if I remove this method. Does anybody know why we have the method in the core? It seems like we can just remove it?

        Show
        Michael Busch added a comment - Good eyes Mark! I was wondering why this didn't result in any unit test failures and it turns out that the only method that calls FieldInfo#update(FieldInfo) is this one in FieldInfos: synchronized public FieldInfo add(FieldInfo fieldInfo) { FieldInfo fi = fieldInfo(fieldInfo.name); if (fi == null ) { return addInternal(fieldInfo.name, fieldInfo.isIndexed, fieldInfo.storeTermVector, fieldInfo.storePositionWithTermVector, fieldInfo.storeOffsetWithTermVector, fieldInfo.omitNorms, fieldInfo.storePayloads, fieldInfo.omitTf); } else { fi.update(fieldInfo); } return fi; } However, nowhere in Lucene's core is this add(FieldInfo) method called. Also all contrib modules build successfully if I remove this method. Does anybody know why we have the method in the core? It seems like we can just remove it?
        Hide
        Michael McCandless added a comment -

        Indeed we should just remove both FieldInfo.add(FIeldInfo) and update(FieldInfo) – they are dead code.

        I had added these early on when refactoring DocumentsWriter but then I refactored further, to not actually call them, but forgot to remove them.

        Show
        Michael McCandless added a comment - Indeed we should just remove both FieldInfo.add(FIeldInfo) and update(FieldInfo) – they are dead code. I had added these early on when refactoring DocumentsWriter but then I refactored further, to not actually call them, but forgot to remove them.
        Hide
        Michael McCandless added a comment -

        Committed revision 718537.

        Thanks Mark!

        Show
        Michael McCandless added a comment - Committed revision 718537. Thanks Mark!

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Mark Miller
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development