Move sqllogictests to sqllogictests crate to break cyclic dependency (#7284)

* Move sqllogictests to datafusion-sqllogictest module

* Port code to be in datafusion

* Update expected output

* update workflows

* update flow

* update postgres check

* fmt

* clippy

* Add moved

* Add RAT to MOVED.md

* Fix tpch instructions and tests

Author: alamb

.github/workflows/dev_pr/labeler.yml

@@ -47,4 +47,4 @@ substrait:
   - datafusion/substrait/**/*
 
 sqllogictest:
-  - datafusion/core/tests/sqllogictests/**/*
+  - datafusion/sqllogictest/**/*

.github/workflows/rust.yml

@@ -229,17 +229,17 @@ jobs:
           rust-version: stable
       - name: Generate benchmark data and expected query results
         run: |
-          mkdir -p datafusion/core/tests/sqllogictests/test_files/tpch/data
+          mkdir -p datafusion/sqllogictest/test_files/tpch/data
           git clone https://github.com/databricks/tpch-dbgen.git
           cd tpch-dbgen
           make
           ./dbgen -f -s 0.1
-          mv *.tbl ../datafusion/core/tests/sqllogictests/test_files/tpch/data
+          mv *.tbl ../datafusion/sqllogictest/test_files/tpch/data
       - name: Verify that benchmark queries return expected results
         run: |
-          export TPCH_DATA=`realpath datafusion/core/tests/sqllogictests/test_files/tpch/data`
+          export TPCH_DATA=`realpath datafusion/sqllogictest/test_files/tpch/data`
           cargo test serde_q --profile release-nonlto --features=ci -- --test-threads=1
-          INCLUDE_TPCH=true cargo test -p datafusion --test sqllogictests
+          INCLUDE_TPCH=true cargo test --test sqllogictests
       - name: Verify Working Directory Clean
         run: git diff --exit-code
 
@@ -270,7 +270,7 @@ jobs:
           rustup toolchain install stable
           rustup default stable
       - name: Run sqllogictest
-        run: PG_COMPAT=true PG_URI="postgresql://postgres:postgres@localhost:$POSTGRES_PORT/db_test" cargo test -p datafusion --test sqllogictests
+        run: PG_COMPAT=true PG_URI="postgresql://postgres:postgres@localhost:$POSTGRES_PORT/db_test" cargo test --features=postgres --test sqllogictests
         env:
           POSTGRES_PORT: ${{ job.services.postgres.ports[5432] }}
 

datafusion/core/Cargo.toml

@@ -102,7 +102,6 @@ bigdecimal = "0.4.1"
 criterion = { version = "0.5", features = ["async_tokio"] }
 csv = "1.1.6"
 ctor = "0.2.0"
-datafusion-sqllogictest = { path = "../sqllogictest", version = "29.0.0", features = ["postgres"] }
 doc-comment = "0.3"
 env_logger = "0.10"
 half = "2.2.1"
@@ -111,7 +110,6 @@ postgres-types = { version = "0.2.4", features = ["derive", "with-chrono-0_4"] }
 regex = "1.5.4"
 rstest = "0.18.0"
 rust_decimal = { version = "1.27.0", features = ["tokio-pg"] }
-sqllogictest = "0.15.0"
 test-utils = { path = "../../test-utils" }
 thiserror = "1.0.37"
 tokio-postgres = "0.7.7"
@@ -161,8 +159,3 @@ name = "sql_query_with_io"
 [[bench]]
 harness = false
 name = "sort"
-
-[[test]]
-harness = false
-name = "sqllogictests"
-path = "tests/sqllogictests/src/main.rs"

datafusion/core/tests/sqllogictests/.gitignore

@@ -0,0 +1,20 @@
+<!---
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+-->
+
+The SQL Logic Test code has moved to `datafusion/sqllogictest`

datafusion/core/tests/sqllogictests/MOVED.md

@@ -0,0 +1,2 @@
+*.py
+test_files/tpch/data

datafusion/sqllogictest/.gitignore

@@ -44,14 +44,25 @@ rust_decimal = {version = "1.27.0"}
 log = "^0.4"
 sqllogictest = "0.15.0"
 sqlparser.workspace = true
+tempfile = "3"
 thiserror = "1.0.44"
 tokio = {version = "1.0"}
 bytes = {version = "1.4.0", optional = true}
-futures = {version = "0.3.28", optional = true}
+futures = {version = "0.3.28"}
 chrono = {version = "0.4.26", optional = true}
 tokio-postgres = {version = "0.7.7", optional = true}
 postgres-types = {version = "0.2.4", optional = true}
 postgres-protocol = {version = "0.6.4", optional = true}
 
 [features]
-postgres = ["bytes", "futures", "chrono", "tokio-postgres", "postgres-types", "postgres-protocol"]
+postgres = ["bytes", "chrono", "tokio-postgres", "postgres-types", "postgres-protocol"]
+
+[dev-dependencies]
+env_logger = "0.10"
+num_cpus = "1.13.0"
+
+
+[[test]]
+harness = false
+name = "sqllogictests"
+path = "bin/sqllogictests.rs"

datafusion/sqllogictest/Cargo.toml

@@ -33,27 +33,27 @@ in [`test_files`](test_files).
 
 ```shell
 # Run all tests
-cargo test -p datafusion --test sqllogictests
+cargo test --test sqllogictests
 ```
 
 ```shell
 # Run all tests, with debug logging enabled
-RUST_LOG=debug cargo test -p datafusion --test sqllogictests
+RUST_LOG=debug cargo test --test sqllogictests
 ```
 
 ```shell
 # Run only the tests in `information_schema.slt`
-cargo test -p datafusion --test sqllogictests -- information_schema
+cargo test --test sqllogictests -- information_schema
 ```
 
 ```shell
 # Automatically update ddl.slt with expected output
-cargo test -p datafusion --test sqllogictests -- ddl --complete
+cargo test --test sqllogictests -- ddl --complete
 ```
 
 ```shell
 # Run ddl.slt, printing debug logging to stdout
-RUST_LOG=debug cargo test -p datafusion --test sqllogictests -- ddl
+RUST_LOG=debug cargo test --test sqllogictests -- ddl
 ```
 
 #### Cookbook: Adding Tests
@@ -76,7 +76,7 @@ SELECT * from foo;
 Running the following command will update `my_awesome_test.slt` with the expected output:
 
 ```shell
-cargo test -p datafusion --test sqllogictests -- my_awesome_test --complete
+cargo test --test sqllogictests -- my_awesome_test --complete
 ```
 
 3. Verify the content
@@ -105,14 +105,14 @@ file to the output produced by that run.
 For example, to run all tests suites in validation mode
 
 ```shell
-cargo test -p datafusion --test sqllogictests
+cargo test --test sqllogictests
 ```
 
 sqllogictests also supports `cargo test` style substring matches on file names to restrict which tests to run
 
 ```shell
 # information_schema.slt matches due to substring matching `information`
-cargo test -p datafusion --test sqllogictests -- information
+cargo test --test sqllogictests -- information
 ```
 
 #### Running tests: Postgres compatibility
@@ -123,7 +123,7 @@ with Postgres by running the same script files both with DataFusion and with Pos
 In order to run the sqllogictests running against a previously running Postgres instance, do:
 
 ```shell
-PG_COMPAT=true PG_URI="postgresql://[email protected]/postgres" cargo test -p datafusion --test sqllogictests
+PG_COMPAT=true PG_URI="postgresql://[email protected]/postgres" cargo test --features=postgres --test sqllogictests
 ```
 
 The environemnt variables:
@@ -153,15 +153,16 @@ command to generate tpch data, assuming you are in the repository
 root:
 
 ```shell
+mkdir -p datafusion/sqllogictest/test_files/tpch/data
 docker run -it \
-  -v "$(realpath datafusion/core/tests/sqllogictests/test_files/tpch/data)":/data \
+  -v "$(realpath datafusion/sqllogictest/test_files/tpch/data)":/data \
   ghcr.io/databloom-ai/tpch-docker:main -vf -s 0.1
 ```
 
 Then you need to add `INCLUDE_TPCH=true` to run tpch tests:
 
 ```shell
-INCLUDE_TPCH=true cargo test -p datafusion --test sqllogictests
+INCLUDE_TPCH=true cargo test --test sqllogictests
 ```
 
 #### Updating tests: Completion Mode
@@ -173,7 +174,7 @@ You can update the tests / generate expected output by passing the `--complete`
 
 ```shell
 # Update ddl.slt with output from running
-cargo test -p datafusion --test sqllogictests -- ddl --complete
+cargo test --test sqllogictests -- ddl --complete
 ```
 
 #### sqllogictests

datafusion/core/tests/sqllogictests/README.md renamed to datafusion/sqllogictest/README.md

@@ -20,18 +20,14 @@ use std::path::{Path, PathBuf};
 #[cfg(target_family = "windows")]
 use std::thread;
 
-use datafusion_sqllogictest::{DataFusion, Postgres};
+use datafusion_sqllogictest::{DataFusion, TestContext};
 use futures::stream::StreamExt;
 use log::info;
 use sqllogictest::strict_column_validator;
-use tempfile::TempDir;
 
-use datafusion::prelude::{SessionConfig, SessionContext};
 use datafusion_common::{DataFusionError, Result};
 
-mod setup;
-
-const TEST_DIRECTORY: &str = "tests/sqllogictests/test_files/";
+const TEST_DIRECTORY: &str = "test_files/";
 const PG_COMPAT_FILE_PREFIX: &str = "pg_compat_";
 
 #[cfg(target_family = "windows")]
@@ -121,7 +117,7 @@ async fn run_test_file(test_file: TestFile) -> Result<()> {
         relative_path,
     } = test_file;
     info!("Running with DataFusion runner: {}", path.display());
-    let Some(test_ctx) = context_for_test_file(&relative_path).await else {
+    let Some(test_ctx) = TestContext::try_new_for_test_file(&relative_path).await else {
         info!("Skipping: {}", path.display());
         return Ok(());
     };
@@ -138,7 +134,9 @@ async fn run_test_file(test_file: TestFile) -> Result<()> {
         .map_err(|e| DataFusionError::External(Box::new(e)))
 }
 
+#[cfg(feature = "postgres")]
 async fn run_test_file_with_postgres(test_file: TestFile) -> Result<()> {
+    use datafusion_sqllogictest::Postgres;
     let TestFile {
         path,
         relative_path,
@@ -154,6 +152,12 @@ async fn run_test_file_with_postgres(test_file: TestFile) -> Result<()> {
     Ok(())
 }
 
+#[cfg(not(feature = "postgres"))]
+async fn run_test_file_with_postgres(_test_file: TestFile) -> Result<()> {
+    use datafusion_common::plan_err;
+    plan_err!("Can not run with postgres as postgres feature is not enabled")
+}
+
 async fn run_complete_file(test_file: TestFile) -> Result<()> {
     let TestFile {
         path,
@@ -163,7 +167,7 @@ async fn run_complete_file(test_file: TestFile) -> Result<()> {
 
     info!("Using complete mode to complete: {}", path.display());
 
-    let Some(test_ctx) = context_for_test_file(&relative_path).await else {
+    let Some(test_ctx) = TestContext::try_new_for_test_file(&relative_path).await else {
         info!("Skipping: {}", path.display());
         return Ok(());
     };
@@ -250,96 +254,6 @@ fn read_dir_recursive<P: AsRef<Path>>(path: P) -> Box<dyn Iterator<Item = PathBu
     )
 }
 
-/// Create a SessionContext, configured for the specific test, if
-/// possible.
-///
-/// If `None` is returned (e.g. because some needed feature is not
-/// enabled), the file should be skipped
-async fn context_for_test_file(relative_path: &Path) -> Option<TestContext> {
-    let config = SessionConfig::new()
-        // hardcode target partitions so plans are deterministic
-        .with_target_partitions(4);
-
-    let test_ctx = TestContext::new(SessionContext::with_config(config));
-
-    let file_name = relative_path.file_name().unwrap().to_str().unwrap();
-    match file_name {
-        "scalar.slt" => {
-            info!("Registering scalar tables");
-            setup::register_scalar_tables(test_ctx.session_ctx()).await;
-        }
-        "information_schema_table_types.slt" => {
-            info!("Registering local temporary table");
-            setup::register_temp_table(test_ctx.session_ctx()).await;
-        }
-        "information_schema_columns.slt" => {
-            info!("Registering table with many types");
-            setup::register_table_with_many_types(test_ctx.session_ctx()).await;
-        }
-        "avro.slt" => {
-            #[cfg(feature = "avro")]
-            {
-                let mut test_ctx = test_ctx;
-                info!("Registering avro tables");
-                setup::register_avro_tables(&mut test_ctx).await;
-                return Some(test_ctx);
-            }
-            #[cfg(not(feature = "avro"))]
-            {
-                info!("Skipping {file_name} because avro feature is not enabled");
-                return None;
-            }
-        }
-        "joins.slt" => {
-            info!("Registering partition table tables");
-
-            let mut test_ctx = test_ctx;
-            setup::register_partition_table(&mut test_ctx).await;
-            return Some(test_ctx);
-        }
-        _ => {
-            info!("Using default SessionContext");
-        }
-    };
-    Some(test_ctx)
-}
-
-/// Context for running tests
-pub struct TestContext {
-    /// Context for running queries
-    ctx: SessionContext,
-    /// Temporary directory created and cleared at the end of the test
-    test_dir: Option<TempDir>,
-}
-
-impl TestContext {
-    pub fn new(ctx: SessionContext) -> Self {
-        Self {
-            ctx,
-            test_dir: None,
-        }
-    }
-
-    /// Enables the test directory feature. If not enabled,
-    /// calling `testdir_path` will result in a panic.
-    pub fn enable_testdir(&mut self) {
-        if self.test_dir.is_none() {
-            self.test_dir = Some(TempDir::new().expect("failed to create testdir"));
-        }
-    }
-
-    /// Returns the path to the test directory. Panics if the test
-    /// directory feature is not enabled via `enable_testdir`.
-    pub fn testdir_path(&self) -> &Path {
-        self.test_dir.as_ref().expect("testdir not enabled").path()
-    }
-
-    /// Returns a reference to the internal SessionContext
-    fn session_ctx(&self) -> &SessionContext {
-        &self.ctx
-    }
-}
-
 /// Parsed command line options
 struct Options {
     // regex like

datafusion/core/tests/sqllogictests/src/main.rs renamed to datafusion/sqllogictest/bin/sqllogictests.rs

@@ -82,6 +82,7 @@ pub(crate) fn i128_to_str(value: i128, precision: &u8, scale: &i8) -> String {
     )
 }
 
+#[cfg(feature = "postgres")]
 pub(crate) fn decimal_to_str(value: Decimal) -> String {
     big_decimal_to_str(BigDecimal::from_str(&value.to_string()).unwrap())
 }

datafusion/sqllogictest/src/engines/conversion.rs

@@ -15,9 +15,14 @@
 // specific language governing permissions and limitations
 // under the License.
 
+//! DataFusion sqllogictest driver
+
 mod engines;
 
 pub use engines::DataFusion;
 
 #[cfg(feature = "postgres")]
 pub use engines::Postgres;
+
+mod test_context;
+pub use test_context::TestContext;