[GitHub] [incubator-druid] leventov commented on issue #6358: Interning in SQLMetadataSegmentManager may obliterate new segment metadata

[GitBox <gi...@apache.org>]

leventov commented on issue #6358: Interning in SQLMetadataSegmentManager may obliterate new segment metadata
URL: https://github.com/apache/incubator-druid/issues/6358#issuecomment-489102183
 
 
   I'm not sure what would be the best way to handle this in terms of both usability and error-proneness. If you just make `equals()` more expensive and somebody puts it in `HashSet` as keys, a lot of unnecessary work may be done in the result. On the other hand, there may be cases when we would still want to have `DataSegment` as keys in Maps to "pack" more data into a Map without creating extra wrapping objects, that is, having `ObjToIntMap<DataSegment>` instead of `Map<SegmentId, SomeWrapperWithDataSegmentAndIntFields>`.
   
   So what I would do is:
    - Leave `DataSegment`'s `equals()` and `hashCode()` as they are now
    - Add `DataSegment.allDataEquals()` checking all data.
    - Add a [Structural Search inspection](https://github.com/apache/incubator-druid/blob/master/dev/teamcity.md#creating-a-custom-inspection-from-a-structural-search-pattern) in intelliJ which inhibits using `DataSegment` in Map keys and Sets, but can still be done using `@SuppressWarnings("SSBasedInspection")` when somebody *really* needs that (see the example above).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org