Bug #74874 incr/decr commands on integer value work incorrect
Submitted: 15 Nov 2014 0:50 Modified: 16 Dec 2014 19:45
Reporter: Piotr Jurkiewicz (OCA) Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Memcached Severity:S2 (Serious)
Version:5.6.21, 5.6.23 OS:Any
Assigned to: CPU Architecture:Any
Tags: Contribution, memcached

[15 Nov 2014 0:50] Piotr Jurkiewicz
Description:
`incr` and `decr` commands on an integer value column does not work properly. 

`incr` command sets the value to the increment amount, instead of incrementing it.

`decr` command sets the value to 0, instead of decrementing it.

How to repeat:
SQL commands to prepare a database:

USE innodb_memcache;

INSERT INTO containers VALUES ("int_test", "test", "int_test",
			       "k", "v",  "", "", "", "PRIMARY");

CREATE DATABASE IF NOT EXISTS test;
USE test

CREATE TABLE int_test (k varchar(32), v int, PRIMARY KEY (k)) ENGINE=InnoDB;

---

Inside a telnet connection to memcached:

get @@int_test     <-- switch to the int_test database
VALUE @@int_test 0 13
test/int_test
END

set somekey 0 0 3
543
STORED

get somekey
VALUE somekey 0 3
543                <-- integer value is properly set to 543 and then read
END

incr somekey 1
1                  <-- increment that value by 1
 
get somekey
VALUE somekey 0 1
1                  <-- value is set to 1, instead of being incremented by that amount
END

incr somekey 8
8                  <-- increment that value by 8

get somekey
VALUE somekey 0 1
8                  <-- value is set to 8, instead of being incremented by that amount
END

decr somekey 1
0                  <-- decrement that value by 1

get somekey
VALUE somekey 0 1
0                  <-- value is set to 0, instead of being incremented by 1
END

Suggested fix:
In file plugin\innodb_memcached\innodb_memcache\src\innodb_api.c
in function innodb_api_arithmetic() :

it should be checked whether `before_val` is string or integer. If it is an integer it should be casted to uint64_t	`value`, instead of being parsed with strtoull().
[17 Nov 2014 13:35] Umesh Shastry
Hello Piotr Jurkiewicz,

Thank you for the report and test case.

Thanks,
Umesh
[17 Nov 2014 13:36] Umesh Shastry
// 5.6.23

[ushastry@cluster-repo mysql-advanced-5.6.23]$ telnet 127.0.0.1 11211
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
stats
..

STAT engine_maxbytes 67108864
END
get @@int_test
VALUE @@int_test 0 13
test/int_test
END
set somekey 10 0 9
123456789
STORED
get somekey
VALUE somekey 0 9
123456789
END

// mysql> confirmed key=value

mysql> select * from test.int_test;
+---------+-----------+
| k       | v         |
+---------+-----------+
| somekey | 123456789 |
+---------+-----------+
1 row in set (0.00 sec)

// telnet session

incr somekey 1
1

mysql> select * from test.int_test;
+---------+------+
| k       | v    |
+---------+------+
| somekey |    1 |
+---------+------+
1 row in set (0.00 sec)

// telnet session
get somekey
VALUE somekey 0 1
1
END

// telnet session

set a11 10 0 2
99
STORED

// mysql> session
mysql> select * from test.int_test;
+-----+------+
| k   | v    |
+-----+------+
| a11 |   99 |
+-----+------+
1 row in set (0.00 sec)

// incr by 2
get a11
VALUE a11 0 2
99
END
incr a11 2
2
get a11
VALUE a11 0 1
2
END

// mysql> session

mysql> select * from test.int_test;
+-----+------+
| k   | v    |
+-----+------+
| a11 |    2 |
+-----+------+
1 row in set (0.00 sec)

// decr

decr a11 1
0
get a11
VALUE a11 0 1
0
END

mysql> select * from test.int_test;
+-----+------+
| k   | v    |
+-----+------+
| a11 |    0 |
+-----+------+
1 row in set (0.00 sec)

// Build used for tests
[root@cluster-repo mysql-advanced-5.6.23]# more docs/INFO_SRC
revision-id: michael.izioumtchenko@oracle.com-20141106152508-nntohvuco3v1rjjx
date: 2014-11-06 16:25:08 +0100
build-date: 2014-11-06 18:00:43 +0100
revno: 6243
branch-nick: daily-5.6
MySQL source 5.6.23
[11 Dec 2014 13:15] Allen Lai
Posted by developer:
 
We didn't handle the integer column value properately in memcached incr/decr command.  We need to check if the value is integer or string in innodb_api_arithmetic.
[11 Dec 2014 20:26] Piotr Jurkiewicz
Patch fixing this bug

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: incr.diff (text/plain), 1.29 KiB.

[11 Dec 2014 20:29] Piotr Jurkiewicz
Adding tag: contribution.
[11 Dec 2014 20:29] Piotr Jurkiewicz
Adding tag: contribution.
[12 Dec 2014 14:42] Sunny Bains
Piotr,

Thanks for the patch. Your contribution is appreciated. Allen (Zheng Lai) fixed this problem only recently :-)

Regards,
-sunny
[15 Dec 2014 20:04] Daniel Price
Posted by developer:
 
Fixed as of the upcoming 5.7.6 release, and here's the changelog entry:

 The last flushing loop on shutdown did not call
"buf_flush_wait_LRU_batch_end()", resulting in an assertion failure.
[15 Dec 2014 20:05] Daniel Price
Posted by developer:
 
Please disregard the previous comment. It is for a different bug.
[16 Dec 2014 19:45] Daniel Price
Posted by developer:
 
Fixed as of the upcoming 5.6.23, 5.7.6 releases, and here's the changelog entry:

The integer column value was handled incorrectly for the "memcached"
"incr" and "decr" commands.
[12 Feb 2015 13:32] Laurynas Biveinis
$ git show -s 5b1102c
commit 5b1102cf95a6b5f6a3b072055b903fe19b94493b
Author: Allen.Lai <zheng.lai@oracle.com>
Date:   Thu Dec 11 19:44:21 2014 +0800

    Bug#20044123        INCR/DECR COMMANDS ON INTEGER VALUE WORK incorrect
    Bug#20083106        INCR ON A LOCKED RECORD RETURNS THE WRONG VALUE.
    
    We didn't handle the integer column value properately in memcached
    incr/decr command.  We need to check if the value is integer or string
    in innodb_api_arithmetic, and we also need to return the right return
    value of this function.
    
    Reviewed-by: Jimmy Yang<jimmy.yang@oracle.com>
    RB:7490