erratic-pattern commented on code in PR #10358:
URL: https://github.com/apache/datafusion/pull/10358#discussion_r1588480661


##########
datafusion/core/tests/simplification.rs:
##########
@@ -658,3 +658,18 @@ fn test_simplify_concat() {
     let expected = concat(vec![col("c0"), lit("hello rust"), col("c1")]);
     test_simplify(expr, expected)
 }
+
+#[test]
+fn test_simplify_iterations() {

Review Comment:
   > does this test fail without the change?
   
   This was the example given in the original issue, though slightly modified 
to make it more test friendly. I thought it was failing without the change, 
though checking again now that appears to not be the case. I likely forgot to 
retest once I went down the `log` simplifier bug  rabbit hole.
   
   > Maybe we could add the test from the ticket too: 
https://github.com/apache/datafusion/issues/1160#issuecomment-952905624
   > @devinjdangelo 's deeply nested expressions usecase would also be 
interesting to test: 
https://github.com/apache/datafusion/issues/1160#issuecomment-1986906627
   
   Good idea. I can add those.
   
   I can also add the `simplify_inner` suggestion and then explicitly test the 
number of iterations used.



-- 
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-unsubscr...@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to