qstommyshu commented on PR #15484: URL: https://github.com/apache/datafusion/pull/15484#issuecomment-2763402403
Hi @alamb , @blaginin , @xudong963 , The code changes is now done, please review carefully as the code changes is **LARGE**. There some several things to note when I'm working on this migration: 1. I found and fixed several minor bugs in the test suite. There are some semantic error in some tests, the name suggest it is a test for `prepare_stmt`, but they aren't, and these tests uses function specialized for `prepare_stmt`. For example: `test_prepare_statement_infer_types_from_join`, `test_prepare_statement_infer_types_from_predicate`, `test_prepare_statement_infer_types_from_between_predicate`... just to name a few. The code runs on those tests, but the logic are incorrect, so I fixed them by changing them back to regular sql statement tests. 2. I renamed a test called `select_order_by()`to `select_with_order_by()`, because the original name `select_order_by()` creates a naming conflict with `test_select_order_by()` for `insta` tests. 3. I also refactored some code where they calls `plan.replace_with_param_values()` then calls `plan.with_param_values()`. Those code are redundant as `plan.with_param_values()` calls `plan.replace_with_param_values()` internally, calling them multiple times only adds execution overheads. 4. I also found a little bug for `insta` (does not have effects on this PR), and plan to raise a issue to them soon. enjoy the code :) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
