- last update
almost 2 years ago Patch Failed to Apply - last update
almost 2 years ago Patch Failed to Apply - last update
almost 2 years ago Patch Failed to Apply - πΊπΈUnited States JonMcL Brooklyn, NY
@amateescu: Thank you for your work on this.
Unfortunately it no longer applies to 9.5.x or 10.x. Any chance you have rerolled this patch recently?
- πΊπΈUnited States mlncn Minneapolis, MN, USA
Stripped down, MySQL only, no tests, try at an updated patch. Sorry will try to do more. Though i do think the adding the upsert multivalue capability and using elsewhere in Drupal can and should be separated issues.
- Merge request !12959Add support for multiple keys in UPSERT queries. β (Open) created by amateescu
- π·π΄Romania amateescu
Thanks @daffie for the review! Addressed most points in the MR, replying below to the other ones:
Can we use the upsert also for Drupal\Core\KeyValueStore\DatabaseStorage::setIfNotExists()?
Nope, because that method relies on the return status of the merge query, and upsert doesn't support that.
Can we replace all calls to Drupal\Core\Database\Query\Merge? If so, can we deprecate Merge?
Not sure, we can explore that in a separate issue.
The parameter $fields, is that only an array with values or can it also be an array with keys and values?
It's only an array with values.
Can we do some performance testing to make sure that with this patch Drupal becomes faster.
I ran
time ddev drush si demo_umami -y
three times before and after this change:Before:
0,06s user 0,02s system 0% cpu 22,412 total
0,06s user 0,02s system 0% cpu 22,000 total
0,06s user 0,02s system 0% cpu 22,642 totalAfter:
0,05s user 0,03s system 0% cpu 21,223 total
0,05s user 0,03s system 0% cpu 20,941 total
0,06s user 0,02s system 0% cpu 20,550 totalSo it definitely improves things :)