From 27c95060590abbed1dd43e231616bea4d77cfecb Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Fri, 26 Sep 2025 12:32:01 +0400 Subject: [PATCH] do not auto-lowercase Name identifiers - this is complicated because column names must preserve case --- core/translate/schema.rs | 2 +- parser/src/ast.rs | 33 ++++++++++++--------------------- parser/src/ast/fmt.rs | 4 ++-- parser/src/parser.rs | 38 +++++++++++++++++++------------------- 4 files changed, 34 insertions(+), 43 deletions(-) diff --git a/core/translate/schema.rs b/core/translate/schema.rs index c3dc5b391..f2da72cd8 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -422,7 +422,7 @@ fn collect_autoindexes( fn create_table_body_to_str(tbl_name: &ast::QualifiedName, body: &ast::CreateTableBody) -> String { let mut sql = String::new(); - sql.push_str(format!("CREATE TABLE {} {}", tbl_name.name.as_quoted(), body).as_str()); + sql.push_str(format!("CREATE TABLE {} {}", tbl_name.name.as_ident(), body).as_str()); match body { ast::CreateTableBody::ColumnsAndConstraints { columns: _, diff --git a/parser/src/ast.rs b/parser/src/ast.rs index 5e0c8b1c0..c14f00e27 100644 --- a/parser/src/ast.rs +++ b/parser/src/ast.rs @@ -1,8 +1,6 @@ pub mod check; pub mod fmt; -use std::sync::OnceLock; - use strum_macros::{EnumIter, EnumString}; /// `?` or `$` Prepared statement arg placeholder(s) @@ -884,8 +882,6 @@ pub struct GroupBy { pub struct Name { quote: Option, value: String, - lowercase: OnceLock, - value_is_lowercase: bool, } #[cfg(feature = "serde")] @@ -927,12 +923,9 @@ impl Name { /// Create name which will have exactly the value of given string /// (e.g. if s = "\"str\"" - the name value will contain quotes and translation to SQL will give us """str""") pub fn exact(s: String) -> Self { - let value_is_lowercase = s.chars().all(|x| x.is_lowercase()); Self { value: s, quote: None, - lowercase: OnceLock::new(), - value_is_lowercase, } } /// Parse name from the bytes (e.g. handle quoting and handle escaped quotes) @@ -957,12 +950,9 @@ impl Name { b'`' => s[1..s.len() - 1].replace("``", "`"), _ => unreachable!(), }; - let value_is_lowercase = s.chars().all(|x| x.is_lowercase()); Name { value: s, quote: Some(bytes[0] as char), - lowercase: OnceLock::new(), - value_is_lowercase, } } else if bytes[0] == b'[' { assert!(s.len() >= 2); @@ -973,16 +963,9 @@ impl Name { } } - /// Return string value of the name alredy converted to the lowercase - /// This value can be safely compared with other values without any normalization logic + /// Return string value of the name pub fn as_str(&self) -> &str { - if self.value_is_lowercase { - return &self.value; - } - if self.lowercase.get().is_none() { - let _ = self.lowercase.set(self.value.to_lowercase()); - } - self.lowercase.get().unwrap() + &self.value } /// Convert value to the string literal (e.g. single-quoted string with escaped single quotes) @@ -991,9 +974,17 @@ impl Name { } /// Convert value to the name string (e.g. double-quoted string with escaped double quotes) - pub fn as_quoted(&self) -> String { + pub fn as_ident(&self) -> String { + // let's keep original quotes if they were set + // (parser.rs tests validates that behaviour) + if let Some(quote) = self.quote { + let single = quote.to_string(); + let double = single.clone() + &single; + return format!("{}{}{}", quote, self.value.replace(&single, &double), quote); + } let value = self.value.as_bytes(); - if !value.is_empty() && value.iter().all(|x| x.is_ascii_alphanumeric()) { + let safe_char = |&c: &u8| c.is_ascii_alphanumeric() || c == b'_'; + if !value.is_empty() && value.iter().all(safe_char) { self.value.clone() } else { format!("\"{}\"", self.value.replace("\"", "\"\"")) diff --git a/parser/src/ast/fmt.rs b/parser/src/ast/fmt.rs index 7fdf19a9c..39f14cb59 100644 --- a/parser/src/ast/fmt.rs +++ b/parser/src/ast/fmt.rs @@ -745,7 +745,7 @@ impl ToTokens for Expr { Self::Collate(expr, collation) => { expr.to_tokens(s, context)?; s.append(TK_COLLATE, None)?; - s.append(TK_ID, Some(&collation.as_quoted())) + s.append(TK_ID, Some(&collation.as_ident())) } Self::DoublyQualified(db_name, tbl_name, col_name) => { db_name.to_tokens(s, context)?; @@ -1370,7 +1370,7 @@ impl ToTokens for Name { s: &mut S, _: &C, ) -> Result<(), S::Error> { - s.append(TK_ID, Some(&self.as_quoted())) + s.append(TK_ID, Some(&self.as_ident())) } } diff --git a/parser/src/parser.rs b/parser/src/parser.rs index b7f1216b6..5cbac5fd1 100644 --- a/parser/src/parser.rs +++ b/parser/src/parser.rs @@ -4105,28 +4105,28 @@ mod tests { b"BEGIN DEFERRED TRANSACTION my_transaction".as_slice(), vec![Cmd::Stmt(Stmt::Begin { typ: Some(TransactionType::Deferred), - name: Some(Name::exact("my_transaction".to_string())), + name: Some(Name::from_string("my_transaction")), })], ), ( b"BEGIN IMMEDIATE TRANSACTION my_transaction".as_slice(), vec![Cmd::Stmt(Stmt::Begin { typ: Some(TransactionType::Immediate), - name: Some(Name::exact("my_transaction".to_string())), + name: Some(Name::from_string("my_transaction")), })], ), ( b"BEGIN EXCLUSIVE TRANSACTION my_transaction".as_slice(), vec![Cmd::Stmt(Stmt::Begin { typ: Some(TransactionType::Exclusive), - name: Some(Name::exact("my_transaction".to_string())), + name: Some(Name::from_string("my_transaction")), })], ), ( b"BEGIN EXCLUSIVE TRANSACTION 'my_transaction'".as_slice(), vec![Cmd::Stmt(Stmt::Begin { typ: Some(TransactionType::Exclusive), - name: Some(Name::from_string("'my_transaction'".to_string())), + name: Some(Name::from_string("'my_transaction'")), })], ), ( @@ -4140,14 +4140,14 @@ mod tests { b"BEGIN CONCURRENT TRANSACTION my_transaction".as_slice(), vec![Cmd::Stmt(Stmt::Begin { typ: Some(TransactionType::Concurrent), - name: Some(Name::exact("my_transaction".to_string())), + name: Some(Name::from_string("my_transaction")), })], ), ( b"BEGIN CONCURRENT TRANSACTION 'my_transaction'".as_slice(), vec![Cmd::Stmt(Stmt::Begin { typ: Some(TransactionType::Concurrent), - name: Some(Name::from_string("'my_transaction'".to_string())), + name: Some(Name::from_string("'my_transaction'")), })], ), ( @@ -4187,13 +4187,13 @@ mod tests { ( b"COMMIT TRANSACTION my_transaction".as_slice(), vec![Cmd::Stmt(Stmt::Commit { - name: Some(Name::exact("my_transaction".to_string())), + name: Some(Name::from_string("my_transaction")), })], ), ( b"END TRANSACTION my_transaction".as_slice(), vec![Cmd::Stmt(Stmt::Commit { - name: Some(Name::exact("my_transaction".to_string())), + name: Some(Name::from_string("my_transaction")), })], ), // Rollback @@ -4208,66 +4208,66 @@ mod tests { b"ROLLBACK TO SAVEPOINT my_savepoint".as_slice(), vec![Cmd::Stmt(Stmt::Rollback { tx_name: None, - savepoint_name: Some(Name::exact("my_savepoint".to_string())), + savepoint_name: Some(Name::from_string("my_savepoint")), })], ), ( b"ROLLBACK TO my_savepoint".as_slice(), vec![Cmd::Stmt(Stmt::Rollback { tx_name: None, - savepoint_name: Some(Name::exact("my_savepoint".to_string())), + savepoint_name: Some(Name::from_string("my_savepoint")), })], ), ( b"ROLLBACK TRANSACTION my_transaction".as_slice(), vec![Cmd::Stmt(Stmt::Rollback { - tx_name: Some(Name::exact("my_transaction".to_string())), + tx_name: Some(Name::from_string("my_transaction")), savepoint_name: None, })], ), ( b"ROLLBACK TRANSACTION my_transaction TO my_savepoint".as_slice(), vec![Cmd::Stmt(Stmt::Rollback { - tx_name: Some(Name::exact("my_transaction".to_string())), - savepoint_name: Some(Name::exact("my_savepoint".to_string())), + tx_name: Some(Name::from_string("my_transaction")), + savepoint_name: Some(Name::from_string("my_savepoint")), })], ), // savepoint ( b"SAVEPOINT my_savepoint".as_slice(), vec![Cmd::Stmt(Stmt::Savepoint { - name: Name::exact("my_savepoint".to_string()), + name: Name::from_string("my_savepoint"), })], ), ( b"SAVEPOINT 'my_savepoint'".as_slice(), vec![Cmd::Stmt(Stmt::Savepoint { - name: Name::from_string("'my_savepoint'".to_string()), + name: Name::from_string("'my_savepoint'"), })], ), // release ( b"RELEASE my_savepoint".as_slice(), vec![Cmd::Stmt(Stmt::Release { - name: Name::exact("my_savepoint".to_string()), + name: Name::from_string("my_savepoint"), })], ), ( b"RELEASE SAVEPOINT my_savepoint".as_slice(), vec![Cmd::Stmt(Stmt::Release { - name: Name::exact("my_savepoint".to_string()), + name: Name::from_string("my_savepoint"), })], ), ( b"RELEASE SAVEPOINT 'my_savepoint'".as_slice(), vec![Cmd::Stmt(Stmt::Release { - name: Name::from_string("'my_savepoint'".to_string()), + name: Name::from_string("'my_savepoint'"), })], ), ( b"RELEASE SAVEPOINT ABORT".as_slice(), vec![Cmd::Stmt(Stmt::Release { - name: Name::exact("ABORT".to_string()), + name: Name::from_string("ABORT"), })], ), // test expr operand