yashmayya commented on code in PR #19791:
URL: https://github.com/apache/pinot/pull/19791#discussion_r4286971371
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ObjectFunctions.java:
##########
@@ -44,14 +48,73 @@ public static boolean isDistinctFrom(@Nullable Object obj1,
@Nullable Object obj
if (obj1 == null || obj2 == null) {
return true;
}
- return !obj1.equals(obj2);
+ if (isNumber(obj1) && isNumber(obj2)) {
+ return !numberEquals((Number) obj1, (Number) obj2);
+ }
+ return !Objects.deepEquals(obj1, obj2);
}
@ScalarFunction(nullableParameters = true)
public static boolean isNotDistinctFrom(@Nullable Object obj1, @Nullable
Object obj2) {
return !isDistinctFrom(obj1, obj2);
}
+ private static boolean isNumber(Object obj) {
+ return obj instanceof Integer || obj instanceof Long || obj instanceof
Float || obj instanceof Double
+ || obj instanceof BigDecimal;
+ }
+
+ private static boolean isFloatingPoint(Number number) {
+ return number instanceof Float || number instanceof Double;
+ }
+
+ /// Compares two numbers by value, so that numbers of different types (e.g.
`Integer` and `Long`) can be equal. This
+ /// is close to `BinaryOperatorTransformFunction`, which runs
`isDistinctFrom` in the single-stage engine and in the
+ /// multi-stage leaf stage. It differs only in two edge cases: a `long`
above 2^53 is compared exactly with a
+ /// floating point number, and NaN or infinity compared with a `BigDecimal`
does not throw.
+ /// - Two integers are compared as `long`.
+ /// - Two floating point numbers are compared with [Double#compare], like
[Double#equals]: a `float` is widened
+ /// (`1.1f` does not equal `1.1`), NaN equals NaN, and `-0.0` does not
equal `0.0`.
+ /// - An integer and a floating point number are compared exactly.
+ /// - When either number is a `BigDecimal`, both are compared as
`BigDecimal`, which ignores the scale (`1.5`
+ /// equals `1.50`). A floating point number is converted with
[BigDecimal#valueOf(double)], so `0.1` equals the
+ /// `BigDecimal` `0.1`. NaN and infinity do not equal any `BigDecimal`.
+ private static boolean numberEquals(Number number1, Number number2) {
+ if (number1 instanceof BigDecimal || number2 instanceof BigDecimal) {
+ BigDecimal value1 = toBigDecimal(number1);
+ BigDecimal value2 = toBigDecimal(number2);
Review Comment:
Agreed. I kept it as-is to match the leaf stage, and documented this case in
the `numberEquals` Javadoc in cd12b5c7c6.
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ObjectFunctions.java:
##########
@@ -44,14 +48,73 @@ public static boolean isDistinctFrom(@Nullable Object obj1,
@Nullable Object obj
if (obj1 == null || obj2 == null) {
return true;
}
- return !obj1.equals(obj2);
+ if (isNumber(obj1) && isNumber(obj2)) {
+ return !numberEquals((Number) obj1, (Number) obj2);
+ }
+ return !Objects.deepEquals(obj1, obj2);
}
@ScalarFunction(nullableParameters = true)
public static boolean isNotDistinctFrom(@Nullable Object obj1, @Nullable
Object obj2) {
return !isDistinctFrom(obj1, obj2);
}
+ private static boolean isNumber(Object obj) {
+ return obj instanceof Integer || obj instanceof Long || obj instanceof
Float || obj instanceof Double
+ || obj instanceof BigDecimal;
Review Comment:
Added a comment on `isNumber` in cd12b5c7c6. It names the covered types and
says that other numbers use `Objects.deepEquals`.
--
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]
