qstommyshu commented on PR #15578:
URL: https://github.com/apache/datafusion/pull/15578#issuecomment-2781439769
> Hi @alamb and @blaginin
>
> I found a way to migrate `roundtrip_statement_with_dialect()` now, just
want to confirm if you like to proceed with it.
>
> What I did below is to incorporate all core test logics into a macro, and
try to minimize the lines of code (so that we don't have too much lines of code
increase in this migration) for each test case but still expressive, by adding
the variable name before each variable in the macro (because rust analyzer
doesn't indicate types for macro).
>
> ```rust
> #[macro_export]
> macro_rules! roundtrip_statement_with_dialect_helper {
> (
> query: $sql:expr,
> parser_dialect: $parser_dialect:expr,
> unparser_dialect: $unparser_dialect:expr,
> expected: @ $expected:literal $(,)?
> ) => {{
> let statement = Parser::new(&$parser_dialect)
> .try_with_sql($sql)?
> .parse_statement()?;
>
> let state = MockSessionState::default()
> .with_aggregate_function(max_udaf())
> .with_aggregate_function(min_udaf())
> .with_expr_planner(Arc::new(CoreFunctionPlanner::default()))
> .with_expr_planner(Arc::new(NestedFunctionPlanner));
>
> let context = MockContextProvider { state };
> let sql_to_rel = SqlToRel::new(&context);
> let plan = sql_to_rel
> .sql_statement_to_plan(statement)
> .unwrap_or_else(|e| panic!("Failed to parse sql: {}\n{e}",
$sql));
>
> let unparser = Unparser::new(&$unparser_dialect);
> let roundtrip_statement = unparser.plan_to_sql(&plan)?;
>
> let actual = &roundtrip_statement.to_string();
> insta::assert_snapshot!(actual, @ $expected);
> }};
> }
> ```
>
> ```rust
> // each test case would look like this
> #[test]
> fn roundtrip_statement_with_dialect_1() -> Result<(), DataFusionError> {
> roundtrip_statement_with_dialect_helper!(
> query: "select min(ta.j1_id) as j1_min from j1 ta order by
min(ta.j1_id) limit 10;",
> parser_dialect: MySqlDialect {},
> unparser_dialect: UnparserMySqlDialect {},
> expected: @"SELECT `j1_min` FROM (SELECT min(`ta`.`j1_id`) AS
`j1_min`, min(`ta`.`j1_id`) FROM `j1` AS `ta` ORDER BY min(`ta`.`j1_id`) ASC)
AS `derived_sort` LIMIT 10",
> );
> Ok(())
> }
> ```
>
> Note: we can't put this macro in a for loop because inline
`assert_snapshot!($actual, @ $expected)` only takes literal as expected
argument, so we can't store the expected value in a variable and then pass to
the macro (expr != literal).
>
> And I feel like this is not a standard way to write test, not sure if I
should go head and do it or not. Please let me know if you want to proceed with
this way.
Just want to confirm, do I need to migrate the big test case
`roundtrip_statement_with_dialect() `?
--
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
