diff --git a/rust/ffi/dataset.rs b/rust/ffi/dataset.rs index 3e5dec8..72af859 100644 --- a/rust/ffi/dataset.rs +++ b/rust/ffi/dataset.rs @@ -9,7 +9,8 @@ use arrow_array::types::UInt64Type; use datafusion::logical_expr::Expr; use datafusion::physical_plan::SendableRecordBatchStream; use datafusion::scalar::ScalarValue; -use datafusion_sql::unparser::expr_to_sql; +use datafusion_sql::unparser::dialect::CustomDialectBuilder; +use datafusion_sql::unparser::Unparser; use futures::TryStreamExt; use lance::dataset::builder::DatasetBuilder; use lance::dataset::statistics::DatasetStatisticsExt; @@ -28,6 +29,28 @@ use super::util::{ FfiResult, }; +/// Convert an optional filter expression into a predicate SQL string for +/// Lance's string-based `delete` API. Identifiers are always backtick-quoted so +/// column names that collide with SQL keywords (e.g. `name`, `value`, `key`, +/// `desc`) survive the unparse/re-parse round-trip. Lance's filter parser uses +/// backtick identifier quoting; double quotes would be treated as string +/// literals. +fn delete_filter_expr_to_sql(filter: Option) -> FfiResult { + match filter { + Some(expr) => { + let dialect = CustomDialectBuilder::new() + .with_identifier_quote_style('`') + .build(); + let unparser = Unparser::new(&dialect); + let sql = unparser.expr_to_sql(&expr).map_err(|err| { + FfiError::new(ErrorCode::DatasetDelete, format!("predicate sql: {err}")) + })?; + Ok(sql.to_string()) + } + None => Ok("true".to_string()), + } +} + #[repr(C)] #[derive(Clone, Copy, Debug)] pub struct LanceFieldStats { @@ -699,14 +722,7 @@ fn delete_transaction_with_storage_options_inner( "delete filter_ir", )? }; - let predicate = match filter { - Some(expr) => expr_to_sql(&expr) - .map_err(|err| { - FfiError::new(ErrorCode::DatasetDelete, format!("predicate sql: {err}")) - })? - .to_string(), - None => "true".to_string(), - }; + let predicate = delete_filter_expr_to_sql(filter)?; let session = unsafe { optional_session_handle(session)? }; let (maybe_txn, deleted_rows) = match runtime::block_on(async { @@ -854,14 +870,7 @@ fn dataset_delete_inner( "delete filter_ir", )? }; - let predicate = match filter { - Some(expr) => expr_to_sql(&expr) - .map_err(|err| { - FfiError::new(ErrorCode::DatasetDelete, format!("predicate sql: {err}")) - })? - .to_string(), - None => "true".to_string(), - }; + let predicate = delete_filter_expr_to_sql(filter)?; let mut ds = (*handle.dataset).clone(); diff --git a/src/lance_delete.cpp b/src/lance_delete.cpp index 37eac8e..d5ed85f 100644 --- a/src/lance_delete.cpp +++ b/src/lance_delete.cpp @@ -99,13 +99,12 @@ static bool TryBuildLanceDeleteFilterIR(LogicalDelete &op, vector parts; - vector column_indexes = get->GetColumnIds(); - TableFunctionInitInput init_input(get->bind_data.get(), - std::move(column_indexes), - get->projection_ids, &get->table_filters); - auto table_filters = - BuildLanceTableFilterIRParts(names, types, init_input, false); - parts = std::move(table_filters.parts); + if (!TryBuildLanceTableFilterIRParts(names, types, get->table_filters, + parts)) { + out_error = "unsupported DELETE predicate for Lance: pushed-down table " + "filter could not be translated to Lance filter IR"; + return false; + } vector predicates; if (!TryCollectDeleteFilterPredicates(*op.children[0], predicates, diff --git a/src/lance_filter_ir.cpp b/src/lance_filter_ir.cpp index fbc6b9d..21b1cb0 100644 --- a/src/lance_filter_ir.cpp +++ b/src/lance_filter_ir.cpp @@ -1084,6 +1084,14 @@ bool TryBuildLanceTableFilterIRParts(const vector &names, if (!filter) { continue; } + // OPTIONAL_FILTER / DYNAMIC_FILTER are pruning hints that are not required + // for correctness: DuckDB pushes them as PUSHED_DOWN_PARTIALLY and keeps + // the exact predicate as a residual LogicalFilter (collected separately for + // DELETE). Skip them here rather than failing translation. + if (filter->filter_type == TableFilterType::OPTIONAL_FILTER || + filter->filter_type == TableFilterType::DYNAMIC_FILTER) { + continue; + } if (col_id == COLUMN_IDENTIFIER_ROW_ID || col_id == COLUMN_IDENTIFIER_EMPTY) { return false; diff --git a/test/sql/dml_delete_keyword_column.test b/test/sql/dml_delete_keyword_column.test new file mode 100644 index 0000000..add3906 --- /dev/null +++ b/test/sql/dml_delete_keyword_column.test @@ -0,0 +1,24 @@ +# name: test/sql/dml_delete_keyword_column.test +# description: DELETE predicates on columns whose names are SQL keywords still match rows +# group: [sql] + +require lance + +statement ok +COPY ( + SELECT 'x'::VARCHAR AS name, 1::BIGINT AS id + UNION ALL + SELECT 'y'::VARCHAR AS name, 2::BIGINT AS id +) TO 'test/.tmp/lance_delete_keyword.lance' (FORMAT lance, mode 'overwrite'); + +statement ok +ATTACH 'test/.tmp' AS del_kw_ns (TYPE LANCE); + +# 'name' is a SQL keyword; the predicate must still match and delete the row. +statement ok +DELETE FROM del_kw_ns.main.lance_delete_keyword WHERE name = 'x'; + +query TI +SELECT name, id FROM del_kw_ns.main.lance_delete_keyword ORDER BY id; +---- +y 2 diff --git a/test/sql/dml_delete_non_first_column_filter.test b/test/sql/dml_delete_non_first_column_filter.test new file mode 100644 index 0000000..db44539 --- /dev/null +++ b/test/sql/dml_delete_non_first_column_filter.test @@ -0,0 +1,29 @@ +# name: test/sql/dml_delete_non_first_column_filter.test +# description: DELETE with a predicate on a non-first column deletes only matching rows +# group: [sql] + +require lance + +statement ok +COPY ( + SELECT 1::BIGINT AS id, 'a'::VARCHAR AS s, 0::BIGINT AS flag + UNION ALL + SELECT 2::BIGINT AS id, 'b'::VARCHAR AS s, 1::BIGINT AS flag + UNION ALL + SELECT 3::BIGINT AS id, 'c'::VARCHAR AS s, 0::BIGINT AS flag + UNION ALL + SELECT 4::BIGINT AS id, 'd'::VARCHAR AS s, 1::BIGINT AS flag +) TO 'test/.tmp/lance_delete_non_first.lance' (FORMAT lance, mode 'overwrite'); + +statement ok +ATTACH 'test/.tmp' AS del_col_ns (TYPE LANCE); + +# Predicate on the non-first column: only the flag=1 rows may be removed. +statement ok +DELETE FROM del_col_ns.main.lance_delete_non_first WHERE flag = 1; + +query IT +SELECT id, s FROM del_col_ns.main.lance_delete_non_first ORDER BY id; +---- +1 a +3 c