blaginin commented on code in PR #15480:
URL: https://github.com/apache/datafusion/pull/15480#discussion_r2020030614


##########
datafusion/substrait/tests/cases/roundtrip_logical_plan.rs:
##########
@@ -1374,30 +1464,32 @@ async fn assert_read_filter_count(
     Ok(())
 }
 
-async fn assert_expected_plan_unoptimized(
+async fn assert_and_generate_plan(

Review Comment:
   two small nitpicks: 
   
   1. in `generate_plan_from_substrait`, you return `LogicalPlan` which is 
asserted later. I personally really like that approach because you convert data 
to string as soon as possible. Maybe we could do the same thing here?
   2. I find `assert_and_generate_plan` a bit misleading because in reality you 
actually don't assert plan here (as it's happening in the test itself).



-- 
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