You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "michaelsembwever (via GitHub)" <gi...@apache.org> on 2026/09/06 21:04:26 UTC
[I] BUG: DELETE with a LIMIT clause ignores the LIMIT and deletes every matching row [datafusion]
michaelsembwever opened a new issue, #24998:
URL: https://github.com/apache/datafusion/issues/24998
### 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
```sql
> 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:
```sql
> 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 #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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] BUG: DELETE with a LIMIT clause ignores the LIMIT and deletes every matching row [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on issue #24998:
URL: https://github.com/apache/datafusion/issues/24998#issuecomment-6017092171
Some investigations:
I've implemented support for DELETE+LIMIT at https://github.com/apache/datafusion/compare/main...martin-g:datafusion:24998-add-support-for-delete-with-limit?expand=1 but I noticed that sqlparser-rs does not support `OFFSET` for `DELETE`. So, I checked what the other major DBs do - only MySQL/MariaDB support `LIMIT` in a `DELETE` statement [[1](https://dev.mysql.com/doc/refman/9.7/en/delete.html)] [[2](https://mariadb.com/docs/server/reference/sql-statements/data-manipulation/changing-deleting-data/delete)]!
Postgres suggests to use a CTE [[3](https://www.postgresql.org/docs/current/sql-delete.html)].
Some RDBMs can emulate it via using ROWNUM or ROWCOUNT.
It seems sqlparser-rs supports `DELETE+LIMIT` for MySQL [[4](https://github.com/apache/datafusion-sqlparser-rs/issues/295)] but IMO it is a bug that it is enabled for the generic dialect too. I will file an issue.
I will abandon my work on adding support for DELETE+LIMIT because without stable order this will be rather dangerous too.
I will help https://github.com/apache/datafusion/pull/25005 to be merged soon!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org