refactor: do not rebuild query_test when .sql or .expected files change (#2816)

* feat: Do not rebuild query_tests if .sql or .expected change

* feat: Add CI check

* refactor: move some sql tests to .sql files

* tests: port tests / expected results to data files

* fix: restore old name check-flatbuffers

.circleci/config.yml

@@ -288,7 +288,8 @@ jobs:
           name: buf lint
           command: buf lint
 
-  # Check that the generated flatbuffers code is up-to-date with the changes in this PR.
+  # Check that any generated files are  is up-to-date with the changes in this PR.
+  # named "check-flatbuffers" because that name is hardcoded into github checks
   check-flatbuffers:
     docker:
       - image: quay.io/influxdb/rust:ci
@@ -296,6 +297,9 @@ jobs:
     steps:
       - checkout
       - rust_components # Regenerating flatbuffers uses rustfmt
+      - run:
+          name: Check Query Tests
+          command: ./query_tests/check-generated.sh
       - run:
           name: Check Flatbuffers
           command: INFLUXDB_IOX_INTEGRATION_LOCAL=1 ./entry/check-flatbuffers.sh

query_tests/cases/in/basic.expected

@@ -0,0 +1,39 @@
+-- Test Setup: TwoMeasurements
+-- SQL: SELECT * from cpu;
++--------+--------------------------------+------+
+| region | time                           | user |
++--------+--------------------------------+------+
+| west   | 1970-01-01T00:00:00.000000100Z | 23.2 |
+| west   | 1970-01-01T00:00:00.000000150Z | 21   |
++--------+--------------------------------+------+
+-- SQL: SELECT user, region from cpu;
++------+--------+
+| user | region |
++------+--------+
+| 23.2 | west   |
+| 21   | west   |
++------+--------+
+-- SQL: SELECT * from cpu where time > to_timestamp('1970-01-01T00:00:00.000000120+00:00');
++--------+--------------------------------+------+
+| region | time                           | user |
++--------+--------------------------------+------+
+| west   | 1970-01-01T00:00:00.000000150Z | 21   |
++--------+--------------------------------+------+
+-- SQL: SELECT user, region from cpu where time > to_timestamp('1970-01-01T00:00:00.000000120+00:00');
++------+--------+
+| user | region |
++------+--------+
+| 21   | west   |
++------+--------+
+-- SQL: SELECT count(*) from cpu group by region;
++-----------------+
+| COUNT(UInt8(1)) |
++-----------------+
+| 2               |
++-----------------+
+-- SQL: SELECT * from disk;
++-------+--------+--------------------------------+
+| bytes | region | time                           |
++-------+--------+--------------------------------+
+| 99    | east   | 1970-01-01T00:00:00.000000200Z |
++-------+--------+--------------------------------+

query_tests/cases/in/basic.sql

@@ -0,0 +1,32 @@
+-- Basic query tests
+-- IOX_SETUP: TwoMeasurements
+
+-- query data
+SELECT * from cpu;
+
+
+-- BUG: https://github.com/influxdata/influxdb_iox/issues/2776
+--        "+----------------+",
+--        "| MIN(cpu.region |",
+--        "+----------------+",
+--        "| west           |",
+--        "+----------------+",
+--SELECT min(region) from cpu;
+
+-- projection
+-- expect that to get a subset of the columns and in the order specified
+SELECT user, region from cpu;
+
+-- predicate on CPU
+SELECT * from cpu where time > to_timestamp('1970-01-01T00:00:00.000000120+00:00');
+
+-- projection and predicate
+-- expect that to get a subset of the columns and in the order specified
+SELECT user, region from cpu where time > to_timestamp('1970-01-01T00:00:00.000000120+00:00');
+
+-- basic grouping
+SELECT count(*) from cpu group by region;
+
+
+-- select from a different measurement
+SELECT * from disk;

query_tests/cases/in/timestamps.expected

@@ -0,0 +1,26 @@
+-- Test Setup: OneMeasurementRealisticTimes
+-- SQL: SELECT * from cpu;
++--------+----------------------+------+
+| region | time                 | user |
++--------+----------------------+------+
+| west   | 2021-07-20T19:28:50Z | 23.2 |
+| west   | 2021-07-20T19:30:30Z | 21   |
++--------+----------------------+------+
+-- SQL: SELECT * FROM cpu WHERE time  > to_timestamp('2021-07-20 19:28:50+00:00');
++--------+----------------------+------+
+| region | time                 | user |
++--------+----------------------+------+
+| west   | 2021-07-20T19:30:30Z | 21   |
++--------+----------------------+------+
+-- SQL: SELECT * FROM cpu WHERE time  > to_timestamp('2021-07-20T19:28:50Z');
++--------+----------------------+------+
+| region | time                 | user |
++--------+----------------------+------+
+| west   | 2021-07-20T19:30:30Z | 21   |
++--------+----------------------+------+
+-- SQL: SELECT * FROM cpu WHERE CAST(time AS BIGINT) > CAST(to_timestamp('2021-07-20T19:28:50Z') AS BIGINT);
++--------+----------------------+------+
+| region | time                 | user |
++--------+----------------------+------+
+| west   | 2021-07-20T19:30:30Z | 21   |
++--------+----------------------+------+

query_tests/cases/in/timestamps.sql

@@ -0,0 +1,12 @@
+-- Timestamp printing / output testss
+-- IOX_SETUP: OneMeasurementRealisticTimes
+
+-- Expect the timestamp output to be formatted correctly (with `Z`)
+SELECT * from cpu;
+-- explicit offset format
+SELECT * FROM cpu WHERE time  > to_timestamp('2021-07-20 19:28:50+00:00');
+-- Use RCF3339 format
+SELECT * FROM cpu WHERE time  > to_timestamp('2021-07-20T19:28:50Z');
+--use cast workaround
+SELECT * FROM cpu WHERE
+  CAST(time AS BIGINT) > CAST(to_timestamp('2021-07-20T19:28:50Z') AS BIGINT);

query_tests/check-generated.sh

@@ -0,0 +1,26 @@
+#!/bin/bash -eu
+
+# Change to the query_tests crate directory, where this script is located
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
+pushd $DIR
+
+echo "Regenerating query_tests..."
+
+(cd generate && cargo run)
+
+echo "Checking for uncommitted changes..."
+
+if ! git diff --quiet HEAD --; then
+  echo "git diff found:"
+  git diff HEAD
+  echo "************************************************************"
+  echo "* Found uncommitted changes to generated flatbuffers code! *"
+  echo "* Please do:"
+  echo "*  cd query_tests/generate"
+  echo "*  cargo run"
+  echo "* to regenerate the query_tests code and check it in!      *"
+  echo "************************************************************"
+  exit 1
+else
+  echo "No uncommitted changes; everything is awesome."
+fi

query_tests/generate/Cargo.lock

@@ -0,0 +1,7 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "generate"
+version = "0.1.0"

query_tests/generate/Cargo.toml

@@ -0,0 +1,11 @@
+[package]
+name = "generate"
+description = "Creates rust #tests for files in .sql"
+version = "0.1.0"
+authors = ["Andrew Lamb <andrew@nerdnetworks.org>"]
+edition = "2018"
+
+[dependencies] # In alphabetical order
+
+# Note this is a standalone binary and not part of the overall workspace
+[workspace]

query_tests/generate/src/main.rs

@@ -1,26 +1,41 @@
-//! Finds all .sql files in `cases/in/` and creates corresponding entries in src/cases.rs
-//! native Rust types.
+//! Finds all .sql files in `cases/in/` and creates corresponding
+//! entries in src/cases.rs as native Rust test runner tests
 
+use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
 
 type Error = Box<dyn std::error::Error>;
 type Result<T, E = Error> = std::result::Result<T, E>;
 
 fn main() -> Result<()> {
+    // Ignores all args and finds relative paths based on PWD and the command
+
+    // example: query_tests/generate/target/debug/generate
+    let current_exe = std::env::current_exe()?;
+
+    // walk up parent tree looking for query_tests
+    let mut query_tests = current_exe.clone();
+    let needle = OsStr::new("query_tests");
+    loop {
+        if query_tests.file_name() == Some(&needle) {
+            break;
+        }
+        if !query_tests.pop() {
+            panic!("Can not find 'query_tests' in the path: {:?}", current_exe);
+        }
+    }
+
     // crate root
-    let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
-    let cases = root.join("cases").join("in");
+    let cases = query_tests.join("cases").join("in");
 
     let sql_files = find_sql_files(&cases);
 
-    // Tell cargo to recompile if anything in the cases directory changes
-    println!("cargo:rerun-if-changed={}", cases.display());
-
     // Now create the generated sql file
     let output_content = make_cases_rs(&sql_files).join("\n");
-    let output_file = root.join("src").join("cases.rs");
+    let output_file = query_tests.join("src").join("cases.rs");
     write_if_changed(&output_file, &output_content);
 
+    println!("Done");
     Ok(())
 }
 
@@ -94,6 +109,8 @@ fn write_if_changed(path: &Path, content: &str) {
     };
 
     if changed {
+        println!("Writing changes to {}", path.display());
+
         std::fs::write(path, content)
             .map_err(|e| format!("Error writing to {:?}: {}", path, e))
             .unwrap();

query_tests/src/cases.rs

@@ -18,6 +18,20 @@ async fn test_cases_all_chunks_dropped_sql() {
         .expect("flush worked");
 }
 
+#[tokio::test]
+// Tests from "basic.sql",
+async fn test_cases_basic_sql() {
+    let input_path = Path::new("cases").join("in").join("basic.sql");
+    let mut runner = Runner::new();
+    runner
+        .run(input_path)
+        .await
+        .expect("test failed");
+    runner
+        .flush()
+        .expect("flush worked");
+}
+
 #[tokio::test]
 // Tests from "chunk_order.sql",
 async fn test_cases_chunk_order_sql() {
@@ -86,4 +100,18 @@ async fn test_cases_stats_plans_sql() {
     runner
         .flush()
         .expect("flush worked");
+}
+
+#[tokio::test]
+// Tests from "timestamps.sql",
+async fn test_cases_timestamps_sql() {
+    let input_path = Path::new("cases").join("in").join("timestamps.sql");
+    let mut runner = Runner::new();
+    runner
+        .run(input_path)
+        .await
+        .expect("test failed");
+    runner
+        .flush()
+        .expect("flush worked");
 }

query_tests/src/scenarios.rs

@@ -57,6 +57,7 @@ pub fn get_all_setups() -> &'static HashMap<String, Arc<dyn DbSetup>> {
             register_setup!(OneMeasurementAllChunksDropped),
             register_setup!(ChunkOrder),
             register_setup!(ThreeDeleteThreeChunks),
+            register_setup!(OneMeasurementRealisticTimes),
         ]
         .into_iter()
         .map(|(name, setup)| (name.to_string(), setup as Arc<dyn DbSetup>))

query_tests/src/sql.rs

@@ -39,168 +39,6 @@ where
     }
 }
 
