From 569d8b47904efd52316e1fed975fc2fc7bf9a86e Mon Sep 17 00:00:00 2001 From: "Paul J. Davis" Date: Fri, 16 Aug 2024 17:03:32 -0500 Subject: [PATCH] Implement multiline query output This is a first pass at adding support for matching query output line by line without attempting to parse column types and values. This just relies on the database driver to return DBOutput::MultiLine variants. Ideally we could inform the driver on every SQL run, but I think that's part of a bigger refactor where we refactor the DB and AsyncDB traits so that they're aware of whether we're expecting statement or results output. Signed-off-by: Paul J. Davis --- sqllogictest/src/parser.rs | 26 ++++++ sqllogictest/src/runner.rs | 165 ++++++++++++++++++++++++++++++++++ tests/harness.rs | 14 +++ tests/slt/multiline_query.slt | 10 +++ 4 files changed, 215 insertions(+) create mode 100644 tests/slt/multiline_query.slt diff --git a/sqllogictest/src/parser.rs b/sqllogictest/src/parser.rs index e1d33bd..b4da675 100644 --- a/sqllogictest/src/parser.rs +++ b/sqllogictest/src/parser.rs @@ -88,6 +88,9 @@ pub enum QueryExpect { label: Option, results: Vec, }, + /// Query output can be returned as a multiline output to accommodate + /// matching tabular output directly. + MultiLine { data: Vec }, /// Query should fail with the given error message. Error(ExpectedError), } @@ -245,6 +248,9 @@ impl std::fmt::Display for Record { write!(f, " {label}")?; } } + QueryExpect::MultiLine { .. } => { + write!(f, "multiline")?; + } QueryExpect::Error(err) => err.fmt_inline(f)?, } writeln!(f)?; @@ -260,6 +266,12 @@ impl std::fmt::Display for Record { // query always ends with a blank line writeln!(f)? } + QueryExpect::MultiLine { data } => { + writeln!(f, "{}", RESULTS_DELIMITER)?; + for line in data { + writeln!(f, "{}", line)?; + } + } QueryExpect::Error(err) => err.fmt_multiline(f)?, } Ok(()) @@ -737,6 +749,7 @@ fn parse_inner(loc: &Location, script: &str) -> Result QueryExpect::MultiLine { data: Vec::new() }, [type_str, res @ ..] => { let types = type_str .chars() @@ -776,6 +789,14 @@ fn parse_inner(loc: &Location, script: &str) -> Result { + for (_, line) in &mut lines { + if line.is_empty() { + break; + } + data.push(line.to_string()) + } + } // If no inline error message is specified, it might be a multiline error. QueryExpect::Error(e) => { if e.is_empty() { @@ -988,6 +1009,11 @@ mod tests { parse_roundtrip::("../tests/slt/rowsort.slt") } + #[test] + fn test_multiline_query() { + parse_roundtrip::("../tests/slt/multiline_query.slt") + } + #[test] fn test_substitution() { parse_roundtrip::("../tests/substitution/basic.slt") diff --git a/sqllogictest/src/runner.rs b/sqllogictest/src/runner.rs index 78fb250..2c5c719 100644 --- a/sqllogictest/src/runner.rs +++ b/sqllogictest/src/runner.rs @@ -36,6 +36,10 @@ pub enum RecordOutput { rows: Vec>, error: Option, }, + MultiLineQuery { + lines: Vec, + error: Option, + }, /// The output of a `statement`. Statement { count: u64, error: Option }, /// The output of a `system` command. @@ -52,6 +56,9 @@ pub enum DBOutput { types: Vec, rows: Vec>, }, + MultiLine { + lines: Vec, + }, /// A statement in the query has completed. /// /// The number of rows modified or selected is returned. @@ -293,6 +300,8 @@ pub enum TestErrorKind { expected: String, actual: String, }, + #[error("query result type mismatch:\n[SQL] {sql}\nThe query expected {expected} output")] + QueryResultTypeMismatch { sql: String, expected: String }, #[error( "query columns mismatch:\n[SQL] {sql}\n{}", format_column_diff(expected, actual, false) @@ -607,6 +616,9 @@ impl> Runner { rows, error: None, }, + DBOutput::MultiLine { lines } => { + RecordOutput::MultiLineQuery { lines, error: None } + } DBOutput::StatementComplete(count) => { RecordOutput::Statement { count, error: None } } @@ -751,6 +763,9 @@ impl> Runner { let (types, mut rows) = match conn.run(&sql).await { Ok(out) => match out { DBOutput::Rows { types, rows } => (types, rows), + DBOutput::MultiLine { lines } => { + return RecordOutput::MultiLineQuery { lines, error: None } + } DBOutput::StatementComplete(count) => { return RecordOutput::Statement { count, error: None }; } @@ -766,6 +781,7 @@ impl> Runner { let sort_mode = match expected { QueryExpect::Results { sort_mode, .. } => sort_mode, + QueryExpect::MultiLine { .. } => None, QueryExpect::Error(_) => None, } .or(self.sort_mode); @@ -881,6 +897,14 @@ impl> Runner { .at(loc)) } QueryExpect::Results { .. } => {} + QueryExpect::MultiLine { data } => { + return Err(TestErrorKind::QueryResultMismatch { + sql, + expected: data.join("\n"), + actual: "".to_string(), + } + .at(loc)); + } }, ( Record::Statement { @@ -959,6 +983,14 @@ impl> Runner { .at(loc)); } } + (Some(e), QueryExpect::MultiLine { .. }) => { + return Err(TestErrorKind::Fail { + sql, + err: Arc::clone(e), + kind: RecordKind::Query, + } + .at(loc)) + } (Some(e), QueryExpect::Results { .. }) => { return Err(TestErrorKind::Fail { sql, @@ -967,6 +999,13 @@ impl> Runner { } .at(loc)); } + (None, QueryExpect::MultiLine { .. }) => { + return Err(TestErrorKind::QueryResultTypeMismatch { + sql, + expected: "multiline".to_string(), + } + .at(loc)); + } ( None, QueryExpect::Results { @@ -997,6 +1036,72 @@ impl> Runner { } }; } + ( + Record::Query { + loc, + conditions: _, + connection: _, + sql, + expected, + }, + RecordOutput::MultiLineQuery { lines, error }, + ) => { + match (error, expected) { + (None, QueryExpect::Error(_)) => { + return Err(TestErrorKind::Ok { + sql, + kind: RecordKind::Query, + } + .at(loc)); + } + (Some(e), QueryExpect::Error(expected_error)) => { + if !expected_error.is_match(&e.to_string()) { + return Err(TestErrorKind::ErrorMismatch { + sql, + err: Arc::clone(e), + expected_err: expected_error.to_string(), + kind: RecordKind::Query, + } + .at(loc)); + } + } + (Some(e), QueryExpect::MultiLine { .. }) => { + return Err(TestErrorKind::Fail { + sql, + err: Arc::clone(e), + kind: RecordKind::Query, + } + .at(loc)) + } + (Some(e), QueryExpect::Results { .. }) => { + return Err(TestErrorKind::Fail { + sql, + err: Arc::clone(e), + kind: RecordKind::Query, + } + .at(loc)); + } + (None, QueryExpect::MultiLine { data }) => { + for (actual, expected) in lines.iter().zip(data.iter()) { + if actual != expected { + return Err(TestErrorKind::QueryResultMismatch { + sql, + expected: data.join("\n"), + actual: lines.join("\n"), + } + .at(loc)); + } + } + } + (None, QueryExpect::Results { .. }) => { + return Err(TestErrorKind::QueryResultTypeMismatch { + sql, + expected: "result".to_string(), + } + .at(loc)); + } + }; + } ( Record::System { loc, @@ -1481,6 +1586,7 @@ pub fn update_record_with_output( (Some(e), r) => { let reference = match &r { QueryExpect::Error(e) => Some(e), + QueryExpect::MultiLine { .. } => None, QueryExpect::Results { .. } => None, }; Some(Record::Query { @@ -1525,6 +1631,12 @@ pub fn update_record_with_output( sort_mode, label, }, + QueryExpect::MultiLine { .. } => QueryExpect::Results { + results, + types, + sort_mode: None, + label: None, + }, QueryExpect::Error(_) => QueryExpect::Results { results, types, @@ -1535,6 +1647,59 @@ pub fn update_record_with_output( }) } }, + // query, multiline query + ( + Record::Query { + loc, + conditions, + connection, + sql, + expected, + }, + RecordOutput::MultiLineQuery { lines, error }, + ) => match (error, expected) { + // Error match + (Some(e), QueryExpect::Error(expected_error)) + if expected_error.is_match(&e.to_string()) => + { + None + } + // Error mismatch + (Some(e), r) => { + let reference = match &r { + QueryExpect::Error(e) => Some(e), + QueryExpect::MultiLine { .. } => None, + QueryExpect::Results { .. } => None, + }; + Some(Record::Query { + sql, + expected: QueryExpect::Error(ExpectedError::from_actual_error( + reference, + &e.to_string(), + )), + loc, + conditions, + connection, + }) + } + (None, expected) => Some(Record::Query { + sql, + loc, + conditions, + connection, + expected: match expected { + QueryExpect::Results { .. } => QueryExpect::MultiLine { + data: lines.clone(), + }, + QueryExpect::MultiLine { .. } => QueryExpect::MultiLine { + data: lines.clone(), + }, + QueryExpect::Error(_) => QueryExpect::MultiLine { + data: lines.clone(), + }, + }, + }), + }, ( Record::System { loc, diff --git a/tests/harness.rs b/tests/harness.rs index cd42e99..59db977 100644 --- a/tests/harness.rs +++ b/tests/harness.rs @@ -54,6 +54,20 @@ impl sqllogictest::DB for FakeDB { ], }); } + if sql == "select * from example_multiline" { + // Check multi-line output return values + return Ok(DBOutput::MultiLine { + lines: vec![ + "+---+---+---+".to_string(), + "| a | b | c |".to_string(), + "+---+---+---+".to_string(), + "| 1 | 2 | 3 |".to_string(), + "| 4 | 5 | 6 |".to_string(), + "| 7 | 8 | 9 |".to_string(), + "+---+---+---+".to_string(), + ], + }); + } if sql == "select counter()" { self.counter += 1; return Ok(DBOutput::Rows { diff --git a/tests/slt/multiline_query.slt b/tests/slt/multiline_query.slt new file mode 100644 index 0000000..ad2e87e --- /dev/null +++ b/tests/slt/multiline_query.slt @@ -0,0 +1,10 @@ +query multiline +select * from example_multiline +---- ++---+---+---+ +| a | b | c | ++---+---+---+ +| 1 | 2 | 3 | +| 4 | 5 | 6 | +| 7 | 8 | 9 | ++---+---+---+