[Maria-developers] MDEV-9371 select insert('a', 2, 1, 'b') doesn't return expected 'a'
Hello Sergei, Please review a fix for MDEV-9371. I'd suggest to push it to 10.0 instead of 5.5, as MySQL fixed this bug in 5.6 (not 5.5). Also, I think it's worthy to backport MDEV-9332 to 10.0 as well, because the patch for MDEV-9371 touches the test which we modified in MDEV-9332. Thanks!
Hi, Alexander! On Feb 09, Alexander Barkov wrote:
Hello Sergei,
Please review a fix for MDEV-9371.
I'd suggest to push it to 10.0 instead of 5.5, as MySQL fixed this bug in 5.6 (not 5.5).
I think 5.5 is fine. It's clearly a bug. And a very safe fix.
Also, I think it's worthy to backport MDEV-9332 to 10.0 as well, because the patch for MDEV-9371 touches the test which we modified in MDEV-9332.
Dunno. Is 5.5 affected? Or only 10.0 is affected?
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index aca66fc..ca0cc47 100644 --- a/sql/item_strfunc.cc +++ b/sql/item_strfunc.cc @@ -1212,7 +1212,7 @@ String *Item_func_insert::val_str(String *str) length= res->charpos((int) length, (uint32) start);
/* Re-testing with corrected params */ - if (start > res->length()) + if (start + 1 > res->length())
that's fine, of course. But please, add a comment, like // remember, start = args[1].val_int() - 1 or something else, whatever you want, to remind about -1.
return res; /* purecov: inspected */ // Wrong param; skip insert if (length > res->length() - start) length= res->length() - start;
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hi Sergei, On 02/09/2016 05:01 PM, Sergei Golubchik wrote:
Hi, Alexander!
On Feb 09, Alexander Barkov wrote:
Hello Sergei,
Please review a fix for MDEV-9371.
I'd suggest to push it to 10.0 instead of 5.5, as MySQL fixed this bug in 5.6 (not 5.5).
I think 5.5 is fine. It's clearly a bug. And a very safe fix.
Also, I think it's worthy to backport MDEV-9332 to 10.0 as well, because the patch for MDEV-9371 touches the test which we modified in MDEV-9332.
Dunno. Is 5.5 affected? Or only 10.0 is affected?
Both 5.5 and 10.0 are affected. But on the other hand, backporting low level String related fixes (like in MDEV-9332) is somewhat dangerous. Strings (like datetime) are fragile. Ok, never mind. Let's just have everything as planned. I added the commend that you suggested below and pushed to 5.5. Thanks.
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index aca66fc..ca0cc47 100644 --- a/sql/item_strfunc.cc +++ b/sql/item_strfunc.cc @@ -1212,7 +1212,7 @@ String *Item_func_insert::val_str(String *str) length= res->charpos((int) length, (uint32) start);
/* Re-testing with corrected params */ - if (start > res->length()) + if (start + 1 > res->length())
that's fine, of course. But please, add a comment, like
// remember, start = args[1].val_int() - 1
or something else, whatever you want, to remind about -1.
return res; /* purecov: inspected */ // Wrong param; skip insert if (length > res->length() - start) length= res->length() - start;
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
participants (2)
-
Alexander Barkov -
Sergei Golubchik