[MDEV-6849] ON UPDATE CURRENT_TIMESTAMP doesn't always work Created: 2014-10-08 Updated: 2018-03-19 Resolved: 2014-11-21 |
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | Data Manipulation - Update |
| Affects Version/s: | 10.0.14 |
| Fix Version/s: | 10.0.15 |
| Type: | Bug | Priority: | Blocker |
| Reporter: | Pavel Ivanov | Assignee: | Sergei Golubchik |
| Resolution: | Fixed | Votes: | 1 |
| Labels: | None | ||
| Attachments: |
|
||||||||
| Issue Links: |
|
||||||||
| Description |
|
If one executes an UPDATE statement that doesn't really change any values and that has an explicit value for the field with ON UPDATE CURRENT_TIMESTAMP, then the next UPDATE statement (without an explicit value for the timestamp) won't automatically set current timestamp into that field. The problem can be easily revealed with the following addition to the existing test:
In the test output you can see that after "UPDATE t1 SET c = 3" the field a is updated, but the field b is not. The problem is that HAS_EXPLICIT_VALUE is reset in TABLE::update_default_fields() which is called from mysql_update() only when compare_record(table) returns true. So when mysql_update() doesn't change the row the flag is not reset and gets spilled into the next row or even the next statement. I don't know yet what's the appropriate way to fix this problem (i.e. where the flag HAS_EXPLICIT_VALUE should be reset), so I'd really appreciate a speedy response. This problem is especially dangerous due to the fact that its manifestation depends on the order of statements executed on the same TABLE object which can be different between master and slaves. |
| Comments |
| Comment by Elena Stepanova [ 2014-10-08 ] |
|
10.0 is affected, MariaDB 5.1-5.5 and MySQL 5.1-5.7 are not. |
| Comment by Pavel Ivanov [ 2014-10-14 ] |
|
It's curious how a critical bug leading to incorrect query results on MariaDB doesn't get any response for almost a week now. Attaching the patch that we applied to our tree in order to fix this issue. The line numbers may be slightly off, but "patch -p 1" was able to apply this on top of head in bzr. |
| Comment by Sergei Golubchik [ 2014-10-14 ] |
|
pivanof, easy — see our release schedule in Jira. Current plan is to release 10.0.15 on November 10th. It does not matter whether this bug gets any attention today, last week, or only on November 9th. Either way, a fix for this bug can only be released when 10.0.15 is released. You can expect this bug to be fixed in 10.0.15, but the fix won't necessarily happen as soon as the bug is reported. |
| Comment by Pavel Ivanov [ 2014-11-21 ] |
|
Can you please link to the commit that fixed this? I can't find it. |
| Comment by Sergei Golubchik [ 2014-11-21 ] |
|
http://bazaar.launchpad.net/~maria-captains/maria/10.0/revision/4500.1.2 It is similar to yours, also uses TABLE::reset_default_fields() and removes the code that resets the flag from TABLE::update_default_fields. But it also tries to reduce the number of TABLE::reset_default_fields() calls and doesn't do it for every row, if possible. |
| Comment by Pavel Ivanov [ 2014-11-22 ] |
|
Why you didn't include test for INSERT and didn't add test for LOAD? |
| Comment by Sergei Golubchik [ 2014-11-22 ] |
|
There were tests for LOAD and INSERT already, they were failing when I was making the changes and I had to make them all pass again. In fact, I've tried three-four different approaches to fixing this bug, and different INSERT and LOAD DATA tests were failing each time. So, I thought that the existing coverage is good enough already. The test for INSERT ... UPDATE already existed too. |
| Comment by Pavel Ivanov [ 2014-11-22 ] |
|
There are no LOAD and INSERT tests that were failing before this change, which means this behavior can easily regress and nobody will notice that. And actually you can't even be sure that the change fixes the bug until the tests are added. |