Skip to content

Commit

Permalink
WIP: Implement multiline query output
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@davisp
davisp committed Aug 16, 2024
1 parent 9857485 commit f33f3d8
Show file tree
Hide file tree
Showing 4 changed files with 215 additions and 0 deletions.
26 changes: 26 additions & 0 deletions sqllogictest/src/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ pub enum QueryExpect<T: ColumnType> {
label: Option<String>,
results: Vec<String>,
},
/// Query output can be returned as a multiline output to accommodate
/// matching tabular output directly.
MultiLine { data: Vec<String> },
/// Query should fail with the given error message.
Error(ExpectedError),
}
Expand Down Expand Up @@ -245,6 +248,9 @@ impl<T: ColumnType> std::fmt::Display for Record<T> {
write!(f, " {label}")?;
}
}
QueryExpect::MultiLine { .. } => {
write!(f, "multiline")?;
}
QueryExpect::Error(err) => err.fmt_inline(f)?,
}
writeln!(f)?;
Expand All @@ -260,6 +266,12 @@ impl<T: ColumnType> std::fmt::Display for Record<T> {
// 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(())
Expand Down Expand Up @@ -737,6 +749,7 @@ fn parse_inner<T: ColumnType>(loc: &Location, script: &str) -> Result<Vec<Record
.map_err(|e| e.at(loc.clone()))?;
QueryExpect::Error(error)
}
["multiline"] => QueryExpect::MultiLine { data: Vec::new() },
[type_str, res @ ..] => {
let types = type_str
.chars()
Expand Down Expand Up @@ -776,6 +789,14 @@ fn parse_inner<T: ColumnType>(loc: &Location, script: &str) -> Result<Vec<Record
results.push(line.to_string());
}
}
QueryExpect::MultiLine { data } => {
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() {
Expand Down Expand Up @@ -988,6 +1009,11 @@ mod tests {
parse_roundtrip::<DefaultColumnType>("../tests/slt/rowsort.slt")
}

#[test]
fn test_multiline_query() {
parse_roundtrip::<DefaultColumnType>("../tests/slt/multiline_query.slt")
}

#[test]
fn test_substitution() {
parse_roundtrip::<DefaultColumnType>("../tests/substitution/basic.slt")
Expand Down
165 changes: 165 additions & 0 deletions sqllogictest/src/runner.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ pub enum RecordOutput<T: ColumnType> {
rows: Vec<Vec<String>>,
error: Option<AnyError>,
},
MultiLineQuery {
lines: Vec<String>,
error: Option<AnyError>,
},
/// The output of a `statement`.
Statement { count: u64, error: Option<AnyError> },
/// The output of a `system` command.
Expand All @@ -52,6 +56,9 @@ pub enum DBOutput<T: ColumnType> {
types: Vec<T>,
rows: Vec<Vec<String>>,
},
MultiLine {
lines: Vec<String>,
},
/// A statement in the query has completed.
///
/// The number of rows modified or selected is returned.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -607,6 +616,9 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
rows,
error: None,
},
DBOutput::MultiLine { lines } => {
RecordOutput::MultiLineQuery { lines, error: None }
}
DBOutput::StatementComplete(count) => {
RecordOutput::Statement { count, error: None }
}
Expand Down Expand Up @@ -751,6 +763,9 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
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 };
}
Expand All @@ -766,6 +781,7 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {

let sort_mode = match expected {
QueryExpect::Results { sort_mode, .. } => sort_mode,
QueryExpect::MultiLine { .. } => None,
QueryExpect::Error(_) => None,
}
.or(self.sort_mode);
Expand Down Expand Up @@ -881,6 +897,14 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
.at(loc))
}
QueryExpect::Results { .. } => {}
QueryExpect::MultiLine { data } => {
return Err(TestErrorKind::QueryResultMismatch {
sql,
expected: data.join("\n"),
actual: "".to_string(),
}
.at(loc));
}
},
(
Record::Statement {
Expand Down Expand Up @@ -959,6 +983,14 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
.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,
Expand All @@ -967,6 +999,13 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
}
.at(loc));
}
(None, QueryExpect::MultiLine { .. }) => {
return Err(TestErrorKind::QueryResultTypeMismatch {
sql,
expected: "multiline".to_string(),
}
.at(loc));
}
(
None,
QueryExpect::Results {
Expand Down Expand Up @@ -997,6 +1036,72 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
}
};
}
(
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,
Expand Down Expand Up @@ -1481,6 +1586,7 @@ pub fn update_record_with_output<T: ColumnType>(
(Some(e), r) => {
let reference = match &r {
QueryExpect::Error(e) => Some(e),
QueryExpect::MultiLine { .. } => None,
QueryExpect::Results { .. } => None,
};
Some(Record::Query {
Expand Down Expand Up @@ -1525,6 +1631,12 @@ pub fn update_record_with_output<T: ColumnType>(
sort_mode,
label,
},
QueryExpect::MultiLine { .. } => QueryExpect::Results {
results,
types,
sort_mode: None,
label: None,
},
QueryExpect::Error(_) => QueryExpect::Results {
results,
types,
Expand All @@ -1535,6 +1647,59 @@ pub fn update_record_with_output<T: ColumnType>(
})
}
},
// 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,
Expand Down
14 changes: 14 additions & 0 deletions tests/harness.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions tests/slt/multiline_query.slt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
query multiline
select * from example_multiline
----
+---+---+---+
| a | b | c |
+---+---+---+
| 1 | 2 | 3 |
| 4 | 5 | 6 |
| 7 | 8 | 9 |
+---+---+---+

0 comments on commit f33f3d8

Please sign in to comment.