BUG: DELETE with a LIMIT clause ignores the LIMIT and deletes every matching row

[michaelsembwever]

Describe the bug

DELETE FROM t LIMIT n deletes every row that the WHERE clause matches, not n rows.

TableProvider::delete_from(session_state, filters) takes a filter list and nothing else, so a row count has no channel to the provider. The SQL planner does build a Limit node (datafusion/sql/src/statement.rs:2288-2293), and extract_dml_filters() walks past it to reach the Filter and TableScan nodes below (datafusion/core/src/physical_planner.rs:2443). The provider therefore sees the WHERE clause alone and applies it to the whole table.

UPDATE ... LIMIT does not have the bug, because the planner rejects it: "Update-limit clause not supported" (datafusion/sql/src/statement.rs:1168-1170). DELETE accepts the clause and drops it.

To Reproduce

> create table t as values (1), (2), (3);

> delete from t limit 1;
+-------+
| count |
+-------+
| 3     |
+-------+

> select * from t;
++
++

With a WHERE clause the statement deletes every matching row:

> create table u as values (1), (2), (3);

> delete from u where column1 > 1 limit 1;
+-------+
| count |
+-------+
| 2     |
+-------+

> select * from u;
+---------+
| column1 |
+---------+
| 1       |
+---------+

The Limit node is present in the plan and has no effect on the result:

logical_plan
01)Dml: op=[Delete] table=[t]
02)--Limit: skip=0, fetch=1
03)----Filter: t.column1 > Int64(1)
04)------TableScan: t projection=[column1]
physical_plan
01)CooperativeExec
02)--DmlResultExec: rows_affected=2

Expected behavior

Either the statement deletes at most n rows, or DataFusion rejects it.

Rejecting it is the smaller change and the consistent one. UPDATE ... LIMIT is already rejected, and DELETE ... ORDER BY is rejected too (datafusion/sql/src/statement.rs:1207-1209), so a DELETE ... LIMIT n names no row order and picks its n rows arbitrarily. A user who writes the clause is asking for something DataFusion cannot express.

Honouring it needs a second argument on TableProvider::delete_from, and a decision about which rows a provider may choose when no order is given. That is a feature, and it belongs in its own issue.

Additional context

Notes for whoever takes the fix:

• The check belongs next to the UPDATE one in datafusion/sql/src/statement.rs, in the Statement::Delete arm, so the statement fails at planning and never reaches a provider. delete_to_plan() then no longer needs its limit argument.
• Pull request fix(core): reject a DELETE or an UPDATE whose WHERE clause cannot reach the provider #24657 adds classify_dml_input(), which rejects a DELETE whose WHERE clause cannot reach the provider. It lets LogicalPlan::Limit through on purpose, with a comment pointing at this issue (datafusion/core/src/physical_planner.rs:2333-2336). Rejecting the clause in the SQL planner makes that arm unreachable from SQL; keep it, because a caller can still build the plan through LogicalPlanBuilder.
• No test covers DELETE ... LIMIT. dml_delete.slt and delete.slt hold no case with the clause, which is why the behaviour went unnoticed.