qstommyshu commented on code in PR #15567:
URL: https://github.com/apache/datafusion/pull/15567#discussion_r2027712552


##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -3388,26 +3389,15 @@ fn 
ident_normalization_parser_options_ident_normalization() -> ParserOptions {
     }
 }
 
-fn prepare_stmt_quick_test(
-    sql: &str,
-    expected_plan: &str,
-    expected_data_types: &str,
-) -> LogicalPlan {
+fn generate_prepare_stmt_and_data_types(sql: &str) -> (LogicalPlan, String) {

Review Comment:
   Just took a closer look at this method 👀. It turns out that 
`assert_debug_snapshot!` doesn’t support inline snapshots — it always writes to 
an external .snap file under the snapshots/ directory (which I assume we’re 
trying to avoid?).
   
   As for the output arguments — if we want to return just the `LogicalPlan`, 
I’d need to extract the `data_types` separately using pattern matching or a 
helper function, and then snapshot that field explicitly.
   
   So the updated test flow for a `PREPARE` statement would look like this:
   
   ```Rust
   let plan = logical_plan(sql).unwrap();
   if let LogicalPlan::Statement(Statement::Prepare(Prepare { data_types, .. 
})) = plan {
        assert_snapshot!(data_types, @r"SOME DATA TYPE");
   }
   assert_snapshot!(
           plan,
           @r#"
       Prepare: "my_plan" [Int32] 
         Projection: person.id, person.age
           Filter: person.age = Int64(10)
             TableScan: person
   "#
       );
   ```
   
   instead of what we have now:
   
   ```Rust
   let (plan, dt) = generate_prepare_stmt_and_data_types(sql);
       assert_snapshot!(
           plan,
           @r#"
       Prepare: "my_plan" [Int32] 
         Projection: person.id, person.age
           Filter: person.age = Int64(10)
             TableScan: person
       "#
       );
       assert_snapshot!(dt, @r#"[Int32]"#);
   ```
   
   I’m happy to switch to the first version if that’s preferred — just let me 
know what works best for you! @blaginin @alamb 



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