[GitHub] [arrow-datafusion] nseekhao commented on a diff in pull request #6604: Support Expr::InList to Substrait::RexType

["nseekhao (via GitHub)" <gi...@apache.org>]

nseekhao commented on code in PR #6604:
URL: https://github.com/apache/arrow-datafusion/pull/6604#discussion_r1228412211


##########
datafusion/substrait/tests/roundtrip_logical_plan.rs:
##########
@@ -334,10 +334,27 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn roundtrip_inlist() -> Result<()> {
+    async fn roundtrip_inlist_1() -> Result<()> {
         roundtrip("SELECT * FROM data WHERE a IN (1, 2, 3)").await
     }
 
+    #[tokio::test]
+    // Test with length <= datafusion_optimizer::simplify_expressions::expr_simplifier::THRESHOLD_INLINE_INLIST
+    async fn roundtrip_inlist_2() -> Result<()> {
+        roundtrip("SELECT * FROM data WHERE f IN ('a', 'b', 'c')").await
+    }
+
+    #[tokio::test]
+    // Test with length > datafusion_optimizer::simplify_expressions::expr_simplifier::THRESHOLD_INLINE_INLIST
+    async fn roundtrip_inlist_3() -> Result<()> {
+        roundtrip("SELECT * FROM data WHERE f IN ('a', 'b', 'c', 'd')").await
+    }

Review Comment:
   Since `datafusion_optimizer::simplify_expressions::expr_simplifier::THRESHOLD_INLINE_INLIST` can technically be changed in later versions of DF, would it make sense to create the test list with length derived directly from `THRESHOLD_INLINE_INLIST`?
   
   Maybe something like:
   ```rust
   let vlist: Vec<String> = (0 .. THRESHOLD_INLINE_INLIST+1).into_iter().map(|v| v.to_string()).collect();
   let vlist_str = vlist.join(",");
   
   roundtrip(format!("SELECT * FROM table_1 WHERE c1 IN ({vlist_str});")).await
   ```



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