← Back to team overview

maria-developers team mailing list archive

MDEV-10306 Wrong results with combination of CONCAT, SUBSTR and CONVERT in subquery

 

Hello Sergei,

Please review a fix for MDEV-10306.

Thanks!
commit af8887b86ccbaea8782cf54fe445cf53aaef7c06
Author: Alexander Barkov <bar@xxxxxxxxxxx>
Date:   Tue Feb 28 10:28:09 2017 +0400

    MDEV-10306 Wrong results with combination of CONCAT, SUBSTR and CONVERT in subquery
    
    The bug happens because of a combination of unfortunate circumstances:
    
    1. Arguments args[0] and args[2] of Item_func_concat point recursively
    (through Item_direct_view_ref's) to the same Item_func_conv_charset.
    Both args[0]->args[0]->ref[0] and args[2]->args[0]->ref[0] refer to
    this Item_func_conv_charset.
    
    2. When Item_func_concat::args[0]->val_str() is called,
    Item_func_conv_charset::val_str() writes its result to
    Item_func_conc_charset::tmp_value.
    
    3. Then, for optimization purposes (to avoid copying),
    Item_func_substr::val_str() initializes Item_func_substr::tmp_value
    to point to the buffer fragment owned by Item_func_conv_charset::tmp_value
    Item_func_substr::tmp_value is returned as a result of
    Item_func_concat::args[0]->val_str().
    
    4. Due to optimization to avoid memory reallocs,
    Item_func_concat::val_str() remembers the result of args[0]->val_str()
    in "res" and further uses "res" to collect the return value.
    
    5. When Item_func_concat::args[2]->val_str() is called,
    Item_func_conv_charset::tmp_value gets overwritten (see #1),
    which effectively overwrites args[0]'s Item_func_substr::tmp_value (see #3),
    which effectively overwrites "res" (see #4).
    
    The fix marks Item_func_substr::tmp_value as a constant string, which
    tells Item_func_concat::val_str "Don't use me as the return value, you cannot
    append to me because I'm pointing to a buffer owned by some other String".

diff --git a/mysql-test/r/func_concat.result b/mysql-test/r/func_concat.result
index 925158a..6dd91d5 100644
--- a/mysql-test/r/func_concat.result
+++ b/mysql-test/r/func_concat.result
@@ -149,3 +149,20 @@ CALL p1();
 ########################################40100.000
 DROP PROCEDURE p1;
 # End of 5.1 tests
+#
+# Start of 10.0 tests
+#
+#
+# MDEV-10306 Wrong results with combination of CONCAT, SUBSTR and CONVERT in subquery
+#
+SET @save_optimizer_switch=@@optimizer_switch;
+SET optimizer_switch='derived_merge=on';
+CREATE TABLE t1 (t VARCHAR(10) CHARSET latin1);
+INSERT INTO t1 VALUES('1234567');
+SELECT CONCAT(SUBSTR(t2, 1, 3), SUBSTR(t2, 5)) c1,
+CONCAT(SUBSTR(t2,1,3),'---',SUBSTR(t2,5)) c2
+FROM (SELECT CONVERT(t USING latin1) t2 FROM t1) sub;
+c1	c2
+123567	123---567
+DROP TABLE t1;
+SET optimizer_switch=@save_optimizer_switch;
diff --git a/mysql-test/t/func_concat.test b/mysql-test/t/func_concat.test
index e56d112..7672965 100644
--- a/mysql-test/t/func_concat.test
+++ b/mysql-test/t/func_concat.test
@@ -145,3 +145,23 @@ CALL p1();
 DROP PROCEDURE p1;
 
 --echo # End of 5.1 tests
+
+
+--echo #
+--echo # Start of 10.0 tests
+--echo #
+
+--echo #
+--echo # MDEV-10306 Wrong results with combination of CONCAT, SUBSTR and CONVERT in subquery
+--echo #
+
+SET @save_optimizer_switch=@@optimizer_switch;
+SET optimizer_switch='derived_merge=on';
+
+CREATE TABLE t1 (t VARCHAR(10) CHARSET latin1);
+INSERT INTO t1 VALUES('1234567');
+SELECT CONCAT(SUBSTR(t2, 1, 3), SUBSTR(t2, 5)) c1,
+       CONCAT(SUBSTR(t2,1,3),'---',SUBSTR(t2,5)) c2
+   FROM (SELECT CONVERT(t USING latin1) t2 FROM t1) sub;
+DROP TABLE t1;
+SET optimizer_switch=@save_optimizer_switch;
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
index 4ea3075..b692a3a 100644
--- a/sql/item_strfunc.cc
+++ b/sql/item_strfunc.cc
@@ -1771,6 +1771,7 @@ String *Item_func_substr::val_str(String *str)
   if (!start && (longlong) res->length() == length)
     return res;
   tmp_value.set(*res, (uint32) start, (uint32) length);
+  tmp_value.mark_as_const();
   return &tmp_value;
 }
 

Follow ups