Add DELETE ... RETURNING clause support - #733
Conversation
|
Per request from Ridha Chahed here's the PR for DELETE .. RETURNING feature ported to trunk. |
|
Thank you for the contribution! |
|
You're right about the standard, haven't checked it properly before posting. |
ebd3532 to
335f58e
Compare
| # | ||
|
|
||
| --disable_warnings | ||
| DROP TABLE IF EXISTS t1, t2; |
There was a problem hiding this comment.
These statements are not necessary, as each test file starts with clean sheets.
| SQL_I_List<Table_ref> *delete_tables; | ||
|
|
||
| /// True if DELETE has a RETURNING clause | ||
| bool m_returning; |
There was a problem hiding this comment.
I think we can place m_returning and has_returning in class Sql_cmd_dml, with default value false.
This is a reasonable change, given that we may soon add RETURNING support for INSERT and UPDATE too.
| the conflict properly would require significant grammar restructuring. | ||
| This is the smallest-impact trade-off. | ||
| */ | ||
| %token<lexer.keyword> RETURNING_SYM 999 /* SQL-2016-N */ |
There was a problem hiding this comment.
I think we can make RETURNING a fully reserved word, since there is not much difference between this definition and a full reservation. The comment above is mostly process-related and can be deleted, I guess.
| | RETAIN_SYM | ||
| | RETURNED_SQLSTATE_SYM | ||
| | RETURNING_SYM | ||
| /* RETURNING_SYM removed, now reserved for DELETE ... RETURNING */ |
| flags = lex->is_explain() ? sp_head::MULTI_RESULTS : 0; | ||
| // DELETE ... RETURNING produces a result set | ||
| if (lex->sql_command == SQLCOM_DELETE && | ||
| lex->m_sql_cmd != nullptr && |
There was a problem hiding this comment.
This check should be redundant, DELETE is always represented by an Sql_cmd object.
| @@ -277,9 +277,12 @@ SELECT JSON_VALUE(json_value, '$.a') AS json_value FROM json_value; | |||
| DROP TABLE json_value; | |||
|
|
|||
| # RETURNING is a non-reserved word both in the standard and in MySQL. | |||
There was a problem hiding this comment.
Replace with "RETURNING is non-reserved word in SQL standard but reserved in MySQL" ?
| --echo # Test 25: EXPLAIN DELETE ... RETURNING (no data returned, no rows deleted) | ||
| --echo # | ||
| --replace_column 10 X | ||
| EXPLAIN DELETE FROM t1 WHERE a=2 RETURNING *; |
There was a problem hiding this comment.
If you add "format=tree" to EXPLAIN, this test file will also run well with --hypergraph.
| 2 BB | ||
| 2 bb | ||
| # | ||
| # Test 6: DELETE ... RETURNING with aggregate function (error) |
There was a problem hiding this comment.
I think we need a similar test with a window function.
| if (m_returning) { | ||
| Table_ref *const table_list = lex->query_block->get_table_list(); | ||
| assert(table_list != nullptr); | ||
| if (table_list == nullptr) return true; // Fail-closed if unexpectedly null |
There was a problem hiding this comment.
I am not sure this will help anything, because the contract for the function is that with a true return, there should be a diagnostics value. I think it is better to delete this statement.
| // items. ORDER BY in DELETE resolves against table columns, not RETURNING, | ||
| // so pass an empty field list. | ||
| mem_root_deque<Item *> empty_fields(thd->mem_root); | ||
| if (setup_order(thd, select->base_ref_items, &tables, |
There was a problem hiding this comment.
I think it will be reasonable to process RETURNING fields before ORDER BY. That way, all fields in "fields" will be resolved, and we don't need the empty list.
Resolving ORDER BY will also populate the empty list with items, which are subquently abandoned. Probably not a good idea...
OTOH, resolving RETURNING before ORDER BY will make us add aliases in the select list, which can later be picked up by ORDER BY, as in:
DELETE FROM t1 WHERE a=1 ORDER BY c RETURNING a+b AS c;
I guess this is an acceptable change.
Implement the RETURNING clause for single-table DELETE statements, allowing the statement to return a result set of the deleted rows. Syntax: DELETE FROM t WHERE ... RETURNING select_expr [, ...] Supported features: - Any SQL expression computable from row fields (columns, functions, arithmetic, subqueries) - Aliases via AS keyword - Wildcard expansion (*, table.*) - Table-qualified column references - Correlated scalar subqueries in RETURNING - IN/EXISTS subqueries in RETURNING - User-defined functions in RETURNING - Works with WHERE, ORDER BY, LIMIT, PARTITION clauses - Works with updatable views - Works with prepared statements - Works with stored procedures (CALL with multi-result protocol) - Works with BEFORE/AFTER DELETE triggers - Respects EXPLAIN (no side effects) - Compatible with ONLY_FULL_GROUP_BY sql_mode Restrictions: - Not allowed in multi-table DELETE (syntax error at parser level) - Aggregate functions not allowed (ER_INVALID_GROUP_FUNC_USE) - Window functions not allowed (ER_WINDOW_INVALID_WINDOW_FUNC_USE) The RETURNING list is resolved before ORDER BY, so ORDER BY may refer to an alias defined in it: DELETE FROM t1 ORDER BY c RETURNING a + b AS c; As in SELECT, such an alias hides a column of the same name, so ORDER BY b in "DELETE FROM t1 ORDER BY b RETURNING a AS b" orders by a. Privileges: RETURNING reads the rows that are deleted, so it requires SELECT in addition to DELETE on the table deleted from. Table-level SELECT_ACL is requested by Sql_cmd_delete::precheck() during preparation, and Sql_cmd_dml::check_all_table_privileges() adds SELECT_ACL for every table marked as deleted from when the statement has a RETURNING clause. The latter covers execution of an already prepared statement, and, since Table_ref::set_deleted() marks the whole referencing view chain, delete targets reached through a view: privileges are checked at each level of the chain, exactly as for a SELECT through the same view. Columns named in the RETURNING list are checked individually as well, so a column-level grant suffices for a list that names only granted columns, while RETURNING * requires table-level SELECT. Incompatible change: RETURNING becomes a reserved word. It was non-reserved before, so unquoted RETURNING is no longer usable as an identifier and must be backtick-quoted. JSON_VALUE(col, path RETURNING <type>) is unaffected, because that production consumes RETURNING as a keyword and never as an identifier. information_schema.KEYWORDS now reports RESERVED=1 for RETURNING. Implementation: - Parser (sql_yacc.yy): Add opt_delete_returning rule to single-table delete_stmt. Remove RETURNING_SYM from ident_keywords_unambiguous and declare the token with a plain %token, making it reserved. The removal is forced. With RETURNING_SYM left in ident_keywords_unambiguous the grammar has one reduce/reduce conflict, in the delete_stmt state following "FROM table_ident": on lookahead RETURNING the parser cannot choose between reducing the empty opt_as, which treats RETURNING as the delete target's table alias, and reducing the empty opt_table_alias, which starts the RETURNING clause. Resolving that in favour of the clause would require restructuring delete_stmt. Given the removal, the token is declared reserved rather than left non-reserved so that information_schema.KEYWORDS stays truthful. gen_keyword_list.cc derives the RESERVED column solely from the presence of <lexer.keyword> on the %token line, never from the ident_keywords_* rules, so a non-reserved declaration would report RESERVED=0 for a word the parser in fact rejects. - Parse tree (parse_tree_nodes.h/.cc): Add opt_returning_list member to PT_delete. Set parsing_place=CTX_SELECT_LIST during contextualization so subqueries get proper outer_context for outer reference resolution. - Command (sql_cmd_dml.h): Add the m_returning flag and has_returning() to Sql_cmd_dml, defaulting to false, and set the flag in the Sql_cmd_delete constructor. - Preparation (sql_delete.cc): In prepare_inner(), resolve the RETURNING list before ORDER BY: expand wildcards via setup_wild(), allocate base_ref_items and resolve the items via setup_fields() with SELECT_ACL, then set up Query_result_send. setup_fields() is called with allow_sum_func=false, which is what rejects aggregate and window functions, so no separate check for them is needed. Skip the multi-table conversion (hypergraph and subquery paths) when RETURNING is present. - Execution (sql_delete.cc): In delete_from_single_table(), skip the delete_all_rows() and read removal optimizations, send the result set metadata before the delete loop and one row after each delete, and complete the statement with end-of-file instead of my_ok(). Two helpers, send_returning_metadata() and send_delete_completed(), hold that logic; the paths that delete no rows at all, including the is_empty_query() one in execute_inner(), send an empty result set (metadata plus end-of-file) rather than an OK packet. - Stored procedures (sp.cc): Flag a DML statement carrying a RETURNING clause with sp_head::MULTI_RESULTS so CALL sets SERVER_MORE_RESULTS_EXISTS, enabling the client multi-result protocol. - Access (sql_lex.h): Make setup_wild() public for use by Sql_cmd_delete. Tests: mysql-test/t/delete_returning.test (29 test cases covering basic expressions, errors, subqueries, ORDER BY and LIMIT, ORDER BY on a RETURNING alias, views, prepared statements, stored procedures, triggers, EXPLAIN in both traditional and TREE format, partitions, sql_mode, and privilege checks on a base table as well as through SQL SECURITY DEFINER and SQL SECURITY INVOKER views) Two existing tests are updated as a consequence of the keyword change: - mysql-test/suite/json/t/json_value.test backtick-quotes the RETURNING identifier it used to spell unquoted. - mysql-test/r/information_schema_keywords.result records RETURNING as RESERVED=1. This contribution is under the OCA signed by Amazon and covering submissions to the MySQL project.
335f58e to
1a9f6f1
Compare
|
Addressed review comments. |
This contribution is under the OCA signed by Amazon and covering submissions to the MySQL project.
What does this change do?
Adds support for a RETURNING clause on single-table DELETE statements, so a DELETE can return a result set built from the rows it deleted instead of just an affected-row count. The clause goes after ORDER BY/LIMIT and accepts the same expression list as a SELECT output list, including *, qualified wildcards, aliases, and subqueries:
DELETE FROM products
WHERE obsolete = 1
ORDER BY created_at
LIMIT 100
RETURNING id, name, created_at;
The rows are sent to the client as an ordinary result set, protocol-identical to a SELECT.
Why is it needed?
Returning data from modified rows is a well-established non-standard SQL pattern: PostgreSQL has supported
it since 8.2, MariaDB since 10.0, SQLite since 3.35 (the SQL standard instead
offers data change delta tables — optional feature T495 of ISO/IEC 9075-2, added
in the 2011 edition — which nobody in the MySQL family implements). It removes the
need to run a SELECT before the DELETE to capture the doomed rows, which matters in
three ways:
*) Audit logging and archival can capture computed or generated column values of deleted rows in one statement rather than a SELECT + DELETE round trip.
*) Queue-table and work-claiming patterns become a single atomic statement, which is the common way to hand rows to a downstream pipeline.
*) It removes a class of application-level race conditions. The SELECT-then-DELETE pattern needs explicit locking or serializable isolation to be correct under concurrency; DELETE ... RETURNING doesn't, because the read and the delete are the same operation.
How was it tested?
Added/updated MTR tests under
mysql-test/t/delete_returning.testscripts/ci/mtr.shpasses locallyRan the relevant full suite (name it): main,innodb
Contributor checklist
I have signed the OCA with the email on these commits
Code is formatted (
scripts/ci/format.sh)Commits are focused with descriptive messages
AI assistance
I did not use AI assistance for this contribution
I used AI assistance for this contribution
If AI assistance was used, describe the tool(s) and extent of use:
Anthropic Opus was used as coding assistant, in test generation and review.
Whole submitted code and tests were manually reviewed and manually tested.
Areas touched
Parser, DELETE query executor.
Side-effects
The change removes RETURNING_SYM from ident_keywords_unambiguous, so unquoted RETURNING can no longer be used as an identifier and must be backtick-quoted. The reasoning is in the sql_yacc.yy comment.