[feature](fe) Support ORDER BY and LIMIT clauses for UPDATE and DELETE commands#61681
[feature](fe) Support ORDER BY and LIMIT clauses for UPDATE and DELETE commands#61681morrySnow wants to merge 1 commit intoapache:masterfrom
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
TPC-H: Total hot run time: 26742 ms |
TPC-DS: Total hot run time: 169630 ms |
FE UT Coverage ReportIncrement line coverage |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR adds ORDER BY and LIMIT clause support for single-table UPDATE and DELETE statements, consistent with MySQL syntax. The implementation reuses existing queryOrganization grammar and withQueryOrganization builder method.
Critical Checkpoint Conclusions
1. Goal accomplishment: The grammar and builder changes correctly parse ORDER BY/LIMIT for both UPDATE and DELETE, producing the expected logical plan tree (Sort -> Limit above Filter). The approach of routing DELETE with ORDER BY/LIMIT through DeleteFromUsingCommand (INSERT INTO SELECT path) is sound since the simple DeleteFromCommand path only supports predicate-based deletion.
2. Modification clarity: The change is focused and minimal — 2 grammar lines, ~10 lines of builder logic, plus tests.
3. Concurrency: No new concurrency concerns. The changes are in the parser/plan-building phase (single-threaded per query).
4. Lifecycle management: No new lifecycle concerns.
5. Configuration: No new configuration items.
6. Incompatible changes: The grammar is purely additive (new optional clauses). Existing SQL continues to work unchanged because queryOrganization can match empty (both sortClause? and limitClause? are optional).
7. Parallel code paths: The visitUpdate and visitDelete paths are both correctly updated. The withQueryOrganization call is a no-op when neither ORDER BY nor LIMIT is present.
8. Test coverage: Good positive test coverage for both UPDATE and DELETE with ORDER BY, LIMIT, OFFSET, WHERE combinations. Unit tests verify plan tree structure. However, negative test cases are completely missing (see inline comments).
9. Observability: No new observability concerns.
10. Data correctness: The plan tree structure is correct — Sort/Limit operate on the filtered result set before the Project layer adds DELETE_SIGN or UPDATE assignments. The INSERT INTO SELECT execution path handles atomicity.
Issues Found
-
[Medium] Misleading error message on non-UNIQUE KEY tables: When
DELETE FROM dup_table WHERE c1=1 LIMIT 5is executed on a DUPLICATE KEY table, the error message says"delete command on with using clause only supports unique key model"even though the user never used a USING clause. This is because the ORDER BY/LIMIT path forces routing throughDeleteFromUsingCommandwhich has this error message. The error message should be updated or a separate check should be added before routing. Note: plainDELETE FROM dup_table WHERE c1=1(without LIMIT) works fine on DUP tables throughDeleteHandler. -
[Medium] Missing negative test cases: No
test { sql ...; exception ... }blocks in either regression test file. Should test at minimum: ORDER BY on non-existent column, LIMIT on non-UNIQUE KEY table (for DELETE), and LIMIT 0 edge case. -
[Low] Missing UPDATE with LIMIT-only test: The DELETE regression test has a LIMIT-only (no ORDER BY) case, but the UPDATE test does not.
| query = withRelations(query, ((FromRelationsContext) ctx.fromClause()).relations().relation()); | ||
| } | ||
| query = withFilter(query, Optional.ofNullable(ctx.whereClause())); | ||
| query = withQueryOrganization(query, ctx.queryOrganization()); |
There was a problem hiding this comment.
When hasQueryOrganization is true (e.g., DELETE FROM dup_table LIMIT 5), this routes to DeleteFromUsingCommand which rejects non-UNIQUE KEY tables with the error "delete command on with using clause only supports unique key model". This is misleading because the user did not use a USING clause.
Plain DELETE FROM dup_table WHERE c1=1 (no LIMIT) works fine on DUP/AGG tables through DeleteHandler.process(), but adding LIMIT changes the error behavior.
Consider either:
- Updating
DeleteFromUsingCommand.checkTargetTable()to provide a clearer error message when ORDER BY/LIMIT is used (e.g.,"DELETE with ORDER BY/LIMIT only supports unique key model"), or - Adding a dedicated check here before routing to
DeleteFromUsingCommandthat gives a clear error for non-UNIQUE KEY tables.
| // skip 1 (id=10), delete 2 (id=9, id=8) | ||
| sql "DELETE FROM test_delete_obl ORDER BY c1 ASC LIMIT 2 OFFSET 1;" | ||
| order_qt_delete_limit_offset_syntax """SELECT * FROM test_delete_obl ORDER BY id;""" | ||
| } |
There was a problem hiding this comment.
Missing negative test cases. Per review standards, regression tests should include test { sql ...; exception ... } blocks for error scenarios. Recommended additions:
- DELETE with ORDER BY/LIMIT on a DUPLICATE KEY table — should produce a clear error
- DELETE with ORDER BY referencing a non-existent column
- DELETE with LIMIT 0 — verify behavior (no-op or error?)
| // update both c1 and c2 for the 3 rows with largest c1 | ||
| sql "UPDATE test_update_obl SET c1 = 999, c2 = 'top3' ORDER BY c1 DESC LIMIT 3;" | ||
| order_qt_update_multi_set """SELECT * FROM test_update_obl ORDER BY id;""" | ||
| } |
There was a problem hiding this comment.
Missing negative test cases and a LIMIT-only (no ORDER BY) case.
Recommended additions:
test { sql "UPDATE non_unique_table SET c1=1 ORDER BY c2 LIMIT 5"; exception "..." }— UPDATE on non-UNIQUE KEY tabletest { sql "UPDATE test_update_obl SET c1=1 ORDER BY nonexistent LIMIT 5"; exception "..." }— non-existent ORDER BY columnUPDATE test_update_obl SET c1=1 LIMIT 5— LIMIT without ORDER BY (positive case, for parity with DELETE test)
|
PR approved by anyone and no changes requested. |
morrySnow
force-pushed
the
delete-update-limit
branch
from
9fbf062 to
d901526
Compare
…E commands
### What problem does this PR solve?
Issue Number: close #xxx
Problem Summary: MySQL supports ORDER BY and LIMIT clauses in single-table
UPDATE and DELETE statements, but Doris does not. This PR adds support for
these clauses so users can write statements like:
DELETE FROM table1 ORDER BY c1 ASC NULLS FIRST LIMIT 10, 3;
UPDATE table1 SET c1 = 10 ORDER BY c2 LIMIT 100, 20;
The implementation reuses the existing queryOrganization grammar rule and
withQueryOrganization builder method. For DELETE, when ORDER BY or LIMIT is
present, the statement is routed through DeleteFromUsingCommand (INSERT INTO
SELECT path) which naturally supports sort/limit semantics. For UPDATE, the
queryOrganization is applied to the query plan after withFilter.
### Release note
Support ORDER BY and LIMIT clauses in UPDATE and DELETE statements, consistent
with MySQL single-table UPDATE/DELETE syntax.
### Check List (For Author)
- Test: Regression test / Unit Test
- Parser syntax tests in NereidsParserTest
- Plan tree structure tests in LogicalPlanBuilderTest
- DELETE regression tests in test_delete_order_by_limit.groovy
- UPDATE regression tests in test_update_order_by_limit.groovy
- Behavior changed: No
- Does this need documentation: No
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
morrySnow
force-pushed
the
delete-update-limit
branch
from
d901526 to
5d7fb19
Compare
|
run buildall |
TPC-H: Total hot run time: 26487 ms |
TPC-DS: Total hot run time: 168904 ms |
4 participants
What problem does this PR solve?
Issue Number: close #xxx
Problem Summary: MySQL supports ORDER BY and LIMIT clauses in single-table
UPDATE and DELETE statements, but Doris does not. This PR adds support for
these clauses so users can write statements like:
The implementation reuses the existing
queryOrganizationgrammar rule andwithQueryOrganizationbuilder method. For DELETE, when ORDER BY or LIMIT ispresent, the statement is routed through
DeleteFromUsingCommand(INSERT INTOSELECT path) which naturally supports sort/limit semantics. For UPDATE, the
queryOrganization is applied to the query plan after
withFilter.Release note
Support ORDER BY and LIMIT clauses in UPDATE and DELETE statements, consistent
with MySQL single-table UPDATE/DELETE syntax.
Check List (For Author)