Optimize MySQL Merge ODKU syntax with field=VALUES(field)
markus_petrux - August 29, 2008 - 11:57
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | database system |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
Please, excuse me if I missed this in the new DB Layer code. I have just browsed the code from CVS, but it looks to me that a particular feature of MySQL ODKU clausule is not used. ie. field = VALUES(field)
INSERT INTO table (pkey, field_a, field_b) VALUES (1, 2, 3)
ON DUPLICATE KEY UPDATE
field_a = VALUES(field_a),
field_b = VALUES(field_b);With this syntax it is not necessary to duplicate INSERT values for the ODKU clausule. This may be specially important when big TEXT/BLOB field literals/expressions are invoved, such as db_merge() usage in cache_set().
It has impact on the size of the SQL statemente itself, max_allowed_packet, ...
#1
Hm, interesting. I wasn't aware of that syntax. So you're saying that cache_set, for instance, would become:
INSERT INTO cache (cid, data, expire, created, headers) VALUES ('foo', 'huge string', 0, 12345, NULL) ON DUPLICATE KEY UPDATE cid=values(cid), data=values(data), expire=values(expire), headers=values(headers);That does seem like a nice optimization. We can probably use that for the default case (just fields()) and for updateExcept(), but not for update(). Can you roll a patch so we can see if it works?
#2
Not exactly. Note that it is not needed to update the key. ie. this is not needed:
cid=values(cid)So the statement would look something like this:
INSERT INTO cache (cid, data, expire, created, headers) VALUES ('foo', 'huge string', 0, 12345, NULL) ON DUPLICATE KEY UPDATE data=values(data), expire=values(expire), headers=values(headers);Rolling a patch created by looking at the code. It is not tested, but I think it may work, or at least you can get the idea. :)
#3
Rerolled.
$update_fieldsis not really needed in execute() method.#4
I'm very sorry I was not enterily familiar with all merge related methods and I figured I missed the fact that the caller can explicitly specify which fields need to be updated.
Patch re-rolled.
#5
#4 totally doesn't apply at all. :-( I suspect it's because of the whitespace changes that were fixed a week ago. Can you reroll please?
On visual inspection it looks reasonable (and small, which I like), although the comments need a spellcheck and some language cleanup.
#6
There you go. This time I believe I have used correct english and technical words. :-/
I wasn't sure whether to include a link to the MySQL manual for this particular syntax. Here's the function VALUES(col_name), if that helps:
http://dev.mysql.com/doc/refman/5.0/en/miscellaneous-functions.html#func...
#7
Looks like a nice optimization, but I'd like to see Crell approve it first.
#8
Looks good to me, and all database tests pass. Sadly there are Windows linebreaks in #6. :-( Attached patch is identical but removes them. Please don't credit me.
This should really help with things like cache sets.
#9
Committed to CVS HEAD. Thanks! Welcome back, markus_petrux. :)
#10
Automatically closed -- issue fixed for two weeks with no activity.