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