MDEV-40927 Crash on long unique insert into partition - #5604
Conversation
gkodinov
added
the
External Contribution
There was a problem hiding this comment.
Pull request overview
Fixes a debug-mode crash when executing INSERT ... PARTITION on partitioned tables with long unique keys and an AUTO_INCREMENT column by ensuring partitions that may be read during long-unique checks are also externally locked for the statement.
Changes:
- Adjust
ha_partition::external_lock()to lockread_partitions(not justlock_partitions) for long-unique tables. - Add a regression test case for MDEV-40927 to
main.long_unique_bugs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sql/ha_partition.cc | Changes partition selection for external_lock() to prevent reading from partitions that weren’t externally locked in the current statement. |
| mysql-test/main/long_unique_bugs.test | Adds a new MDEV-40927 regression test exercising INSERT … PARTITION with long unique hash + AUTO_INCREMENT. |
| mysql-test/main/long_unique_bugs.result | Updates expected output for the new regression test. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution! This is a preliminary review.
I believe co-pilot has a point in what it's saying. Please address that.
Otherwise, LGTM.
INSERT ... PARTITION into a partitioned table with a long unique key prunes lock_partitions to the named partitions, but check_insert_or_replace_autoincrement() widens read_partitions to all. The unique hash check then reads a partition that was never external locked for the statement, tripping the assertion. Lock all read_partitions in ha_partition::external_lock() for write locks (F_WRLCK) on a table with a long unique key, so every partition the check reads is bound to the current statement. lock_partitions is left untouched, so the write stays restricted to the selected partitions. Add a test to main.long_unique_bugs. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
|
@gkodinov I made the change Copilot suggested - could you please take another look? Thank you |
|
FYI: According to our development cycle we work on bugs In the following periods 15 Mar-30 Apr, 15 Jun-30 Jul, 15 Sep-30 Oct and 15 Dec-31 Jan. So, please, expect to get a review somewhere between these two dates and the goal is to have your PR merged before the second date |
Description
INSERT ... PARTITIONinto a partitioned table with a long unique key pruneslock_partitionsto the named partitions, butcheck_insert_or_replace_autoincrement()widensread_partitionsto all.The unique hash check then reads a partition that was never external locked for the statement, tripping the assertion.
Lock all
read_partitionsinha_partition::external_lock()for write locks (F_WRLCK) on a table with a long unique key, so every partition the check reads is bound to the current statement.lock_partitionsis left untouched, so the write stays restricted to the selected partitions.Add a test to
main.long_unique_bugs.Release Notes
N/A
How can this PR be tested?
Execute the main suite in mysql-test-run in debug mode.
This commit adds a test in the
long_unique_bugstest of the main suite (main.long_unique_bugs).Before the fix
Running
main.long_unique_bugs(with the added test) fails as below in debug mode:After the fix
Running
main.long_unique_bugs(with the added test) succeeds in debug mode:Basing the PR against the correct MariaDB version
Copyright
All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.