Implement DELETE statement support in the optimizer and executor#55
Conversation
- Enable DELETE, DROP TABLE, and Transaction (BEGIN, COMMIT, ROLLBACK) parsing in Tokenizer and Parser. - Implement `handle_sql_delete` in QueryExecutor by delegating to a new `delete_execution` module. - Add `delete_execution.rs` which orchestrates row selection via `SELECT` logic, executes physical deletes via `DeleteOperator`, and handles index updates and MVCC Undo Logs. - Fix `test_physical_wal_lsn_integration` test by using public Connection API and verifying successful DELETE execution.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 8d5f078..0d95d75
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (9)
• src/core/query/binder/mod.rs
• src/core/query/executor/delete_execution.rs
• src/core/query/executor/executor.rs
• src/core/query/executor/mod.rs
• src/core/query/sql/ast.rs
• src/core/query/sql/parser/statement.rs
• src/core/query/sql/tokenizer.rs
• src/core/query/sql/translator.rs
• src/core/storage/engine/implementations/tests/file_storage_tests.rs
1 participant
Implemented full support for the SQL DELETE statement. This involved updating the tokenizer and parser to recognize DELETE and transaction-related keywords. The execution logic was implemented in
src/core/query/executor/delete_execution.rs, which handles finding rows to delete, removing them from storage and indexes, and logging undo operations for transaction rollback support. Thetest_physical_wal_lsn_integrationtest was enabled and updated to verify the end-to-end functionality.PR created automatically by Jules for task 9265704686498036509 started by @ryancinsight
High-level PR Summary
This PR implements full support for SQL
DELETEstatements along with transaction control statements (BEGIN,COMMIT,ROLLBACK). The implementation spans the entire query processing pipeline from tokenization and parsing through to physical execution. A newDeleteOperatorhandles row deletion with proper transaction support including undo logging for rollback, index maintenance for both primary key and unique constraints, and WAL integration. The previously disabled integration testtest_physical_wal_lsn_integrationwas re-enabled and updated to verify the complete functionality including DELETE operations within transactional contexts.⏱️ Estimated Review Time: 30-90 minutes
💡 Review Order Suggestion
src/core/query/sql/tokenizer.rssrc/core/query/sql/ast.rssrc/core/query/sql/parser/statement.rssrc/core/query/sql/translator.rssrc/core/query/binder/mod.rssrc/core/query/executor/mod.rssrc/core/query/executor/delete_execution.rssrc/core/query/executor/executor.rssrc/core/storage/engine/implementations/tests/file_storage_tests.rs