Lucene - Core
  1. Lucene - Core
  2. LUCENE-3554

AbstractField / Field / NumericField do not override equals and hashcode methods

    Details

    • Type: Wish Wish
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: core/index
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Field, NumericField and AbstractField implementations of Fieldable do not override equals and hashcode methods and cannot be compared as such.

        Activity

        Alexandre Dupriez created issue -
        Hide
        Alexandre Dupriez added a comment - - edited

        Is there any special reason which prevent these classes to override the equals/hashcode methods? If not, would a patch be welcome?

        Show
        Alexandre Dupriez added a comment - - edited Is there any special reason which prevent these classes to override the equals/hashcode methods? If not, would a patch be welcome?
        Alexandre Dupriez made changes -
        Field Original Value New Value
        Summary AbstractField / Field / NumericField override equals and hashcode methods AbstractField / Field / NumericField do not override equals and hashcode methods
        Hide
        Robert Muir added a comment -

        How would equals work if you use public Field(String name, TokenStream tokenStream) ?

        Show
        Robert Muir added a comment - How would equals work if you use public Field(String name, TokenStream tokenStream) ?
        Hide
        Alexandre Dupriez added a comment -

        Could the equals method of Field rely on the equals implementation of AttributeSource, which TokenStream inherits from?

        Show
        Alexandre Dupriez added a comment - Could the equals method of Field rely on the equals implementation of AttributeSource, which TokenStream inherits from?
        Hide
        Robert Muir added a comment -

        The problem is you would have to consume the entire stream (analyze the potentially large document) to compute this...
        I think its better to just return Object.equals/hashcode than to have such a costly implementation

        Show
        Robert Muir added a comment - The problem is you would have to consume the entire stream (analyze the potentially large document) to compute this... I think its better to just return Object.equals/hashcode than to have such a costly implementation
        Hide
        Hoss Man added a comment -

        The problem is you would have to consume the entire stream (analyze the potentially large document) to compute this

        ...but TokenStream (via AttributeSource) already has equals+hashCode methods that aren't just inherited from Object – are they doing this stream consumption?

        if they are, that seems like a bug (TokenStream should probably override AttributeSource to use "==" and System.identiyHashCode, right?)

        That seems like an orthogonal issue to whether AbstractField/Field/NumericField could have useful equals/hashCode methods – it seems like those classes should be able to compose the equals/hashCodes of their parts, and if in some cases one of those parts is a TokenStream (which uses Object.equals) so be it – but in other cases (ie: NumericField, or other user implemented subclasses of AbstractField) it might be very useful.

        Show
        Hoss Man added a comment - The problem is you would have to consume the entire stream (analyze the potentially large document) to compute this ...but TokenStream (via AttributeSource) already has equals+hashCode methods that aren't just inherited from Object – are they doing this stream consumption? if they are, that seems like a bug (TokenStream should probably override AttributeSource to use "==" and System.identiyHashCode, right?) That seems like an orthogonal issue to whether AbstractField/Field/NumericField could have useful equals/hashCode methods – it seems like those classes should be able to compose the equals/hashCodes of their parts, and if in some cases one of those parts is a TokenStream (which uses Object.equals) so be it – but in other cases (ie: NumericField, or other user implemented subclasses of AbstractField) it might be very useful.
        Hide
        Robert Muir added a comment -

        ...but TokenStream (via AttributeSource) already has equals+hashCode methods that aren't just inherited from Object – are they doing this stream consumption?

        Only at that 'state' in time. You would have to walk the tokenstreams in parallel, verifying equals() along the way, and ensure # of tokens is the same to verify that two tokenstreams are 'equal'.

        if they are, that seems like a bug (TokenStream should probably override AttributeSource to use "==" and System.identiyHashCode, right?)

        No, because it works fine for that stream at that moment in time.

        That seems like an orthogonal issue to whether AbstractField/Field/NumericField could have useful equals/hashCode methods

        Not at all, because fields can be set by tokenstreams, equals/hashcode would be broken if we did this.

        broken != useful.

        Show
        Robert Muir added a comment - ...but TokenStream (via AttributeSource) already has equals+hashCode methods that aren't just inherited from Object – are they doing this stream consumption? Only at that 'state' in time. You would have to walk the tokenstreams in parallel, verifying equals() along the way, and ensure # of tokens is the same to verify that two tokenstreams are 'equal'. if they are, that seems like a bug (TokenStream should probably override AttributeSource to use "==" and System.identiyHashCode, right?) No, because it works fine for that stream at that moment in time. That seems like an orthogonal issue to whether AbstractField/Field/NumericField could have useful equals/hashCode methods Not at all, because fields can be set by tokenstreams, equals/hashcode would be broken if we did this. broken != useful.

          People

          • Assignee:
            Unassigned
            Reporter:
            Alexandre Dupriez
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:

              Time Tracking

              Estimated:
              Original Estimate - 1h 55m
              1h 55m
              Remaining:
              Remaining Estimate - 1h 55m
              1h 55m
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development