github-actions[bot] commented on code in PR #63481:
URL: https://github.com/apache/doris/pull/63481#discussion_r3297680431
##########
fe/fe-catalog/src/main/java/org/apache/doris/analysis/TimeV2Literal.java:
##########
@@ -85,7 +85,23 @@ public boolean isMinValue() {
@Override
public int compareLiteral(LiteralExpr expr) {
- return 0;
+ throw new RuntimeException("Not support comparison between TIMEV2
literals");
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof TimeV2Literal)) {
+ return false;
+ }
+ return Double.compare(this.getValue(), ((TimeV2Literal)
obj).getValue()) == 0;
+ }
+
+ @Override
+ public int hashCode() {
Review Comment:
`equals()` now compares only `getValue()`, but `hashCode()` includes
`super.hashCode()`, and `super.hashCode()` includes the `ScalarType` scale.
That violates the Java equality contract for equal TIMEV2 values with different
scales, for example `new TimeV2Literal(1, 2, 3, 0, 0, false).equals(new
TimeV2Literal(1, 2, 3, 0, 6, false))` is true while their hashes differ because
`TIMEV2(0)` and `TIMEV2(6)` have different type hashes. Since this PR is
specifically fixing hash/equality behavior for hash-based predicate dedup, this
can still leave equal literals in different hash buckets and make dedup
inconsistent. Please make `equals()` and `hashCode()` use the same fields, and
add a test covering same TIMEV2 value with different scales.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
