You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by yu...@apache.org on 2022/09/27 02:52:37 UTC

[pinot] branch master updated: Fix Data-Correctness Bug in GTE Comparison in BinaryOperatorTransformFunction (#9461)

This is an automated email from the ASF dual-hosted git repository.

yupeng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 83b7f157f7 Fix Data-Correctness Bug in GTE Comparison in BinaryOperatorTransformFunction (#9461)
83b7f157f7 is described below

commit 83b7f157f77c07675d7760569a796199a41b5555
Author: Ankit Sultana <an...@uber.com>
AuthorDate: Tue Sep 27 08:22:32 2022 +0530

    Fix Data-Correctness Bug in GTE Comparison in BinaryOperatorTransformFunction (#9461)
    
    * Fix Bug in Handling GTE Comparison in BinaryOperatorTransformFunction
    
    * Add UT
    
    * Fix bug and add another test
---
 .../function/BinaryOperatorTransformFunction.java      | 12 ++++++------
 .../function/BinaryOperatorTransformFunctionTest.java  | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/BinaryOperatorTransformFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/BinaryOperatorTransformFunction.java
index d1531558ef..b7e173722e 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/BinaryOperatorTransformFunction.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/BinaryOperatorTransformFunction.java
@@ -400,7 +400,7 @@ public abstract class BinaryOperatorTransformFunction extends BaseTransformFunct
   private void fillLongResultArray(ProjectionBlock projectionBlock, float[] leftValues, int length) {
     long[] rightValues = _rightTransformFunction.transformToLongValuesSV(projectionBlock);
     for (int i = 0; i < length; i++) {
-      _results[i] = compare(leftValues[i], rightValues[i]);
+      _results[i] = getIntResult(compare(leftValues[i], rightValues[i]));
     }
   }
 
@@ -446,7 +446,7 @@ public abstract class BinaryOperatorTransformFunction extends BaseTransformFunct
   private void fillLongResultArray(ProjectionBlock projectionBlock, double[] leftValues, int length) {
     long[] rightValues = _rightTransformFunction.transformToLongValuesSV(projectionBlock);
     for (int i = 0; i < length; i++) {
-      _results[i] = compare(leftValues[i], rightValues[i]);
+      _results[i] = getIntResult(compare(leftValues[i], rightValues[i]));
     }
   }
 
@@ -526,17 +526,17 @@ public abstract class BinaryOperatorTransformFunction extends BaseTransformFunct
 
   private int compare(long left, double right) {
     if (Math.abs(left) <= 1L << 53) {
-      return getIntResult(Double.compare(left, right));
+      return Double.compare(left, right);
     } else {
-      return getIntResult(BigDecimal.valueOf(left).compareTo(BigDecimal.valueOf(right)));
+      return BigDecimal.valueOf(left).compareTo(BigDecimal.valueOf(right));
     }
   }
 
   private int compare(double left, long right) {
     if (Math.abs(right) <= 1L << 53) {
-      return getIntResult(Double.compare(left, right));
+      return Double.compare(left, right);
     } else {
-      return getIntResult(BigDecimal.valueOf(left).compareTo(BigDecimal.valueOf(right)));
+      return BigDecimal.valueOf(left).compareTo(BigDecimal.valueOf(right));
     }
   }
 
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/BinaryOperatorTransformFunctionTest.java b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/BinaryOperatorTransformFunctionTest.java
index 774f24721a..974812d8f2 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/BinaryOperatorTransformFunctionTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/BinaryOperatorTransformFunctionTest.java
@@ -100,6 +100,24 @@ public abstract class BinaryOperatorTransformFunctionTest extends BaseTransformF
       expectedValues[i] = getExpectedValue(_stringSVValues[i].compareTo(_stringSVValues[0]));
     }
     testTransformFunction(transformFunction, expectedValues);
+
+    // Test with heterogeneous arguments (long on left-side, double on right-side)
+    expression = RequestContextUtils.getExpression(
+        String.format("%s(%s, '%s')", functionName, LONG_SV_COLUMN, _doubleSVValues[0]));
+    transformFunction = TransformFunctionFactory.get(expression, _dataSourceMap);
+    for (int i = 0; i < NUM_ROWS; i++) {
+      expectedValues[i] = getExpectedValue(Double.compare(_longSVValues[i], _doubleSVValues[0]));
+    }
+    testTransformFunction(transformFunction, expectedValues);
+
+    // Test with heterogeneous arguments (double on left-side, long on right-side)
+    expression = RequestContextUtils.getExpression(
+        String.format("%s(%s, '%s')", functionName, DOUBLE_SV_COLUMN, _longSVValues[0]));
+    transformFunction = TransformFunctionFactory.get(expression, _dataSourceMap);
+    for (int i = 0; i < NUM_ROWS; i++) {
+      expectedValues[i] = getExpectedValue(Double.compare(_doubleSVValues[i], _longSVValues[0]));
+    }
+    testTransformFunction(transformFunction, expectedValues);
   }
 
   @Test(dataProvider = "testIllegalArguments", expectedExceptions = {BadQueryRequestException.class})


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org