risinglightdb/sqllogictest-rs

enhancement: better diff for result mismatch

xxchan opened this issue · 5 comments

We use space as separator for actual, but expected might use tab. This can look bad.

This can be tricky since we support custom validator.

if !(self.validator)(&rows, &expected_results) {
let output_rows = rows
.into_iter()
.map(|strs| strs.iter().join(" "))
.collect_vec();
return Err(TestErrorKind::QueryResultMismatch {
sql,
expected: expected_results.join("\n"),
actual: output_rows.join("\n"),
}
.at(loc));


Here's an example from @BugenZhao
risingwavelabs/risingwave#9671 (comment)

image

Maybe we can:

  • change validator to sth like normalizer and output normalized results.
  • Output normalized output if default_validator is used.

I'm curious what's the use case for validators? 👀

Original added by @Xuanwo #15

As indicated by its signature, the actual result is separated by columns Vec<Vec<String>>, but the expected result is not. So we need to normalize the results. validator is the most flexible form to achieve this, but not sure if it's really needed.

See also #109

/// Validator will be used by [`Runner`] to validate the output.
///
/// # Default
///
/// By default ([`default_validator`]), we will use compare normalized results.
pub type Validator = fn(actual: &[Vec<String>], expected: &[String]) -> bool;
pub fn default_validator(actual: &[Vec<String>], expected: &[String]) -> bool {
let expected_results = expected.iter().map(normalize_string).collect_vec();
// Default, we compare normalized results. Whitespace characters are ignored.
let normalized_rows = actual
.iter()
.map(|strs| strs.iter().map(normalize_string).join(" "))
.collect_vec();
normalized_rows == expected_results
}

skyzh commented

I think we can parse both space and tab as separator in parser?

skyzh commented

Okay well this is not handled by parser. Then it might be a good idea to split by both space and tab in default validator :(