mirror of
https://github.com/lancedb/lancedb.git
synced 2026-08-18 12:08:35 +00:00
fix: support double-quoted filter identifiers
This commit is contained in:
@@ -20,6 +20,7 @@
|
||||
mod sql;
|
||||
|
||||
pub use sql::expr_to_sql_string;
|
||||
pub(crate) use sql::normalize_sql_filter;
|
||||
|
||||
use std::sync::Arc;
|
||||
|
||||
|
||||
@@ -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<String> {
|
||||
#[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<String> {
|
||||
}
|
||||
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")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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::<Vec<_>>()
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(batches.iter().map(RecordBatch::num_rows).sum::<usize>(), 2);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_select_with_transform() {
|
||||
// TODO: Switch back to memory://foo after https://github.com/lancedb/lancedb/issues/1051
|
||||
|
||||
Reference in New Issue
Block a user