You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Jackie-Jiang (via GitHub)" <gi...@apache.org> on 2024/02/05 18:29:58 UTC

[I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Jackie-Jiang opened a new issue, #12367:
URL: https://github.com/apache/pinot/issues/12367

   ```
   Error:  org.apache.pinot.core.operator.transform.function.CaseTransformFunctionTest.testCaseTransformFunctionWithIntResults -- Time elapsed: 1.254 s <<< FAILURE!
   java.lang.AssertionError: expected [-766578] but found [10]
   	at org.testng.Assert.fail(Assert.java:111)
   	at org.testng.Assert.failNotEquals(Assert.java:1578)
   	at org.testng.Assert.assertEqualsImpl(Assert.java:150)
   	at org.testng.Assert.assertEquals(Assert.java:132)
   	at org.testng.Assert.assertEquals(Assert.java:1419)
   	at org.testng.Assert.assertEquals(Assert.java:1383)
   	at org.testng.Assert.assertEquals(Assert.java:1429)
   	at org.apache.pinot.core.operator.transform.function.BaseTransformFunctionTest.testTransformFunction(BaseTransformFunctionTest.java:345)
   	at org.apache.pinot.core.operator.transform.function.CaseTransformFunctionTest.testCaseQueryWithIntResults(CaseTransformFunctionTest.java:595)
   	at org.apache.pinot.core.operator.transform.function.CaseTransformFunctionTest.testCaseQueries(CaseTransformFunctionTest.java:223)
   	at org.apache.pinot.core.operator.transform.function.CaseTransformFunctionTest.testCaseTransformFunctionWithIntResults(CaseTransformFunctionTest.java:108)
   ```


-- 
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: commits-unsubscribe@pinot.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "aditya0811 (via GitHub)" <gi...@apache.org>.
aditya0811 commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-1969084205

   > @aditya0811 Thanks for taking this issue! I feel the problem is from the implicit cast from float to double within the double comparison. The literal is parsed as double, thus causing the implicit cast. In order to reproduce the issue, we need to pick a float value which will change when casting to double.
   
   So, shouldn't we then consider this as bug, in L434 BinaryOperatorTransformFunction.java, rather than updating UT? Maybe explore BigDecimal, similar to BigDecimal and float comparison in L441
    `_intValuesSV[i] = getIntResult(BigDecimal.valueOf(leftValues[i]).compareTo(rightBigDecimalValues[i]));`
    
    > In order to reproduce the issue, we need to pick a float value which will change when casting to double.
    
    Let me try this.
   


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "aditya0811 (via GitHub)" <gi...@apache.org>.
aditya0811 commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-2002499800

   ohk, so in the test, we try to cast `floatSVValues[INDEX_TO_COMPARE]` as as float?


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang closed issue #12367: [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults()
URL: https://github.com/apache/pinot/issues/12367


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "aditya0811 (via GitHub)" <gi...@apache.org>.
aditya0811 commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-2045759962

   Hello Jackie, I was able to reproduce the error by changing `protected static final int NUM_ROWS = 1` and below in `setUp()` in `BaseTransformFunctionTest.java`. 
   ```
   floatSVValues[i] = 0.1f
   ```
   
   And below in `CaseTransformFunctionTest.java`
   
   ```
     private static final TransformFunctionType[] BINARY_OPERATOR_TRANSFORM_FUNCTIONS = new TransformFunctionType[]{
         TransformFunctionType.EQUALS
     };
   ```
   Error trace
   ```
   java.lang.AssertionError:
   Expected :1876674542
   Actual   :10
   <Click to see difference>
   
   
   	at org.testng.Assert.fail(Assert.java:111)
   	at org.testng.Assert.failNotEquals(Assert.java:1578)
   	at org.testng.Assert.assertEqualsImpl(Assert.java:150)
   	at org.testng.Assert.assertEquals(Assert.java:132)
   	at org.testng.Assert.assertEquals(Assert.java:1419)
   	at org.testng.Assert.assertEquals(Assert.java:1383)
   	at org.testng.Assert.assertEquals(Assert.java:1429)
   	at org.apache.pinot.core.operator.transform.function.BaseTransformFunctionTest.testTransformFunction(BaseTransformFunctionTest.java:345)
   	at org.apache.pinot.core.operator.transform.function.CaseTransformFunctionTest.testCaseQueryWithIntResults(CaseTransformFunctionTest.java:595)
   	at org.apache.pinot.core.operator.transform.function.CaseTransformFunctionTest.testCaseQueries(CaseTransformFunctionTest.java:223)
   	at org.apache.pinot.core.operator.transform.function.CaseTransformFunctionTest.testCaseTransformFunctionWithIntResults(CaseTransformFunctionTest.java:108)
   
   
   ```
   And the test passed with this
   
   ```
   testCaseQueries(String.format("%s(%s, %s)", functionType.getName(), FLOAT_SV_COLUMN,
             "CAST (" + String.format("%f", _floatSVValues[INDEX_TO_COMPARE]) +" AS FLOAT)"), getPredicateResults(FLOAT_SV_COLUMN, functionType));
   ```


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-2052547069

   @aditya0811 Great! Would you like to help fix the test?


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-1970089446

   Comparing a float column with a double value might never match when the implicit casting changes the float value. This is actually the standard SQL behavior, and the expected result in the test is wrong.
   @zhtaoxiang Did you create an issue about this behavior?


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "aditya0811 (via GitHub)" <gi...@apache.org>.
aditya0811 commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-2053672245

   Raised PR https://github.com/apache/pinot/pull/12922


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "aditya0811 (via GitHub)" <gi...@apache.org>.
aditya0811 commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-1974871309

   Hey Jackie, sorry I am not able to follow. 
   Lets say `_floatSVValues[INDEX_TO_COMPARE] = 0.1f`. In the case block in L721 in `CaseTransformFunctionTest` we are considering this as float `0.1f`. Lets say `_floatSVValues[i]=0.2f`. So the comparison is happening in the case block as, considering `GREATER_THAN_OR_EQUAL` `0.2f >= 0.1f` 
   The `_floatSVValues[INDEX_TO_COMPARE]` when considered as `LiteralTransformFunction` is getting parsed as double i.e. `0.1d`. 
   So now the comparison is happening as `(double) 0.2f >= 0.1d` ?
   


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "aditya0811 (via GitHub)" <gi...@apache.org>.
aditya0811 commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-1973584832

   Hey Jackie, when you say expected result is wrong, you mean the comparison in the case block in L721 in CaseTransformFunctionTest.java?
   ```     
    case FLOAT_SV_COLUMN:
             switch (type) {
               case EQUALS:
                 results[i] = _floatSVValues[i] == _floatSVValues[INDEX_TO_COMPARE];
                 break;
               case NOT_EQUALS:
                 results[i] = _floatSVValues[i] != _floatSVValues[INDEX_TO_COMPARE];
                 break;
               case GREATER_THAN:
                 results[i] = _floatSVValues[i] > _floatSVValues[INDEX_TO_COMPARE];
                 break;
               case GREATER_THAN_OR_EQUAL:
                 results[i] = _floatSVValues[i] >= _floatSVValues[INDEX_TO_COMPARE];
                 break;
               case LESS_THAN:
                 results[i] = _floatSVValues[i] < _floatSVValues[INDEX_TO_COMPARE];
                 break;
               case LESS_THAN_OR_EQUAL:
                 results[i] = _floatSVValues[i] <= _floatSVValues[INDEX_TO_COMPARE];
                 break;
   ```
   


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "aditya0811 (via GitHub)" <gi...@apache.org>.
aditya0811 commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-1967249666

   Hello Jackie, I was trying to identify if there is any gap in the test. I can think of two possible reasons that this test could fail
   a) The case statement computation is incorrect.
   b) The predicate results are incorrect. 
   
   Primarily I observed the difference in the way we compare the values in a) in L434 BinaryOperatorTransformFunction.java
   `_intValuesSV[i] = getIntResult(Double.compare(leftValues[i], rightDoubleValues[i]));`
   Here we are comparing float and double, as leftValues[i] is float and rightDoubleValues[i] is double. 
   
   and in (b) L724 CaseTransformFunctionTest.java which results in expectedValues[i] in the assert statement in L345 BaseTransformFunctionTest
   `results[i] = _floatSVValues[i] == _floatSVValues[INDEX_TO_COMPARE];`
   
   
   
   I tried running the test only for that assert statement 100k times with only intSVValues and floatSVValues as test data. All passed. So been unable to conclude anything. Do we have any more information for this test like, if it specific to expression or any binary function? 


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-2004993115

   Yes. Let's first try to reproduce the failure, then see if adding a cast can fix it


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-1974115082

   Yes. When we use float as literal (e.g. `WHERE col = 0.1`), it is actually parsed as double. If the `col` is of type `FLOAT`, it might not match this filter because `(double) 0.1f != 0.1d`


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-1967742248

   @aditya0811 Thanks for taking this issue!
   I feel the problem is from the implicit cast from float to double within the double comparison. The literal is parsed as double, thus causing the implicit cast.
   In order to reproduce the issue, we need to pick a float value which will change when casting to double.


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [I] [Flaky test] CaseTransformFunctionTest. testCaseTransformFunctionWithIntResults() [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on issue #12367:
URL: https://github.com/apache/pinot/issues/12367#issuecomment-1980200987

   When casting `0.1f` to `double`, the result is actually `0.10000000149011612`, which is not equal to `0.1d`. We might hit this scenario because the literal is parsed as double instead of float. I guess if we send the literal as `CAST(0.1 AS FLOAT)` it might fix the issue


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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