Store original, quoted form in SQLIdent

Also move more things to use SQLIdent instead of String in the hope of
making it a newtype eventually.

Add tests that quoted identifiers round-trip parsing/serialization correctly.

Author: nickolay

src/sqlast/mod.rs

@@ -27,7 +27,7 @@ pub use self::value::Value;
 
 pub use self::sql_operator::SQLOperator;
 
-// This could be enhanced to remember the way the identifier was quoted
+/// Identifier name, in the originally quoted form (e.g. `"id"`)
 pub type SQLIdent = String;
 
 /// SQL Abstract Syntax Tree (AST)
@@ -64,7 +64,8 @@ pub enum ASTNode {
     /// SQLValue
     SQLValue(Value),
     /// Scalar function call e.g. `LEFT(foo, 5)`
-    SQLFunction { id: String, args: Vec<ASTNode> },
+    /// TODO: this can be a compound SQLObjectName as well (for UDFs)
+    SQLFunction { id: SQLIdent, args: Vec<ASTNode> },
     /// CASE [<operand>] WHEN <condition> THEN <result> ... [ELSE <result>] END
     SQLCase {
         // TODO: support optional operand for "simple case"
@@ -317,7 +318,7 @@ impl ToString for SQLObjectName {
 /// SQL assignment `foo = expr` as used in SQLUpdate
 #[derive(Debug, Clone, PartialEq)]
 pub struct SQLAssignment {
-    id: String,
+    id: SQLIdent,
     value: Box<ASTNode>,
 }
 

src/sqlast/query.rs

@@ -134,7 +134,7 @@ pub enum JoinOperator {
 #[derive(Debug, Clone, PartialEq)]
 pub enum JoinConstraint {
     On(ASTNode),
-    Using(Vec<String>),
+    Using(Vec<SQLIdent>),
     Natural,
 }
 

src/sqlast/table_key.rs

@@ -3,7 +3,7 @@ use super::{SQLIdent, SQLObjectName};
 #[derive(Debug, PartialEq, Clone)]
 pub enum AlterOperation {
     AddConstraint(TableKey),
-    RemoveConstraint { name: String },
+    RemoveConstraint { name: SQLIdent },
 }
 
 impl ToString for AlterOperation {

src/sqlparser.rs

@@ -172,12 +172,12 @@ impl Parser {
                         })
                     }
                     _ => match self.peek_token() {
-                        Some(Token::LParen) => self.parse_function(&w.value),
+                        Some(Token::LParen) => self.parse_function(w.as_sql_ident()),
                         Some(Token::Period) => {
-                            let mut id_parts: Vec<String> = vec![w.value];
+                            let mut id_parts: Vec<SQLIdent> = vec![w.as_sql_ident()];
                             while self.consume_token(&Token::Period) {
                                 match self.next_token() {
-                                    Some(Token::SQLWord(w)) => id_parts.push(w.value),
+                                    Some(Token::SQLWord(w)) => id_parts.push(w.as_sql_ident()),
                                     _ => {
                                         return parser_err!(format!(
                                             "Error parsing compound identifier"
@@ -187,7 +187,7 @@ impl Parser {
                             }
                             Ok(ASTNode::SQLCompoundIdentifier(id_parts))
                         }
-                        _ => Ok(ASTNode::SQLIdentifier(w.value)),
+                        _ => Ok(ASTNode::SQLIdentifier(w.as_sql_ident())),
                     },
                 },
                 Token::Mult => Ok(ASTNode::SQLWildcard),
@@ -213,20 +213,17 @@ impl Parser {
         }
     }
 
-    pub fn parse_function(&mut self, id: &str) -> Result<ASTNode, ParserError> {
+    pub fn parse_function(&mut self, id: SQLIdent) -> Result<ASTNode, ParserError> {
         self.expect_token(&Token::LParen)?;
         if self.consume_token(&Token::RParen) {
             Ok(ASTNode::SQLFunction {
-                id: id.to_string(),
+                id: id,
                 args: vec![],
             })
         } else {
             let args = self.parse_expr_list()?;
             self.expect_token(&Token::RParen)?;
-            Ok(ASTNode::SQLFunction {
-                id: id.to_string(),
-                args,
-            })
+            Ok(ASTNode::SQLFunction { id, args })
         }
     }
 
@@ -573,7 +570,7 @@ impl Parser {
                                 Some(Token::Comma) => {
                                     self.next_token();
                                     columns.push(SQLColumnDef {
-                                        name: column_name.value,
+                                        name: column_name.as_sql_ident(),
                                         data_type: data_type,
                                         allow_null,
                                         is_primary,
@@ -584,7 +581,7 @@ impl Parser {
                                 Some(Token::RParen) => {
                                     self.next_token();
                                     columns.push(SQLColumnDef {
-                                        name: column_name.value,
+                                        name: column_name.as_sql_ident(),
                                         data_type: data_type,
                                         allow_null,
                                         is_primary,
@@ -622,15 +619,15 @@ impl Parser {
         }
     }
 
-    pub fn parse_table_key(&mut self, constraint_name: &str) -> Result<TableKey, ParserError> {
+    pub fn parse_table_key(&mut self, constraint_name: SQLIdent) -> Result<TableKey, ParserError> {
         let is_primary_key = self.parse_keywords(vec!["PRIMARY", "KEY"]);
         let is_unique_key = self.parse_keywords(vec!["UNIQUE", "KEY"]);
         let is_foreign_key = self.parse_keywords(vec!["FOREIGN", "KEY"]);
         self.expect_token(&Token::LParen)?;
         let column_names = self.parse_column_names()?;
         self.expect_token(&Token::RParen)?;
         let key = Key {
-            name: constraint_name.to_string(),
+            name: constraint_name,
             columns: column_names,
         };
         if is_primary_key {
@@ -664,7 +661,7 @@ impl Parser {
             if self.parse_keywords(vec!["ADD", "CONSTRAINT"]) {
                 match self.next_token() {
                     Some(Token::SQLWord(ref id)) => {
-                        let table_key = self.parse_table_key(&id.value)?;
+                        let table_key = self.parse_table_key(id.as_sql_ident())?;
                         Ok(AlterOperation::AddConstraint(table_key))
                     }
                     _ => {
@@ -1012,8 +1009,7 @@ impl Parser {
             Some(Token::SQLWord(ref w))
                 if after_as || !reserved_kwds.contains(&w.keyword.as_str()) =>
             {
-                // have to clone here until #![feature(bind_by_move_pattern_guards)] is enabled by default
-                Ok(Some(w.value.clone()))
+                Ok(Some(w.as_sql_ident()))
             }
             ref not_an_ident if after_as => parser_err!(format!(
                 "Expected an identifier after AS, got {:?}",
@@ -1036,7 +1032,7 @@ impl Parser {
             match token {
                 Some(Token::SQLWord(s)) if expect_identifier => {
                     expect_identifier = false;
-                    idents.push(s.to_string());
+                    idents.push(s.as_sql_ident());
                 }
                 Some(token) if token == separator && !expect_identifier => {
                     expect_identifier = true;
@@ -1386,3 +1382,9 @@ impl Parser {
         }
     }
 }
+
+impl SQLWord {
+    pub fn as_sql_ident(&self) -> SQLIdent {
+        self.to_string()
+    }
+}

tests/sqlparser_generic.rs

@@ -427,6 +427,34 @@ fn parse_select_version() {
     );
 }
 
+#[test]
+fn parse_delimited_identifiers() {
+    // check that quoted identifiers in any position remain quoted after serialization
+    let sql = r#"SELECT "alias"."bar baz", "myfun"(), "simple id" FROM "a table" AS "alias""#;
+    let select = verified_only_select(sql);
+    // check SELECT
+    assert_eq!(3, select.projection.len());
+    assert_eq!(
+        &ASTNode::SQLCompoundIdentifier(vec![r#""alias""#.to_string(), r#""bar baz""#.to_string()]),
+        expr_from_projection(&select.projection[0]),
+    );
+    assert_eq!(
+        &ASTNode::SQLFunction {
+            id: r#""myfun""#.to_string(),
+            args: vec![]
+        },
+        expr_from_projection(&select.projection[1]),
+    );
+    assert_eq!(
+        &ASTNode::SQLIdentifier(r#""simple id""#.to_string()),
+        expr_from_projection(&select.projection[2]),
+    );
+
+    verified_stmt(r#"CREATE TABLE "foo" ("bar" "int")"#);
+    verified_stmt(r#"ALTER TABLE foo ADD CONSTRAINT "bar" PRIMARY KEY (baz)"#);
+    //TODO verified_stmt(r#"UPDATE foo SET "bar" = 5"#);
+}
+
 #[test]
 fn parse_parens() {
     use self::ASTNode::*;