Skip to content

Commit a23b89f

Browse files
ryux1alambmartin-g
authored
fix: reject unsupported DELETE LIMIT (#25005)
## Which issue does this PR close? - Part of #24998; this PR adds a safe rejection until bounded DELETE is implemented. ## Rationale for this change DELETE with LIMIT currently appears to plan successfully, but the table provider receives only the filters and deletes every matching row. Rejecting the unsupported clause prevents a bounded delete from silently becoming an unbounded one and matches the existing UPDATE LIMIT behavior. ## What changes are included in this PR? - reject DELETE LIMIT during SQL planning - remove the unreachable limit construction from delete_to_plan - replace the misleading EXPLAIN snapshots with error regressions, both with and without WHERE - directly cover both planner rejection branches in the SQL integration suite ## What is the testing strategy for this PR? - cargo test -p datafusion-sqllogictest --test sqllogictests -- delete - cargo test -p datafusion-sql --lib (88 passed) - cargo test -p datafusion-sql --test sql_integration plan_delete_rejects_limit (2 passed) - cargo clippy -p datafusion-sql --all-targets -- -D warnings - cargo fmt --all -- --check ## Are there any user-facing changes? Yes. DELETE statements containing LIMIT now return a not-implemented planning error instead of accepting the limit and potentially deleting every matching row. There is no public Rust API change. Implementation and validation were completed with AI coding assistance under the account owner’s direction. --------- Co-authored-by: Andrew Lamb <[email protected]> Co-authored-by: Martin Grigorov <[email protected]> Co-authored-by: Martin Tzvetanov Grigorov <[email protected]>
1 parent 409e3b8 commit a23b89f

3 files changed

Lines changed: 21 additions & 33 deletions

File tree

‎datafusion/sql/src/statement.rs‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,6 +1248,10 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
12481248
plan_err!("Delete-order-by clause not yet supported")?;
12491249
}
12501250

1251+
if limit.is_some() {
1252+
return plan_err!("Delete-limit clause is not supported");
1253+
}
1254+
12511255
if !optimizer_hints.is_empty() {
12521256
plan_err!("Optimizer hints not supported")?;
12531257
}
@@ -1256,7 +1260,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
12561260
}
12571261

12581262
let table_name = self.get_delete_target(from)?;
1259-
self.delete_to_plan(&table_name, selection, limit)
1263+
self.delete_to_plan(&table_name, selection)
12601264
}
12611265

12621266
Statement::Merge(merge) => self.merge_to_plan(merge),
@@ -2346,7 +2350,6 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
23462350
&self,
23472351
table_name: &ObjectName,
23482352
predicate_expr: Option<SQLExpr>,
2349-
limit: Option<SQLExpr>,
23502353
) -> Result<LogicalPlan> {
23512354
// Do a table lookup to verify the table exists
23522355
let table_ref = self.object_name_to_table_reference(table_name.clone())?;
@@ -2360,7 +2363,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
23602363
.build()?;
23612364
let mut planner_context = PlannerContext::new();
23622365

2363-
let mut source = match predicate_expr {
2366+
let source = match predicate_expr {
23642367
None => scan,
23652368
Some(predicate_expr) => {
23662369
let filter_expr =
@@ -2377,14 +2380,6 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
23772380
}
23782381
};
23792382

2380-
if let Some(limit) = limit {
2381-
let empty_schema = DFSchema::empty();
2382-
let limit = self.sql_to_expr(limit, &empty_schema, &mut planner_context)?;
2383-
source = LogicalPlanBuilder::from(source)
2384-
.limit_by_expr(None, Some(limit))?
2385-
.build()?
2386-
}
2387-
23882383
let plan = LogicalPlan::Dml(DmlStatement::new(
23892384
table_ref,
23902385
table_source,

‎datafusion/sql/tests/sql_integration.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -850,6 +850,17 @@ fn plan_delete() {
850850
);
851851
}
852852

853+
#[rstest]
854+
#[case("delete from person limit 1")]
855+
#[case("delete from person where id = 1 limit 1")]
856+
fn plan_delete_rejects_limit(#[case] sql: &str) {
857+
let err = logical_plan(sql).expect_err("DELETE LIMIT should be rejected");
858+
assert_eq!(
859+
err.strip_backtrace(),
860+
"Error during planning: Delete-limit clause is not supported"
861+
);
862+
}
863+
853864
#[test]
854865
fn plan_delete_quoted_identifier_case_sensitive() {
855866
let sql =

‎datafusion/sqllogictest/test_files/delete.slt‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -117,29 +117,11 @@ physical_plan_error This feature is not implemented: Physical plan does not supp
117117

118118
# Delete with limit
119119

120-
query TT
121-
explain delete from t1 limit 10
122-
----
123-
logical_plan
124-
01)Dml: op=[Delete] table=[t1]
125-
02)--Limit: skip=0, fetch=10
126-
03)----TableScan: t1
127-
physical_plan
128-
01)CooperativeExec
129-
02)--MemDeleteExec
120+
statement error DataFusion error: Error during planning: Delete-limit clause is not supported
121+
delete from t1 limit 10;
130122

131-
132-
query TT
133-
explain delete from t1 where a = 1 and b = '2' limit 10
134-
----
135-
logical_plan
136-
01)Dml: op=[Delete] table=[t1]
137-
02)--Limit: skip=0, fetch=10
138-
03)----Filter: CAST(t1.a AS Int64) = Int64(1) AND t1.b = CAST(Utf8("2") AS Utf8View)
139-
04)------TableScan: t1
140-
physical_plan
141-
01)CooperativeExec
142-
02)--MemDeleteExec: predicate=[CAST(a@0 AS Int64) = 1, b@1 = CAST(2 AS Utf8View)]
123+
statement error DataFusion error: Error during planning: Delete-limit clause is not supported
124+
delete from t1 where a = 1 and b = '2' limit 10;
143125

144126
# Config reset
145127
statement ok

0 commit comments

Comments
 (0)