refactor: show tables (apache#382)

* refactor show tables * remove unwrap * fix clippy
chunshao90 · Nov 9, 2022 · fc500ce · fc500ce
1 parent d410637
commit fc500ce
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 61 deletions.
diff --git a/interpreters/src/show.rs b/interpreters/src/show.rs
@@ -69,6 +69,12 @@ pub enum Error {
 
     #[snafu(display("Failed to fetch schema, err:{}", source))]
     FetchSchema { source: catalog::Error },
+
+    #[snafu(display("Invalid regexp, err:{}.\nBacktrace\n:{}", source, backtrace))]
+    InvalidRegexp {
+        source: regex::Error,
+        backtrace: Backtrace,
+    },
 }
 
 define_result!(Error);
@@ -102,13 +108,16 @@ impl ShowInterpreter {
     ) -> Result<Output> {
         let schema = get_default_schema(&ctx, &catalog_manager)?;
         let tables_names = match plan.pattern {
-            Some(sc) => schema
-                .all_tables()
-                .context(FetchTables)?
-                .iter()
-                .filter(|t| is_table_matched(t.name(), &sc).unwrap())
-                .map(|t| t.name().to_string())
-                .collect::<Vec<_>>(),
+            Some(pattern) => {
+                let pattern_re = to_pattern_re(&pattern)?;
+                schema
+                    .all_tables()
+                    .context(FetchTables)?
+                    .iter()
+                    .map(|t| t.name().to_string())
+                    .filter(|table_name| pattern_re.is_match(table_name))
+                    .collect::<Vec<_>>()
+            }
             None => schema
                 .all_tables()
                 .context(FetchTables)?
@@ -158,15 +167,15 @@ impl ShowInterpreter {
     }
 }
 
-fn is_table_matched(str: &str, search_re: &str) -> Result<bool> {
-    let regex_str = search_re.replace('_', ".");
-    let regex_st = if regex_str.contains('%') {
-        regex_str.replace('%', ".*")
-    } else {
-        format!("^{}$", &regex_str)
-    };
-    let re = Regex::new(&regex_st).unwrap();
-    Ok(re.is_match(str))
+fn to_pattern_re(pattern: &str) -> Result<Regex> {
+    // In MySQL
+    // `_` match any single character
+    // `% ` match an arbitrary number of characters (including zero characters).
+    // so replace those meta character to regexp syntax
+    // TODO: support escape char to match exact those two chars
+    let pattern = pattern.replace('_', ".").replace('%', ".*");
+    let pattern = format!("^{}$", pattern);
+    Regex::new(&pattern).context(InvalidRegexp)
 }
 
 #[async_trait]
@@ -214,51 +223,24 @@ fn get_default_schema(
 
 #[cfg(test)]
 mod tests {
-    use crate::show::is_table_matched;
+    use crate::show::to_pattern_re;
     #[test]
 
     fn test_is_table_matched() {
-        assert_eq!(
-            "true".to_string(),
-            is_table_matched("01_system_table1", "01%")
-                .unwrap()
-                .to_string()
-        );
-        assert_eq!(
-            "true".to_string(),
-            is_table_matched("01_system_table1", "01_%")
-                .unwrap()
-                .to_string()
-        );
-        assert_eq!(
-            "false".to_string(),
-            is_table_matched("01_system_table1", "01_system_table")
-                .unwrap()
-                .to_string()
-        );
-        assert_eq!(
-            "true".to_string(),
-            is_table_matched("01_system_table1", "01_system_table1")
-                .unwrap()
-                .to_string()
-        );
-        assert_eq!(
-            "true".to_string(),
-            is_table_matched("01_system_table1", "01_system_table.")
-                .unwrap()
-                .to_string()
-        );
-        assert_eq!(
-            "false".to_string(),
-            is_table_matched("01_system_table1", "01_system_tabl.")
-                .unwrap()
-                .to_string()
-        );
-        assert_eq!(
-            "true".to_string(),
-            is_table_matched("01_system_table1", "%system%")
-                .unwrap()
-                .to_string()
-        );
+        let testcases = vec![
+            // table, pattern, matched
+            ("abc", "abc", true),
+            ("abc", "abcd", false),
+            ("abc", "ab%", true),
+            ("abc", "%b%", true),
+            ("abc", "_b_", true),
+            ("aabcc", "%b%", true),
+            ("aabcc", "_b_", false),
+        ];
+
+        for (table_name, pattern, matched) in testcases {
+            let pattern = to_pattern_re(pattern).unwrap();
+            assert_eq!(matched, pattern.is_match(table_name));
+        }
     }
 }
diff --git a/sql/src/parser.rs b/sql/src/parser.rs
@@ -237,9 +237,10 @@ impl<'a> Parser<'a> {
         let pattern = match self.parser.next_token() {
             Token::Word(w) => match w.keyword {
                 Keyword::LIKE => Some(self.parser.parse_literal_string()?),
-                _ => None,
+                _ => return self.expected("like", self.parser.peek_token()),
             },
-            _ => None,
+            Token::SemiColon | Token::EOF => None,
+            _ => return self.expected(";", self.parser.peek_token()),
         };
         Ok(Statement::ShowTables(ShowTables { pattern }))
     }
@@ -897,4 +898,43 @@ mod tests {
             expect_parse_ok(sql, expected).unwrap()
         }
     }
+
+    #[test]
+    fn test_show_tables() {
+        {
+            let sql = "show tables;";
+            let statements = Parser::parse_sql(sql).unwrap();
+            assert_eq!(statements.len(), 1);
+            assert!(matches!(
+                statements[0],
+                Statement::ShowTables(ShowTables { pattern: None })
+            ));
+        }
+
+        {
+            let sql = "show tables";
+            let statements = Parser::parse_sql(sql).unwrap();
+            assert_eq!(statements.len(), 1);
+            assert!(matches!(
+                statements[0],
+                Statement::ShowTables(ShowTables { pattern: None })
+            ));
+        }
+
+        {
+            let sql = "show tables like '%abc%'";
+            let statements = Parser::parse_sql(sql).unwrap();
+            assert_eq!(statements.len(), 1);
+
+            assert!(matches!(
+                &statements[0],
+                Statement::ShowTables(ShowTables { pattern }) if pattern == &Some("%abc%".to_string())
+            ));
+        }
+
+        {
+            let sql = "show tables '%abc%'";
+            assert!(Parser::parse_sql(sql).is_err());
+        }
+    }
 }