diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 6f3839c1d..bfd6e4eb3 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -3471,7 +3471,7 @@ pub fn bind_and_rewrite_expr<'a>( // SQLite behavior: Only double-quoted identifiers get fallback to string literals // Single quotes are handled as literals earlier, unquoted identifiers must resolve to columns - if id.is_double_quoted() { + if id.quoted_with('"') { // Convert failed double-quoted identifier to string literal *expr = Expr::Literal(ast::Literal::String(id.as_str().to_string())); return Ok(WalkControl::Continue); diff --git a/core/translate/insert.rs b/core/translate/insert.rs index 55d0bfc9d..c8e2627a1 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -148,9 +148,11 @@ pub fn translate_insert( for expr in values_expr.iter_mut().flat_map(|v| v.iter_mut()) { match expr.as_mut() { Expr::Id(name) => { - if name.is_double_quoted() { - *expr = - Expr::Literal(ast::Literal::String(name.to_string())).into(); + if name.quoted_with('"') { + *expr = Expr::Literal(ast::Literal::String( + name.as_literal().to_string(), + )) + .into(); } else { // an INSERT INTO ... VALUES (...) cannot reference columns crate::bail_parse_error!("no such column: {name}"); diff --git a/core/translate/optimizer/mod.rs b/core/translate/optimizer/mod.rs index 9273212d7..edbd35ec9 100644 --- a/core/translate/optimizer/mod.rs +++ b/core/translate/optimizer/mod.rs @@ -709,7 +709,7 @@ impl Optimizable for ast::Expr { Expr::FunctionCallStar { .. } => false, Expr::Id(id) => { // If we got here with an id, this has to be double-quotes identifier - assert!(id.is_double_quoted()); + assert!(id.quoted_with('"')); true } Expr::Column { .. } => false, diff --git a/core/translate/pragma.rs b/core/translate/pragma.rs index 3bc8668e8..ac4662eec 100644 --- a/core/translate/pragma.rs +++ b/core/translate/pragma.rs @@ -610,9 +610,11 @@ fn query_pragma( if let Some(value_expr) = value { let is_query_only = match value_expr { ast::Expr::Literal(Literal::Numeric(i)) => i.parse::().unwrap() != 0, - ast::Expr::Literal(Literal::String(ref s)) - | ast::Expr::Name(Name::Ident(ref s)) => { - let s = s.as_bytes(); + ast::Expr::Literal(Literal::String(..)) | ast::Expr::Name(..) => { + let s = match value_expr { + Literal::String(s) => s.as_bytes(), + ast::Expr::Name(n) => n.as_str().as_bytes(), + }; match_ignore_ascii_case!(match s { b"1" | b"on" | b"true" => true, _ => false, diff --git a/core/translate/result_row.rs b/core/translate/result_row.rs index 04b7454f5..03e18fe98 100644 --- a/core/translate/result_row.rs +++ b/core/translate/result_row.rs @@ -205,8 +205,8 @@ pub fn try_fold_expr_to_i64(expr: &Box) -> Option { match expr.as_ref() { Expr::Literal(Literal::Numeric(n)) => n.parse::().ok(), Expr::Literal(Literal::Null) => Some(0), - Expr::Id(Name::Ident(s)) => { - let lowered = s.to_ascii_lowercase(); + Expr::Id(name) if !name.quoted() => { + let lowered = name.as_str(); if lowered == "true" { Some(1) } else if lowered == "false" { diff --git a/core/util.rs b/core/util.rs index 2a10d5302..e5ce1a613 100644 --- a/core/util.rs +++ b/core/util.rs @@ -1019,11 +1019,7 @@ pub fn parse_signed_number(expr: &Expr) -> Result { pub fn parse_string(expr: &Expr) -> Result { match expr { - Expr::Name(ast::Name::Ident(s)) | Expr::Name(ast::Name::Quoted(s)) - if s.len() >= 2 && s.starts_with("'") && s.ends_with("'") => - { - Ok(s[1..s.len() - 1].to_string()) - } + Expr::Name(name) if name.quoted_with('\'') => Ok(name.as_str().to_string()), _ => Err(LimboError::InvalidArgument(format!( "string parameter expected, got {expr:?} instead" ))), diff --git a/parser/src/ast.rs b/parser/src/ast.rs index f543e6d24..d921e8c99 100644 --- a/parser/src/ast.rs +++ b/parser/src/ast.rs @@ -891,6 +891,9 @@ impl Name { pub fn exact(s: String) -> Self { Self::Ident(s) } + pub fn from_bytes(s: &[u8]) -> Self { + Self::new(unsafe { std::str::from_utf8_unchecked(s) }.to_owned()) + } pub fn new(s: impl AsRef) -> Self { let s = s.as_ref(); let bytes = s.as_bytes(); @@ -911,13 +914,27 @@ impl Name { } } - /// Checks if a name represents a double-quoted string that should get fallback behavior - pub fn is_double_quoted(&self) -> bool { + pub fn as_literal(&self) -> &str { + match self { + Name::Ident(s) | Name::Quoted(s) => s.as_str(), + } + } + + /// Checks if a name represents a quoted string that should get fallback behavior + /// Need to detect legacy conversion of double quoted keywords to string literals + /// (see https://sqlite.org/lang_keywords.html) + /// + /// Also, used to detect string literals in PRAGMA cases + pub fn quoted_with(&self, quote: char) -> bool { if let Self::Quoted(ident) = self { - return ident.starts_with("\""); + return ident.starts_with(quote); } false } + + pub fn quoted(&self) -> bool { + matches!(self, Self::Quoted(..)) + } } /// Qualified name diff --git a/parser/src/parser.rs b/parser/src/parser.rs index 309bf7365..d9bfc593f 100644 --- a/parser/src/parser.rs +++ b/parser/src/parser.rs @@ -605,12 +605,7 @@ impl<'a> Parser<'a> { fn parse_nm(&mut self) -> Result { let tok = eat_expect!(self, TK_ID, TK_STRING, TK_INDEXED, TK_JOIN_KW); - - let first_char = tok.value[0]; // no need to check empty - match first_char { - b'[' | b'\'' | b'`' | b'"' => Ok(Name::Quoted(from_bytes(tok.value))), - _ => Ok(Name::exact(from_bytes(tok.value))), - } + Ok(Name::from_bytes(tok.value)) } fn parse_transopt(&mut self) -> Result> { @@ -1465,7 +1460,10 @@ impl<'a> Parser<'a> { } _ => { let can_be_lit_str = tok.token_type == Some(TK_STRING); - let name = self.parse_nm()?; + + // can be either Literal::String or Name - so we parse raw value early and decide later + let tok = eat_expect!(self, TK_ID, TK_STRING, TK_INDEXED, TK_JOIN_KW); + let name = tok.value; let second_name = if let Some(tok) = self.peek()? { if tok.token_type == Some(TK_DOT) { @@ -1487,7 +1485,7 @@ impl<'a> Parser<'a> { eat_assert!(self, TK_STAR); eat_expect!(self, TK_RP); return Ok(Box::new(Expr::FunctionCallStar { - name, + name: Name::from_bytes(name), filter_over: self.parse_filter_over()?, })); } @@ -1498,7 +1496,7 @@ impl<'a> Parser<'a> { eat_expect!(self, TK_RP); let filter_over = self.parse_filter_over()?; return Ok(Box::new(Expr::FunctionCall { - name, + name: Name::from_bytes(name), distinctness: distinct, args: exprs, order_by, @@ -1528,34 +1526,29 @@ impl<'a> Parser<'a> { if let Some(second_name) = second_name { if let Some(third_name) = third_name { Ok(Box::new(Expr::DoublyQualified( - name, + Name::from_bytes(name), second_name, third_name, ))) } else { - Ok(Box::new(Expr::Qualified(name, second_name))) + Ok(Box::new(Expr::Qualified( + Name::from_bytes(name), + second_name, + ))) } } else if can_be_lit_str { - Ok(Box::new(Expr::Literal(match name { - Name::Quoted(s) => Literal::String(s), - Name::Ident(s) => Literal::String(s), - }))) + Ok(Box::new(Expr::Literal(Literal::String(from_bytes(name))))) } else { - match name { - Name::Ident(s) => { - let s_bytes = s.as_bytes(); - match_ignore_ascii_case!(match s_bytes { - b"true" => { - Ok(Box::new(Expr::Literal(Literal::Numeric("1".into())))) - } - b"false" => { - Ok(Box::new(Expr::Literal(Literal::Numeric("0".into())))) - } - _ => Ok(Box::new(Expr::Id(Name::exact(s)))), - }) + + match_ignore_ascii_case!(match name { + b"true" => { + Ok(Box::new(Expr::Literal(Literal::Numeric("1".into())))) } - _ => Ok(Box::new(Expr::Id(name))), - } + b"false" => { + Ok(Box::new(Expr::Literal(Literal::Numeric("0".into())))) + } + _ => Ok(Box::new(Expr::Id(Name::from_bytes(name)))), + }) } } } @@ -1919,11 +1912,7 @@ impl<'a> Parser<'a> { } let tok = eat_expect!(self, TK_ID, TK_STRING); - let first_char = tok.value[0]; // no need to check empty - match first_char { - b'[' | b'\'' | b'`' | b'"' => Ok(Some(Name::Quoted(from_bytes(tok.value)))), - _ => Ok(Some(Name::exact(from_bytes(tok.value)))), - } + Ok(Some(Name::from_bytes(tok.value))) } fn parse_sort_order(&mut self) -> Result> { @@ -4138,7 +4127,7 @@ mod tests { b"BEGIN EXCLUSIVE TRANSACTION 'my_transaction'".as_slice(), vec![Cmd::Stmt(Stmt::Begin { typ: Some(TransactionType::Exclusive), - name: Some(Name::Quoted("'my_transaction'".to_string())), + name: Some(Name::new("'my_transaction'".to_string())), })], ), ( @@ -4159,7 +4148,7 @@ mod tests { b"BEGIN CONCURRENT TRANSACTION 'my_transaction'".as_slice(), vec![Cmd::Stmt(Stmt::Begin { typ: Some(TransactionType::Concurrent), - name: Some(Name::Quoted("'my_transaction'".to_string())), + name: Some(Name::new("'my_transaction'".to_string())), })], ), ( @@ -4254,7 +4243,7 @@ mod tests { ( b"SAVEPOINT 'my_savepoint'".as_slice(), vec![Cmd::Stmt(Stmt::Savepoint { - name: Name::Quoted("'my_savepoint'".to_string()), + name: Name::new("'my_savepoint'".to_string()), })], ), // release @@ -4273,7 +4262,7 @@ mod tests { ( b"RELEASE SAVEPOINT 'my_savepoint'".as_slice(), vec![Cmd::Stmt(Stmt::Release { - name: Name::Quoted("'my_savepoint'".to_string()), + name: Name::new("'my_savepoint'".to_string()), })], ), ( @@ -11485,13 +11474,13 @@ mod tests { if_not_exists: false, tbl_name: QualifiedName { db_name: None, - name: Name::Quoted("\"settings\"".to_owned()), + name: Name::new("\"settings\"".to_owned()), alias: None, }, body: CreateTableBody::ColumnsAndConstraints{ columns: vec![ ColumnDefinition { - col_name: Name::Quoted("\"enabled\"".to_owned()), + col_name: Name::new("\"enabled\"".to_owned()), col_type: Some(Type { name: "INTEGER".to_owned(), size: None,