You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/03/25 06:45:41 UTC

[GitHub] [arrow-datafusion] HaoYang670 opened a new issue #1727: Remove duplicate tests from `test_const_evaluator_scalar_functions`

HaoYang670 opened a new issue #1727:
URL: https://github.com/apache/arrow-datafusion/issues/1727


   **Describe the bug**
   I find that we have 2 same tests:
   ```rust
           // check that non foldable arguments are folded
           // to_timestamp(a) --> to_timestamp(a) [no rewrite possible]
           let expr = Expr::ScalarFunction {
               args: vec![col("a")],
               fun: BuiltinScalarFunction::ToTimestamp,
           };
           test_evaluate(expr.clone(), expr);
   
   
           // check that non foldable arguments are folded
           // to_timestamp(a) --> to_timestamp(a) [no rewrite possible]
           let expr = Expr::ScalarFunction {
               args: vec![col("a")],
               fun: BuiltinScalarFunction::ToTimestamp,
           };
           test_evaluate(expr.clone(), expr);
   ```
   https://github.com/apache/arrow-datafusion/blob/master/datafusion/src/optimizer/simplify_expressions.rs#L1026-L1040
   
   **To Reproduce**
   
   **Expected behavior**
   Remove one of the test or add some comment about why we need 2 same tests.
   
   **Additional context**
   


-- 
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: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-datafusion] HaoYang670 commented on issue #1727: Remove duplicate tests from `test_const_evaluator_scalar_functions`

Posted by GitBox <gi...@apache.org>.
HaoYang670 commented on issue #1727:
URL: https://github.com/apache/arrow-datafusion/issues/1727#issuecomment-1029496111


   Sure. Thank you, @bridyash13 


-- 
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: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-datafusion] jackwener commented on issue #1727: Remove duplicate tests from `test_const_evaluator_scalar_functions`

Posted by GitBox <gi...@apache.org>.
jackwener commented on issue #1727:
URL: https://github.com/apache/arrow-datafusion/issues/1727#issuecomment-1078720313


   I make a stupid mistake....ignored the supplement following.😅 I will add another PR......


-- 
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: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-datafusion] bridyash13 commented on issue #1727: Remove duplicate tests from `test_const_evaluator_scalar_functions`

Posted by GitBox <gi...@apache.org>.
bridyash13 commented on issue #1727:
URL: https://github.com/apache/arrow-datafusion/issues/1727#issuecomment-1029080710


   Hey @HaoYang670 can I work on this 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: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-datafusion] HaoYang670 commented on issue #1727: Remove duplicate tests in `test_const_evaluator_scalar_functions`

Posted by GitBox <gi...@apache.org>.
HaoYang670 commented on issue #1727:
URL: https://github.com/apache/arrow-datafusion/issues/1727#issuecomment-1027559822


   Another that I find
   https://github.com/apache/arrow-datafusion/blob/master/datafusion/src/optimizer/simplify_expressions.rs#L1042-L1054
   ```rust
           // volatile / stable functions should not be evaluated
           // rand() + (1 + 2) --> rand() + 3
           let fun = BuiltinScalarFunction::Random;
           assert_eq!(fun.volatility(), Volatility::Volatile);
           let rand = Expr::ScalarFunction { args: vec![], fun };
           let expr = rand.clone() + (lit(1) + lit(2));
           let expected = rand + lit(3);
           test_evaluate(expr, expected);
   
   
           // parenthesization matters: can't rewrite
           // (rand() + 1) + 2 --> (rand() + 1) + 2)
           let fun = BuiltinScalarFunction::Random;
           assert_eq!(fun.volatility(), Volatility::Volatile);
   ```
   
   We test `BuiltinScalarFunction::Random.volatility()` twice.


-- 
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: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-datafusion] xudong963 closed issue #1727: Remove duplicate tests from `test_const_evaluator_scalar_functions`

Posted by GitBox <gi...@apache.org>.
xudong963 closed issue #1727:
URL: https://github.com/apache/arrow-datafusion/issues/1727


   


-- 
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: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-datafusion] alamb closed issue #1727: Remove duplicate tests from `test_const_evaluator_scalar_functions`

Posted by GitBox <gi...@apache.org>.
alamb closed issue #1727:
URL: https://github.com/apache/arrow-datafusion/issues/1727


   


-- 
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: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-datafusion] jackwener edited a comment on issue #1727: Remove duplicate tests from `test_const_evaluator_scalar_functions`

Posted by GitBox <gi...@apache.org>.
jackwener edited a comment on issue #1727:
URL: https://github.com/apache/arrow-datafusion/issues/1727#issuecomment-1078673351


   I found the problem to be a very simple one, and it didn't seem to be progressing. so I fix it.
   
   BTW, I search the `bug` label and find it, and it shouldn't be a `bug`  but an enhancement.


-- 
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: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-datafusion] jackwener commented on issue #1727: Remove duplicate tests from `test_const_evaluator_scalar_functions`

Posted by GitBox <gi...@apache.org>.
jackwener commented on issue #1727:
URL: https://github.com/apache/arrow-datafusion/issues/1727#issuecomment-1078673351


   I found the problem to be a very simple one, and it didn't seem to be progressing. so I 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: github-unsubscribe@arrow.apache.org

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