as stated by chx at #1209532: Count node views via AJAX in the statistics module, despite what the doxygen says db_merge does not allow multivalue inserts.
at http://api.drupal.org/api/drupal/includes!database!query.inc/class/Merge...
INSERT (column1 [, column2 ...]) VALUES (value1 [, value2 ...
but MergeQuery::execute runs
$this->connection->insert($this->table)->fields($this->insertFields)->execute()
which is not using $insertValues anywhere.
We need to add an insertValues method and change execute to work with it and add unit tests.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | database_multiple_insert_test-1499738-1.patch | 777 bytes | lucascaro |
Comments
Comment #1
lucascaro commentedOk, here is my first attempt to move this issue forward.
Since the problem is that there is no way to add values to a merge query, I've added a simple test case that checks for the existence of the methods value and insertValues.
Please let me know if there is a better way to start. I'm planning on adding those two methods and then create some tests for them.
Is this test I'm posting now really needed or is there another way to do this?
Comment #3
sunHrm. Conceptually, this doesn't make sense to me. The UPDATE query would update the row specified by
key(), but the INSERT query would potentially insert more?Aren't we massively diverging from built-in database driver support for MERGE queries with this? (which we're not leveraging, but those should still lead the conceptual design)
Comment #4
danblack commented> diverging from built-in database driver support for MERGE
Which driver has built in driver support for merge?
I see db_merge with multiple values as a nice convince function rather than convoluted module implementations (#512962-273: Optimize menu_router_build() / _menu_router_save()).
Where unique key(s) are specified to the merge can become.
(or just REPLACE / ON DUPLICATE KEY UPDATE[#965646] in MYSQL).
I may have missed come concept here. Perhaps implementing #1800286: Update, rather than Delete and insert - for significant database performance improvement). I may come across something new. Like it says here #310085: Merge queries using db_merge some things may not make sense.
Doing multivalue changes will also mean the return code can't be a STATUS_INSERT or STATUS_UPDATE. I guess since multiple values are here this is really an API addition and the STATUS code could be changed to STATUS_BOTH_INSERT_UPDATE if really needed.
Comment #5
danblack commentedComment #6
danblack commentedLets look at which multivalue db_merge statements make sense and don't:
Works
update fields/values == insert fields/values where one of the columns is a key
This is the case for bulk updates.
update is expression
merge has constant condition
Update the values where for active rows and otherwise insert them.
merge has multiple inserts (and key isn't unique)
inserts two rows when found otherwise updates those rows (assumes col1 isn't unique).
Doesn't work
multiple insert values where key is unique
merge has multiple inserts and key
Can't insert two rows as unique on col1=red would be violated on second insert.
(most probably incomplete). Is there enough valid cases here to throw InvalidMergeQueryException for the invalid cases?
Comment #7
danblack commentedComment #10
joelpittetThis could use a subsystem maintainer's response on this as it's getting a bit stale
Comment #17
amateescu commentedI think the new UPSERT query in core and the proposed patch from #2547493: Add support for unique / primary key constraints composed of multiple fields for Upsert queries makes this issue obsolete.