You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/11/29 02:36:01 UTC
[GitHub] [iceberg] smallx opened a new pull request, #6305: API: Override `equals` and `hashCode` methods for primitive types
smallx opened a new pull request, #6305:
URL: https://github.com/apache/iceberg/pull/6305
Deserialization of POJOs in Flink will call the default parameterless constructor to generate a new instance.
However, `Comparators` uses the `PrimitiveType` instance as the key of `Map`:
https://github.com/apache/iceberg/blob/master/api/src/main/java/org/apache/iceberg/types/Comparators.java#L33-L47
We need to override `equals` and `hashCode` methods for primitive types, otherwise, exception will occur after serializing `Schema`:
```
java.lang.UnsupportedOperationException: Cannot determine comparator for type: int
at org.apache.iceberg.types.Comparators.forType(Comparators.java:71)
at org.apache.iceberg.types.Comparators.internal(Comparators.java:77)
at org.apache.iceberg.types.Comparators.access$300(Comparators.java:31)
at org.apache.iceberg.types.Comparators$StructLikeComparator.lambda$new$0(Comparators.java:109)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
at java.base/java.util.Collections$2.tryAdvance(Collections.java:4747)
at java.base/java.util.Collections$2.forEachRemaining(Collections.java:4755)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:550)
at java.base/java.util.stream.AbstractPipeline.evaluateToArrayNode(AbstractPipeline.java:260)
at java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:517)
at org.apache.iceberg.types.Comparators$StructLikeComparator.<init>(Comparators.java:112)
at org.apache.iceberg.types.Comparators$StructLikeComparator.<init>(Comparators.java:102)
at org.apache.iceberg.types.Comparators.forType(Comparators.java:53)
at org.apache.iceberg.util.StructLikeWrapper.<init>(StructLikeWrapper.java:43)
at org.apache.iceberg.util.StructLikeWrapper.forType(StructLikeWrapper.java:34)
at org.apache.iceberg.util.StructLikeMap.put(StructLikeMap.java:87)
at org.apache.iceberg.util.StructLikeMap.put(StructLikeMap.java:32)
at org.apache.iceberg.io.BaseTaskWriter$BaseEqualityDeltaWriter.write(BaseTaskWriter.java:125)
at org.apache.iceberg.flink.sink.BaseDeltaTaskWriter.write(BaseDeltaTaskWriter.java:79)
at org.apache.iceberg.flink.sink.BaseDeltaTaskWriter.write(BaseDeltaTaskWriter.java:39)
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a diff in pull request #6305: API: Override `equals` and `hashCode` methods for primitive types
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #6305:
URL: https://github.com/apache/iceberg/pull/6305#discussion_r1038391965
##########
api/src/main/java/org/apache/iceberg/types/Type.java:
##########
@@ -112,6 +113,23 @@ public PrimitiveType asPrimitiveType() {
Object writeReplace() throws ObjectStreamException {
return new PrimitiveHolder(toString());
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ } else if (!(o instanceof PrimitiveType)) {
+ return false;
+ }
+
+ PrimitiveType that = (PrimitiveType) o;
+ return typeId() == that.typeId();
Review Comment:
@smallx: Oh, I missed that. Thanks!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a diff in pull request #6305: API: Override `equals` and `hashCode` methods for primitive types
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #6305:
URL: https://github.com/apache/iceberg/pull/6305#discussion_r1037861696
##########
api/src/main/java/org/apache/iceberg/types/Type.java:
##########
@@ -112,6 +113,23 @@ public PrimitiveType asPrimitiveType() {
Object writeReplace() throws ObjectStreamException {
return new PrimitiveHolder(toString());
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ } else if (!(o instanceof PrimitiveType)) {
+ return false;
+ }
+
+ PrimitiveType that = (PrimitiveType) o;
+ return typeId() == that.typeId();
Review Comment:
What about `PrimitiveType` where we have an attribute?
Like:
- `TimestampType(boolean adjustToUTC)`
- `FixedType ofLength(int length)`
- `DecimalType of(int precision, int scale)`
Do we consider them equals if the parameters are different?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu merged pull request #6305: API: Override `equals` and `hashCode` methods for primitive types
Posted by GitBox <gi...@apache.org>.
stevenzwu merged PR #6305:
URL: https://github.com/apache/iceberg/pull/6305
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on pull request #6305: API: Override `equals` and `hashCode` methods for primitive types
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on PR #6305:
URL: https://github.com/apache/iceberg/pull/6305#issuecomment-1331485071
this looks good to me. but let's wait for more reviews
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on pull request #6305: API: Override `equals` and `hashCode` methods for primitive types
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on PR #6305:
URL: https://github.com/apache/iceberg/pull/6305#issuecomment-1336556429
thanks @smallx for the contribution and @pvary for the review
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] smallx commented on a diff in pull request #6305: API: Override `equals` and `hashCode` methods for primitive types
Posted by GitBox <gi...@apache.org>.
smallx commented on code in PR #6305:
URL: https://github.com/apache/iceberg/pull/6305#discussion_r1038214032
##########
api/src/main/java/org/apache/iceberg/types/Type.java:
##########
@@ -112,6 +113,23 @@ public PrimitiveType asPrimitiveType() {
Object writeReplace() throws ObjectStreamException {
return new PrimitiveHolder(toString());
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ } else if (!(o instanceof PrimitiveType)) {
+ return false;
+ }
+
+ PrimitiveType that = (PrimitiveType) o;
+ return typeId() == that.typeId();
Review Comment:
@pvary `TimestampType`, `FixedType` and `DecimalType` have overridden `equals` and `hashCode` methods of `Object` and `PrimitiveType`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org