tshauck opened a new pull request, #7134: URL: https://github.com/apache/arrow-datafusion/pull/7134
Hi! Opening this PR a bit early to see if it's going in there right direction. This is from the issue here https://github.com/apache/arrow-datafusion/issues/7133 and the discussion here https://github.com/apache/arrow-datafusion/discussions/7032. The gist is there's a `sqllogictest` folder that adds a cate for `datafusion-sqllogictest`. The main export is `DataFusionTestRunner` that implements the sqllogictest-rs `AsyncDB`. See below for an example of it being used in a branch of one of my projects. My main question is how to handle the existing code that's internal to `core` and the ancillary postgres engine code... should the former be replaced with this and the latter stay, or should the latter come along into this new crate? Certainly direction here and anything else big or small. Thanks! # Which issue does this PR close? Closes #7133 # Rationale for this change This change makes it possible for crates that use datafusion to use its sqllogictest engine. E.g. presuming this is merged, the end user could do the following... ```rust async fn run_tests() -> Result<(), DataFusionError> { let test_options = TestOptions::default(); // Iterate through the test files and run the tests. let test_files = std::fs::read_dir(&test_options.test_dir).unwrap(); let exon_context = Arc::new(SessionContext::new_exon()); for test_file in test_files { let test_file = test_file.unwrap(); // if the file doesn't end with an slt extension skip it if test_file.path().extension().unwrap() != "slt" { continue; } let mut runner = sqllogictest::Runner::new(|| async { Ok(DataFusionTestRunner::new(exon_context.clone())) }); runner.run_file_async(test_file.path()).await.unwrap(); } Ok(()) } #[tokio::main] pub async fn main() -> Result<(), DataFusionError> { run_tests().await } ``` This is on a branch of the exon library: https://github.com/wheretrue/exon/blob/bfb985c89f228e8ce86dbfd96e2a6e0698e780dc/exon/tests/sqllogictests/src/main.rs # What changes are included in this PR? A new crate is added that exposes `DataFusionTestRunner` which is used in the example above. I haven't yet updated the existing test code to use this crate, but presumably would want to? I'm also not sure where the postgres code would go in that scenario. Should it also go in this crate or stay where it is? # Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> # Are there any user-facing changes? A new crate would be available. -- 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]