-#[tokio::test]
-async fn sql_select_from_cpu() {
-    let expected = vec![
-        "+--------+--------------------------------+------+",
-        "| region | time                           | user |",
-        "+--------+--------------------------------+------+",
-        "| west   | 1970-01-01T00:00:00.000000100Z | 23.2 |",
-        "| west   | 1970-01-01T00:00:00.000000150Z | 21   |",
-        "+--------+--------------------------------+------+",
-    ];
-    run_sql_test_case(TwoMeasurements {}, "SELECT * from cpu", &expected).await;
-}
-
-// BUG: https://github.com/influxdata/influxdb_iox/issues/2776
-#[ignore]
-#[tokio::test]
-async fn sql_select_from_cpu_min_utf8() {
-    let expected = vec![
-        "+----------------+",
-        "| MIN(cpu.region |",
-        "+----------------+",
-        "| west           |",
-        "+----------------+",
-    ];
-    run_sql_test_case(TwoMeasurements {}, "SELECT min(region) from cpu", &expected).await;
-}
-
-#[tokio::test]
-async fn sql_select_from_cpu_2021() {
-    let expected = vec![
-        "+--------+----------------------+------+",
-        "| region | time                 | user |",
-        "+--------+----------------------+------+",
-        "| west   | 2021-07-20T19:28:50Z | 23.2 |",
-        "| west   | 2021-07-20T19:30:30Z | 21   |",
-        "+--------+----------------------+------+",
-    ];
-    run_sql_test_case(
-        OneMeasurementRealisticTimes {},
-        "SELECT * from cpu",
-        &expected,
-    )
-    .await;
-}
-
-#[tokio::test]
-async fn sql_select_from_cpu_with_timestamp_predicate_explicit_utc() {
-    let expected = vec![
-        "+--------+----------------------+------+",
-        "| region | time                 | user |",
-        "+--------+----------------------+------+",
-        "| west   | 2021-07-20T19:30:30Z | 21   |",
-        "+--------+----------------------+------+",
-    ];
-
-    run_sql_test_case(
-        OneMeasurementRealisticTimes {},
-        "SELECT * FROM cpu WHERE time  > to_timestamp('2021-07-20 19:28:50+00:00')",
-        &expected,
-    )
-    .await;
-
-    // Use RCF3339 format
-    run_sql_test_case(
-        OneMeasurementRealisticTimes {},
-        "SELECT * FROM cpu WHERE time  > to_timestamp('2021-07-20T19:28:50Z')",
-        &expected,
-    )
-    .await;
-
-    // use cast workaround
-    run_sql_test_case(
-        OneMeasurementRealisticTimes {},
-        "SELECT * FROM cpu WHERE \
-         CAST(time AS BIGINT) > CAST(to_timestamp('2021-07-20T19:28:50Z') AS BIGINT)",
-        &expected,
-    )
-    .await;
-}
-
-#[tokio::test]
-async fn sql_select_from_cpu_with_projection() {
-    // expect that to get a subset of the columns and in the order specified
-    let expected = vec![
-        "+------+--------+",
-        "| user | region |",
-        "+------+--------+",
-        "| 23.2 | west   |",
-        "| 21   | west   |",
-        "+------+--------+",
-    ];
-    run_sql_test_case(
-        TwoMeasurements {},
-        "SELECT user, region from cpu",
-        &expected,
-    )
-    .await;
-}
-
-#[tokio::test]
-async fn sql_select_from_cpu_pred() {
-    let expected = vec![
-        "+--------+--------------------------------+------+",
-        "| region | time                           | user |",
-        "+--------+--------------------------------+------+",
-        "| west   | 1970-01-01T00:00:00.000000150Z | 21   |",
-        "+--------+--------------------------------+------+",
-    ];
-    run_sql_test_case(
-        TwoMeasurements {},
-        "SELECT * from cpu where time > to_timestamp('1970-01-01T00:00:00.000000120+00:00')",
-        &expected,
-    )
-    .await;
-}
-
-#[tokio::test]
-async fn sql_select_from_cpu_with_projection_and_pred() {
-    // expect that to get a subset of the columns and in the order specified
-    let expected = vec![
-        "+------+--------+",
-        "| user | region |",
-        "+------+--------+",
-        "| 21   | west   |",
-        "+------+--------+",
-    ];
-    run_sql_test_case(
-        TwoMeasurements {},
-        "SELECT user, region from cpu where time > to_timestamp('1970-01-01T00:00:00.000000120+00:00')",
-        &expected
-    ).await;
-}
-
-#[tokio::test]
-async fn sql_select_from_cpu_group() {
-    let expected = vec![
-        "+-----------------+",
-        "| COUNT(UInt8(1)) |",
-        "+-----------------+",
-        "| 2               |",
-        "+-----------------+",
-    ];
-    run_sql_test_case(
-        TwoMeasurements {},
-        "SELECT count(*) from cpu group by region",
-        &expected,
-    )
-    .await;
-}
-
-#[tokio::test]
-async fn sql_select_from_disk() {
-    let expected = vec![
-        "+-------+--------+--------------------------------+",
-        "| bytes | region | time                           |",
-        "+-------+--------+--------------------------------+",
-        "| 99    | east   | 1970-01-01T00:00:00.000000200Z |",
-        "+-------+--------+--------------------------------+",
-    ];
-    run_sql_test_case(TwoMeasurements {}, "SELECT * from disk", &expected).await;
-}
-
 #[tokio::test]
 async fn sql_select_with_schema_merge() {
     let expected = vec![