michaelsembwever opened a new issue, #24654: URL: https://github.com/apache/datafusion/issues/24654
### Describe the bug A `DELETE` or an `UPDATE` whose `WHERE` clause holds an `IN` subquery or an `EXISTS` subquery applies to all rows of the target table. The statement reports the whole table as affected, and it reports no error. The cause is a chain of three steps. 1. `decorrelate_predicate_subquery` rewrites the subquery into a semi join. The rule builds `JoinType::LeftSemi` for `IN` and for `EXISTS`, and `JoinType::LeftAnti` for the negated forms (`datafusion/optimizer/src/decorrelate_predicate_subquery.rs:277-278`). The condition then lives in the `on` clause of the `Join`, and no longer in a `Filter` above the target `TableScan`. 2. `extract_dml_filters()` collects predicates from `Filter` nodes and from the pushed-down filters of the target `TableScan` (`datafusion/core/src/physical_planner.rs:2227-2325`). It reads a `Join` as a node that "may contain filters in child nodes" and continues past it. For this plan shape it finds nothing, so it returns an empty vector. 3. The physical planner passes that empty vector to `TableProvider::delete_from()` (`datafusion/core/src/physical_planner.rs:799-803`). An empty vector is the encoding for "the statement has no `WHERE` clause", so the provider applies the statement to every row. `MemTable` takes the `None` arm of the mask match and deletes each row of each batch (`datafusion/catalog/src/memory/table.rs:411-419`). The defect is not confined to `MemTable`. Every provider that implements `delete_from()` or `update()` receives the same empty vector, and the documented contract tells the provider to treat it as "all rows". ### To Reproduce ```sql > create table s1 as values (1), (2), (3); > create table s2 as values (2); > delete from s1 where column1 in (select column1 from s2); +-------+ | count | +-------+ | 3 | +-------+ > select * from s1; +---------+ | column1 | +---------+ +---------+ ``` The correct count is 1, and `s1` must keep the rows 1 and 3. The same result follows for these shapes: ```sql delete from s1 where exists (select 1 from s2 where s2.column1 = s1.column1); delete from s1 where column1 not in (select column1 from s2); update s1 set column1 = 0 where column1 in (select column1 from s2); ``` An always-false `WHERE` clause reaches the provider the same way, and it is easier to hit than any subquery. The simplification rule folds the predicate into `EmptyRelation: rows=0`, so no `Filter` node survives: ```sql > create table s3 as values (1), (2), (3); > delete from s3 where false; +-------+ | count | +-------+ | 3 | +-------+ > select * from s3; +---------+ | column1 | +---------+ +---------+ ``` `where 1 = 2` gives the same result. The `UPDATE` form keeps the rows, because the projection assigns each row its own value, but it reports the whole table as updated. The correct count is 0 in each case. A scalar subquery behaves differently, and safely: it reaches the physical planner as an expression that has no physical form, and the statement fails with `This feature is not implemented: Physical plan does not support logical expression ScalarSubquery`. See `datafusion/sqllogictest/test_files/delete.slt:90` and `:107`, which record that error with the optimizer turned off. ### Expected behavior Either of these is acceptable, and the first is the smaller change: 1. The statement fails with a clear "not implemented" error, and the table keeps every row. The provider hook must not run at all, so a provider that writes to durable storage stays untouched. 2. The statement deletes or updates the rows that the subquery selects, and reports that count. The always-false case needs neither answer. The planner already knows that no row matches, so the statement must report a count of 0 and must not call the provider hook. Silence is the one unacceptable answer. A user who writes this statement today loses the table. ### Additional context The documentation added by pull request #24567 records the behaviour as a warning in two places, and neither cites a tracker issue: - `docs/source/user-guide/sql/dml.md`: "An `IN` or an `EXISTS` subquery is worse: the statement applies to **all** rows of the table." - `docs/source/library-user-guide/custom-table-providers.md`: "An `IN` or an `EXISTS` subquery reaches your hook with an empty `filters` vector, because the optimizer rewrites the subquery into a join. Your hook then changes every row, which is the wrong answer. DataFusion does not yet protect a provider against this case." This issue asks for that protection. Related but separate, and out of scope here: - A `LIMIT` on a `DELETE` is ignored. `extract_dml_filters()` walks past the `Limit` node, and the provider sees only the filters. `datafusion/sqllogictest/test_files/delete.slt:121` and `:133` record the plans. - `UPDATE ... FROM` is rejected in the SQL planner, so a multi-table `UPDATE` cannot reach this path. #19950 tracks that gap. No test covers a `DELETE` or an `UPDATE` with a subquery against a populated table. `datafusion/sqllogictest/test_files/delete.slt` sets `datafusion.optimizer.max_passes = 0`, which keeps the subquery in the `Filter` and therefore never reaches the rewritten shape. `dml_delete.slt` and `dml_update.slt` hold the behavioural tests and use no subqueries. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
