Merge pull request #2 from featuremesh/fix/error-comparing-columns

skip checking col names if not provided

Author: fantayeneh

sqllogictest/src/parser.rs

@@ -585,6 +585,8 @@ pub enum ParseErrorKind {
     UnexpectedEOF,
     #[error("invalid sort mode: {0:?}")]
     InvalidSortMode(String),
+    #[error("invalid column: {0:?}")]
+    InvalidColumn(String),
     #[error("invalid line: {0:?}")]
     InvalidLine(String),
     #[error("invalid type character: {0:?} in type string")]
@@ -747,8 +749,8 @@ fn parse_inner<T: ColumnType>(loc: &Location, script: &str) -> Result<Vec<Record
                             .map_err(|e| e.at(loc.clone()))?;
                         QueryExpect::Error(error)
                     }
-                    [type_str, res @ ..] => {
-                        let cols = parse_cols(&loc, type_str)?;
+                    [col_str, res @ ..] => {
+                        let cols = parse_cols(col_str, &loc)?;
                         let sort_mode = res
                             .first()
                             .map(|&s| SortMode::try_from_str(s))
@@ -840,43 +842,36 @@ fn parse_inner<T: ColumnType>(loc: &Location, script: &str) -> Result<Vec<Record
     Ok(records)
 }
 
-fn parse_cols<T: ColumnType>(
-    loc: &Location,
-    type_str: &&str,
-) -> Result<Vec<Column<T>>, ParseError> {
-    // Check if contains ':' or ',' to determine format
-    if type_str.contains(':') {
-        // Parse "c1:I,name:I" format
-        type_str
+fn parse_cols<T: ColumnType>(col_str: &&str, loc: &Location) -> Result<Vec<Column<T>>, ParseError> {
+    fn parse_type_char<T: ColumnType>(
+        part: &str,
+        name: String,
+        loc: &Location,
+    ) -> Result<Column<T>, ParseError> {
+        let type_char = part
+            .trim()
+            .chars()
+            .next()
+            .ok_or_else(|| ParseErrorKind::InvalidSortMode(part.into()).at(loc.clone()))?;
+
+        T::from_char(type_char)
+            .map(|t| Column { name, r#type: t })
+            .ok_or_else(|| ParseErrorKind::InvalidType(type_char).at(loc.clone()))
+    }
+    if col_str.contains(':') {
+        col_str
             .split(',')
-            .map(|part| {
-                let (name, type_char) = part
-                    .split_once(':')
-                    .ok_or_else(|| ParseErrorKind::InvalidSortMode(part.into()).at(loc.clone()))?;
-
-                let type_char =
-                    type_char.trim().chars().next().ok_or_else(|| {
-                        ParseErrorKind::InvalidSortMode(part.into()).at(loc.clone())
-                    })?;
-
-                T::from_char(type_char)
-                    .map(|t| Column {
-                        name: name.trim().to_string(),
-                        r#type: t,
-                    })
-                    .ok_or_else(|| ParseErrorKind::InvalidType(type_char).at(loc.clone()))
+            .map(|part| match part.split_once(':') {
+                Some((name, type_str)) => parse_type_char(type_str, name.trim().to_string(), loc),
+                None => parse_type_char(part, "?".into(), loc),
             })
             .try_collect()
     } else {
-        // Original "III" format
-        type_str
+        col_str
             .chars()
             .map(|ch| {
                 T::from_char(ch)
-                    .map(|t| Column {
-                        name: "?".into(),
-                        r#type: t,
-                    })
+                    .map(|t| Column::anon(t))
                     .ok_or_else(|| ParseErrorKind::InvalidType(ch).at(loc.clone()))
             })
             .try_collect()
@@ -992,6 +987,20 @@ mod tests {
     use super::*;
     use crate::DefaultColumnType;
 
+    #[test]
+    fn test_mixed_col_name_and_types() {
+        let script = r#"
+query NAME:I,B
+select * from t
+----
+1 true
+
+"#;
+
+        let result = parse::<CustomColumnType>(script);
+        assert!(result.is_ok())
+    }
+
     #[test]
     fn test_trailing_comment() {
         let script = "\

sqllogictest/src/runner.rs

@@ -46,12 +46,19 @@ pub enum RecordOutput<T: ColumnType> {
     },
 }
 
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone)]
 pub struct Column<T: ColumnType> {
     pub name: String,
     pub r#type: T,
 }
 
+impl<T: ColumnType> PartialEq for Column<T> {
+    fn eq(&self, other: &Self) -> bool {
+        self.r#type == other.r#type
+            && (other.name == "?" || self.name.to_lowercase() == other.name.to_lowercase())
+    }
+}
+
 impl<T: ColumnType + Display> Display for Column<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{}:{}", self.name, self.r#type.to_char())
@@ -530,11 +537,19 @@ pub fn strict_column_validator<T: ColumnType>(
     actual: &Vec<Column<T>>,
     expected: &Vec<Column<T>>,
 ) -> bool {
-    actual.len() == expected.len()
-        && !actual
-            .iter()
-            .zip(expected.iter())
-            .any(|(actual_column, expected_column)| actual_column != expected_column)
+    if actual.len() != expected.len() {
+        return false;
+    }
+    !actual
+        .iter()
+        .zip(expected.iter())
+        .any(|(actual_col, expected_col)| {
+            let type_mismatch = actual_col.r#type != expected_col.r#type;
+            let name_mismatch = expected_col.name != "?"
+                && actual_col.name.to_lowercase() != expected_col.name.to_lowercase();
+
+            type_mismatch || name_mismatch
+        })
 }
 
 /// Sqllogictest runner.
@@ -978,11 +993,7 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
                     sql,
                     expected,
                 },
-                RecordOutput::Query {
-                    cols: types,
-                    rows,
-                    error,
-                },
+                RecordOutput::Query { cols, rows, error },
             ) => {
                 match (error, expected) {
                     (None, QueryExpect::Error(_)) => {
@@ -1014,16 +1025,26 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
                     (
                         None,
                         QueryExpect::Results {
-                            cols: expected_types,
+                            cols: expected_cols,
                             results: expected_results,
                             ..
                         },
                     ) => {
-                        if !(self.column_type_validator)(types, &expected_types) {
+                        if !(self.column_type_validator)(cols, &expected_cols) {
                             return Err(TestErrorKind::QueryResultColumnsMismatch {
                                 sql,
-                                expected: expected_types.iter().map(|c| c.to_char()).join(""),
-                                actual: types.iter().map(|c| c.to_char()).join(""),
+                                expected: expected_cols
+                                    .iter()
+                                    .map(|Column { name, r#type }| {
+                                        format!("{}:{}", name, r#type.to_char())
+                                    })
+                                    .join(","),
+                                actual: cols
+                                    .iter()
+                                    .map(|Column { name, r#type }| {
+                                        format!("{}:{}", name, r#type.to_char())
+                                    })
+                                    .join(","),
                             }
                             .at(loc));
                         }