Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-6849

ON UPDATE CURRENT_TIMESTAMP doesn't always work

Details

    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:

      --- a/mysql-test/include/function_defaults.inc
      		 
      +++ b/mysql-test/include/function_defaults.inc
      		 
      @@ -408,15 +408,28 @@ UPDATE t1 SET c = 2;
      		 
       SELECT * FROM t1;
      		 
       
       --echo #
      		 
      +--echo # Test that ON UPDATE CURRENT_TIMESTAMP works after non-changing UPDATE.
      		 
      +--echo #
      		 
      +
      		 
      +--echo # 2011-04-20 09:54:13 UTC
      		 
      +SET TIMESTAMP = 1303293253.794613;
      		 
      +
      		 
      +UPDATE t1 SET c = 2, b = '2011-04-20 09:53:41.794613';
      		 
      +SELECT * FROM t1;
      		 
      +
      		 
      +UPDATE t1 SET c = 3;
      		 
      +SELECT * FROM t1;
      		 
      +
      		 
      +--echo #
      		 
       --echo # Test of multiple-table UPDATE for ON UPDATE CURRENT_TIMESTAMP
      		 
       --echo #
      		 
       --echo # 2011-04-20 15:06:13 UTC
      		 
       SET TIMESTAMP = 1303311973.534231;
      		 
       
      -UPDATE t1 t11, t1 t12 SET t11.c = 2;
      		 
      +UPDATE t1 t11, t1 t12 SET t11.c = 3;
      		 
       SELECT * FROM t1;
      		 
       
      -UPDATE t1 t11, t1 t12 SET t11.c = 3;
      		 
      +UPDATE t1 t11, t1 t12 SET t11.c = 2;
      		 
       SELECT * FROM t1;
      		 
       
       DROP TABLE t1;

      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.

      Attachments

        Issue Links

          relates to

          Bug - A problem which impairs or prevents the functions of the product. MDEV-15602 server occasionally does not update timestamp for column (ON UPDATE CURRENT_TIMESTAMP)

          • Major - Major loss of function.
          • Closed

          Activity

            Ascending order - Click to sort in descending order
            elenst Elena Stepanova added a comment -

            10.0 is affected, MariaDB 5.1-5.5 and MySQL 5.1-5.7 are not.

            elenst Elena Stepanova added a comment - 10.0 is affected, MariaDB 5.1-5.5 and MySQL 5.1-5.7 are not.
            pivanof Pavel Ivanov added a comment -

            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.

            pivanof Pavel Ivanov added a comment - 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.
            serg Sergei Golubchik added a comment -

            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.

            serg Sergei Golubchik added a comment - 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.
            pivanof Pavel Ivanov added a comment -

            Can you please link to the commit that fixed this? I can't find it.

            pivanof Pavel Ivanov added a comment - Can you please link to the commit that fixed this? I can't find it.
            serg Sergei Golubchik added a comment -

            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.

            serg Sergei Golubchik added a comment - 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.
            pivanof Pavel Ivanov added a comment -

            Why you didn't include test for INSERT and didn't add test for LOAD?

            pivanof Pavel Ivanov added a comment - Why you didn't include test for INSERT and didn't add test for LOAD?
            serg Sergei Golubchik added a comment -

            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.

            serg Sergei Golubchik added a comment - 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.
            pivanof Pavel Ivanov added a comment -

            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.

            pivanof Pavel Ivanov added a comment - 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.

            People

              serg Sergei Golubchik
              pivanof Pavel Ivanov
              Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.