voonhous commented on code in PR #17655:
URL: https://github.com/apache/hudi/pull/17655#discussion_r2674794012
##########
hudi-common/src/main/java/org/apache/hudi/stats/HoodieColumnRangeMetadata.java:
##########
@@ -38,85 +40,29 @@
* associated with
*/
@SuppressWarnings("rawtype")
+@Value
public class HoodieColumnRangeMetadata<T extends Comparable> implements
Serializable {
- private final String filePath;
- private final String columnName;
- @Nullable
- private final T minValue;
- @Nullable
- private final T maxValue;
- private final long nullCount;
- private final long valueCount;
- private final long totalSize;
- private final long totalUncompressedSize;
- private final ValueMetadata valueMetadata;
-
- private HoodieColumnRangeMetadata(String filePath,
- String columnName,
- @Nullable T minValue,
- @Nullable T maxValue,
- long nullCount,
- long valueCount,
- long totalSize,
- long totalUncompressedSize,
- ValueMetadata valueMetadata) {
- this.filePath = filePath;
- this.columnName = columnName;
- this.minValue = minValue;
- this.maxValue = maxValue;
- this.nullCount = nullCount;
- this.valueCount = valueCount;
- this.totalSize = totalSize;
- this.totalUncompressedSize = totalUncompressedSize;
- this.valueMetadata = valueMetadata;
- }
-
- public String getFilePath() {
- return this.filePath;
- }
-
- public String getColumnName() {
- return this.columnName;
- }
+ String filePath;
+ String columnName;
@Nullable
- public T getMinValue() {
- return this.minValue;
- }
+ T minValue;
+ @Nullable
+ T maxValue;
+ long nullCount;
+ long valueCount;
+ long totalSize;
+ long totalUncompressedSize;
+ ValueMetadata valueMetadata;
public Object getMinValueWrapped() {
return getValueMetadata().wrapValue(getMinValue());
}
- @Nullable
- public T getMaxValue() {
- return this.maxValue;
- }
-
public Object getMaxValueWrapped() {
return getValueMetadata().wrapValue(getMaxValue());
}
- public long getNullCount() {
- return nullCount;
- }
-
- public long getValueCount() {
- return valueCount;
- }
-
- public long getTotalSize() {
- return totalSize;
- }
-
- public long getTotalUncompressedSize() {
- return totalUncompressedSize;
- }
-
- public ValueMetadata getValueMetadata() {
- return valueMetadata;
- }
-
@Override
public boolean equals(final Object o) {
Review Comment:
Okay, this is not feasible:
https://codefinity.com/blog/How-equals()-and-hashCode()-Work-in-Java-and-Why-Following-Their-Contract-Matters
According to various sources, the contract of "if two objects are equal
according to `equals()`, they MUST have the same `hashCode()`"
Our current refactor is causing failures in
`TestHoodieMetadataTableValidator#testValidate`. The falure is mainly caused by
us manually overriding `hashCode` while using the `@EqualsAndHashCode`
annotation. This will cause Lombok to not generate the `equals` override,
causing the test failures.
**Before Lombok (manual code)**:
- equals(): 8 fields
- hashCode(): 4 fields
**After Lombok (current broken state)**:
- equals() (Lombok-generated): 8 fields
- hashCode() (manual override): 4 fields
So i propose that we compare 8 fields for both `Equals` and `hashCode` to
remove this contract violation.
On top of that, using more fields will reduce the chance of hash collisions.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
