Add equals/hashcode to fieldtypes #11644
Conversation
In order to restrict a single set of field type settings for a given field name across an index, we need the ability to compare field types. This change adds equals and hashcode, as well as tests for every field type.
rjernst
added
v2.0.0-beta1
:Search/Mapping
|
Note there were also some bugs found in GeoShapeFieldMapper that are fixed here. Notably the default strategy, tree level, and tree precision were not correctly serialized. |
clintongormley
changed the title
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (!(o instanceof MappedFieldType)) return false; |
There was a problem hiding this comment.
super.equals already ensures this, since it does: if (getClass() != obj.getClass()) return false;
There was a problem hiding this comment.
I generated all equals/hashcode methods with intellij. I've removed the checks before super.equals, and filed an issue with intellij [1] to be smarter about this.
I also had noticed always using Objects.equals here, even for primitive types. It looks like this is fixed [2] in an unreleased version of intellij. For now, I've gone through and changed each primitive type to use == instead.
[1] https://youtrack.jetbrains.com/issue/IDEA-141479
[2] https://youtrack.jetbrains.com/issue/IDEA-139622
|
I left some minor comments but I like the PR in general, especially the careful unit tests. |
| geoShapeFieldType.setStrategies(strategyName, recursiveStrategy, termStrategy); | ||
| geoShapeFieldType.setOrientation(orientation); | ||
| if (context.indexCreatedVersion().before(Version.V_2_0_0) || | ||
| (geoShapeFieldType.treeLevels() == 0 && geoShapeFieldType.precisionInMeters() < 0)) { |
There was a problem hiding this comment.
I know this was here before, definitely not a blocker as we can clean it up in another PR, but this treeLevels == 0 check has bothered me. Especially since a treeLevel of 0 doesn't make sense and this is only used as an "initial state" condition. It seems cleaner to set the default tree and level at construction and appropriately change if/when the user explicitly defines the tree? That might make these conditionals cleaner and the whole tree parameter code easier to read?
There was a problem hiding this comment.
Since this isn't a blocker we'll cleanup these defaults in a separate 2.x issue. Opened #11682 to track.
|
Geo field changes LGTM |
|
@jpountz I pushed a new commit. |
|
LGTM |
Add equals/hashcode to fieldtypes
In order to restrict a single set of field type settings for a given
field name across an index, we need the ability to compare field types.
This change adds equals and hashcode, as well as tests for every field
type.