The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Follow up of #2547493: Add support for unique / primary key constraints composed of multiple fields for Upsert queries.
Problem/Motivation
Use the upsert with composite key capability in core where appropriate.
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|
Issue fork drupal-3586760
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3586760-11x
changes, plain diff MR !16110 - 3586760-use-composite-key changes, plain diff MR !15567
Comments
Comment #3
mondrakeComment #4
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #5
mondrakemerge with main
Comment #6
mondrakeUhm upsert with multiple values seems not to really work. Reverted to 480019bf .
Comment #7
mondrakeKeyValueStore\DatabaseStorage::setMultiple()may be passed an empty array in which case we need to bail out immediately.Comment #8
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #9
mondrakerebased
Comment #10
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #11
mondrakerebased
Comment #12
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #13
mondrakerebased
Comment #14
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #15
mondrakerebased
Comment #16
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #17
mondrakerebased
Comment #18
mondrakeOps, did not notice it was NR and not RTBC. Putting back.
Comment #19
catchLooks good to me, let's make it RTBC after all :)
Comment #20
mondrakeTried to run on all dbs, mysqli seems to have a consistent fail in kernel tests. Checking.
Comment #21
mondrakemysqli failure is due to mysqli and mysql having a different returned value from Upsert::execute() related to internals of affected rows calculations.
See #3569213: Deprecate UPSERT ::execute() method returning a value and some discussions on #2547493: Add support for unique / primary key constraints composed of multiple fields for Upsert queries.
I stand by my opinion expressed there that we should not rely on those returned values in any case. While that would be preferable, here I am just changing the test to reread the data after the rewrite and checking that it still as prior (which is an even more accurate test imho anyway).
Comment #22
mondrakeThanks @catch for review. Mysqli failure was not trivial, so I need to put back to NR for one more round.
Comment #23
mondrakeGreen on all dbs now.
Comment #24
amitgoyal commentedLGTM!
Comment #25
alexpottCommitted and pushed d7f1e63a569 to main. Thanks!
We should backport this to 11.x
Comment #30
mondrakeComment #31
mondrakeAFAICS the functional test failure due to a deprecation occurs in HEAD too
Comment #32
catchBackport looks good!
Comment #33
alexpottCommitted and pushed 8ba2f05e904 to 11.x and c8c3972c9f1 to 11.4.x. Thanks!
Backported to 11.4.x as a nice small performance improvement for using existing API.