From 00b1663732afc5e4b386bec12238260ad5d5f51b Mon Sep 17 00:00:00 2001 From: Gatefixer <313497061+lancedb-gatefixer[bot]@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:16:09 +0000 Subject: [PATCH] fix: support double-quoted filter identifiers --- rust/lancedb/src/expr.rs | 1 + rust/lancedb/src/expr/sql.rs | 126 +++++++++++++++++++++++++++++++++++ rust/lancedb/src/query.rs | 45 +++++++++++++ 3 files changed, 172 insertions(+) diff --git a/rust/lancedb/src/expr.rs b/rust/lancedb/src/expr.rs index 75cce443d..fe9b03270 100644 --- a/rust/lancedb/src/expr.rs +++ b/rust/lancedb/src/expr.rs @@ -20,6 +20,7 @@ mod sql; pub use sql::expr_to_sql_string; +pub(crate) use sql::normalize_sql_filter; use std::sync::Arc; diff --git a/rust/lancedb/src/expr/sql.rs b/rust/lancedb/src/expr/sql.rs index 23b89821a..08440de69 100644 --- a/rust/lancedb/src/expr/sql.rs +++ b/rust/lancedb/src/expr/sql.rs @@ -30,6 +30,98 @@ impl Dialect for LanceSqlDialect { } } +/// Translate SQL-standard double-quoted identifiers into the backtick-quoted +/// identifiers expected by Lance's SQL parser. +/// +/// Lance historically interpreted double-quoted values as string literals. +/// Rewriting them at the query boundary avoids silently evaluating a predicate +/// such as `"mixedCase" = 'value'` as a comparison between two literals. String +/// contents and existing backtick-quoted identifiers are left unchanged. +pub fn normalize_sql_filter(filter: &str) -> crate::Result { + #[derive(Clone, Copy, PartialEq, Eq)] + enum Quote { + None, + Single, + Backtick, + Double, + } + + let mut normalized = String::with_capacity(filter.len()); + let mut chars = filter.chars().peekable(); + let mut quote = Quote::None; + + while let Some(ch) = chars.next() { + match quote { + Quote::None => match ch { + '\'' => { + normalized.push(ch); + quote = Quote::Single; + } + '`' => { + normalized.push(ch); + quote = Quote::Backtick; + } + '"' => { + normalized.push('`'); + quote = Quote::Double; + } + _ => normalized.push(ch), + }, + Quote::Single => { + normalized.push(ch); + if ch == '\\' { + if let Some(escaped) = chars.next() { + normalized.push(escaped); + } + } else if ch == '\'' { + if chars.peek() == Some(&'\'') { + normalized.push(chars.next().expect("peeked character must exist")); + } else { + quote = Quote::None; + } + } + } + Quote::Backtick => { + normalized.push(ch); + if ch == '`' { + if chars.peek() == Some(&'`') { + normalized.push(chars.next().expect("peeked character must exist")); + } else { + quote = Quote::None; + } + } + } + Quote::Double => { + if ch == '"' { + if chars.peek() == Some(&'"') { + // SQL escapes a double quote within an identifier by + // doubling it. A quote needs no escaping inside Lance's + // backtick-delimited form. + normalized.push('"'); + chars.next(); + } else { + normalized.push('`'); + quote = Quote::None; + } + } else if ch == '`' { + // Lance escapes a backtick within an identifier by doubling it. + normalized.push_str("``"); + } else { + normalized.push(ch); + } + } + } + } + + if quote == Quote::Double { + return Err(crate::Error::InvalidInput { + message: "unterminated double-quoted identifier in SQL filter".to_string(), + }); + } + + Ok(normalized) +} + /// Prefix for placeholder strings inserted in place of binary literals. Chosen /// to be extremely unlikely to occur in user data. const BINARY_PLACEHOLDER_PREFIX: &str = "__lancedb_binary_placeholder_"; @@ -113,3 +205,37 @@ pub fn expr_to_sql_string(expr: &Expr) -> crate::Result { } Ok(sql) } + +#[cfg(test)] +mod tests { + use super::normalize_sql_filter; + + #[test] + fn normalizes_double_quoted_identifiers() { + assert_eq!( + normalize_sql_filter(r#""PartyAbbrev" = 'D'"#).unwrap(), + "`PartyAbbrev` = 'D'" + ); + assert_eq!( + normalize_sql_filter(r#""MetaData"."userId" = 5"#).unwrap(), + "`MetaData`.`userId` = 5" + ); + assert_eq!(normalize_sql_filter(r#""a""b" = 1"#).unwrap(), "`a\"b` = 1"); + } + + #[test] + fn preserves_quotes_inside_literals_and_backticks() { + let filter = r#"name = 'Alice "Ace"' AND `quoted"field` = 1"#; + assert_eq!(normalize_sql_filter(filter).unwrap(), filter); + } + + #[test] + fn rejects_unterminated_double_quoted_identifier() { + let error = normalize_sql_filter(r#""PartyAbbrev = 'D'"#).unwrap_err(); + assert!( + error + .to_string() + .contains("unterminated double-quoted identifier") + ); + } +} diff --git a/rust/lancedb/src/query.rs b/rust/lancedb/src/query.rs index b76865043..21e549ac5 100644 --- a/rust/lancedb/src/query.rs +++ b/rust/lancedb/src/query.rs @@ -399,6 +399,9 @@ pub trait QueryBase { /// x > 5 OR y = 'test' /// ``` /// + /// Identifiers may be delimited with SQL-standard double quotes or + /// backticks. String literals must use single quotes. + /// /// Filtering performance can often be improved by creating a scalar index /// on the filter column(s). /// @@ -913,6 +916,17 @@ impl QueryRequest { /// use different representations) the error is recorded and surfaced later /// by [`Self::check_filter`]. pub(crate) fn add_filter(&mut self, new: QueryFilter) { + let new = match new { + QueryFilter::Sql(filter) => match crate::expr::normalize_sql_filter(&filter) { + Ok(filter) => QueryFilter::Sql(filter), + Err(err) => { + self.filter_error = Some(err.to_string()); + return; + } + }, + other => other, + }; + self.filter = Some(match self.filter.take() { None => new, Some(existing) => match and_filters(existing, new) { @@ -1887,6 +1901,37 @@ mod tests { query.execute().await.unwrap(); } + #[tokio::test] + async fn test_double_quoted_filter_identifier() { + let tmp_dir = tempdir().unwrap(); + let dataset_path = tmp_dir.path().join("test.lance"); + let uri = dataset_path.to_str().unwrap(); + let schema = Arc::new(ArrowSchema::new(vec![ArrowField::new( + "PartyAbbrev", + DataType::Utf8, + false, + )])); + let batch = RecordBatch::try_new( + schema, + vec![Arc::new(StringArray::from(vec!["D", "R", "R", "D"]))], + ) + .unwrap(); + + let conn = connect(uri).execute().await.unwrap(); + let table = conn.create_table("parties", batch).execute().await.unwrap(); + let batches = table + .query() + .only_if(r#""PartyAbbrev" = 'D'"#) + .execute() + .await + .unwrap() + .try_collect::>() + .await + .unwrap(); + + assert_eq!(batches.iter().map(RecordBatch::num_rows).sum::(), 2); + } + #[tokio::test] async fn test_select_with_transform() { // TODO: Switch back to memory://foo after https://github.com/lancedb/lancedb/issues/1051