Skip to content

fix: reject unsupported DELETE LIMIT - #25005

Open
ryux1 wants to merge 2 commits into
apache:mainfrom
ryux1:fix/24998-reject-delete-limit
Open

fix: reject unsupported DELETE LIMIT#25005
ryux1 wants to merge 2 commits into
apache:mainfrom
ryux1:fix/24998-reject-delete-limit

Conversation

@ryux1

@ryux1 ryux1 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

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.

@github-actions github-actions Bot added sql SQL Planner sqllogictest SQL Logic Tests (.slt) labels Sep 7, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.69%. Comparing base (d25ffaa) to head (380ddd3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #25005   +/-   ##
=======================================
  Coverage   81.69%   81.69%           
=======================================
  Files        1126     1126           
  Lines      415193   415188    -5     
  Branches   415193   415188    -5     
=======================================
- Hits       339182   339181    -1     
+ Misses      56079    56077    -2     
+ Partials    19932    19930    -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants