Valgrind failure and crash in having.test in Item_in_subselect::init_left_expr_cache()

Bug #598972 reported by Sergey Petrunia
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MariaDB
Invalid
High
Unassigned

Bug Description

After the 5.2->5.3 merge having.test produces the following valgrind warning (same on both 32 and 64 bit):

http://buildbot.askmonty.org/buildbot/builders/work-amd64-valgrind/builds/576/steps/test/logs/stdio :

main.having w1 [ fail ] Found warnings/errors in server log file!
        Test ended at 2010-06-27 01:39:56
line
==1869== Thread 4:
==1869== Conditional jump or move depends on uninitialised value(s)
==1869== at 0x5EFFDC: Item_in_subselect::init_left_expr_cache() (item_subselect.cc:2215)
==1869== by 0x5F4A04: Item_in_subselect::exec() (item_subselect.cc:509)
==1869== by 0x5EEFD8: Item_in_subselect::val_bool() (item_subselect.cc:1133)
==1869== by 0x5B2601: Item_in_optimizer::val_int() (item_cmpfunc.cc:1832)
==1869== by 0x56AB3B: Item::val_bool() (item.cc:184)
==1869== by 0x5B2851: Item_func_not::val_int() (item_cmpfunc.cc:287)
==1869== by 0x7813E8: filesort(THD*, st_table*, st_sort_field*, unsigned int, SQL_SELECT*, unsigned long long, bool, unsigned long long*) (opt_range.h:767)
==1869== by 0x6DF379: create_sort_index(THD*, JOIN*, st_order*, unsigned long long, unsigned long long, bool) (sql_select.cc:15629)
==1869== by 0x6E4C3C: JOIN::exec() (sql_select.cc:2150)
==1869== by 0x6E64F9: mysql_select(THD*, Item***, TABLE_LIST*, unsigned int, List<Item>&, Item*, unsigned int, st_order*, st_order*, Item*, st_order*, unsigned long long, select_result*, st_select_lex_unit*, st_select_lex*) (sql_select.cc:2408)
==1869== by 0x6EACB8: handle_select(THD*, st_lex*, select_result*, unsigned long) (sql_select.cc:277)
==1869== by 0x65C072: execute_sqlcom_select(THD*, TABLE_LIST*) (sql_parse.cc:5081)
==1869== by 0x6637ED: mysql_execute_command(THD*) (sql_parse.cc:2265)
==1869== by 0x66837F: mysql_parse(THD*, char const*, unsigned int, char const**) (sql_parse.cc:6027)
==1869== by 0x668CAF: dispatch_command(enum_server_command, THD*, char*, unsigned int) (sql_parse.cc:1184)
==1869== by 0x669B2C: do_command(THD*) (sql_parse.cc:890)
^ Found warnings in /var/lib/buildbot/maria-slave/work-opensuse-amd64/build/mysql-test/var/1/log/mysqld.1.err
ok

Revision history for this message
Sergey Petrunia (sergefp) wrote :

The branch where this happens is lp:~maria-captains/maria/5.3-merge-from-5.2 .

Revision history for this message
Sergey Petrunia (sergefp) wrote :

A testcase:

--disable_warnings
DROP TABLE if exists t1, t2, t3;
--enable_warnings

CREATE TABLE t1 (f1 INT, f2 VARCHAR(1));
INSERT INTO t1 VALUES (16,'f');
INSERT INTO t1 VALUES (16,'f');
CREATE TABLE t2 (f1 INT, f2 VARCHAR(1));
INSERT INTO t2 VALUES (13,'f');
INSERT INTO t2 VALUES (20,'f');
CREATE TABLE t3 (f1 INT, f2 VARCHAR(1));
INSERT INTO t3 VALUES (7,'f');

SELECT t1.f2 FROM t1
STRAIGHT_JOIN (t2 JOIN t3 ON t3.f2 = t2.f2 ) ON t3 .f2 = t2 .f2
HAVING ('v', 'i') NOT IN (SELECT f2, MIN(f2) FROM t1)
ORDER BY f2;

Revision history for this message
Sergey Petrunia (sergefp) wrote :

The problem is with this piece of code:

bool Item_in_subselect::init_left_expr_cache()
{
  ...
  /*
    If we use end_[send | write]_group to handle complete rows of the outer
    query, make the cache of the left IN operand use Item_field::result_field
    instead of Item_field::field. We need this because normally
    Cached_item_field uses Item::field to fetch field data, while
    copy_ref_key() that copies the left IN operand into a lookup key uses
    Item::result_field. In the case end_[send | write]_group result_field is
    one row behind field.
  */
  end_select= outer_join->join_tab[outer_join->tables-1].next_select;
  if (end_select == end_send_group || end_select == end_write_group)
    use_result_field= TRUE;

It tries to access

   outer_join->join_tab[outer_join->tables-1].next_select (*)

which is not yet set by the time we invoke the subquery. It is not yet set, because we're using the following query plan:

MariaDB [test]> explain SELECT t1.f2 FROM t1
    -> STRAIGHT_JOIN (t2 JOIN t3 ON t3.f2 = t2.f2 ) ON t3 .f2 = t2 .f2
    -> HAVING ('v', 'i') NOT IN (SELECT f2, MIN(f2) FROM t1)
    -> ORDER BY f2;
+----+-------------+-------+--------+---------------+------+---------+------+------+---------------------------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra |
+----+-------------+-------+--------+---------------+------+---------+------+------+---------------------------------+
| 1 | PRIMARY | t3 | system | NULL | NULL | NULL | NULL | 1 | Using temporary; Using filesort |
| 1 | PRIMARY | t1 | ALL | NULL | NULL | NULL | NULL | 2 | |
| 1 | PRIMARY | t2 | ALL | NULL | NULL | NULL | NULL | 2 | Using where; Using join buffer |
| 2 | SUBQUERY | t1 | ALL | NULL | NULL | NULL | NULL | 2 | |
+----+-------------+-------+--------+---------------+------+---------+------+------+---------------------------------+
4 rows in set (0.03 sec)

we first do filesort() over table t1, and then join it with t2. The subquery is evaluated during filesort(), and we don't yet have (*) set at that point.

Revision history for this message
Sergey Petrunia (sergefp) wrote :

I think that the above-quoted part of code uses an approach that's in general, incorrect.

As far as I understand it, the code tries to solve the following issue:

if a subquery in HAVING/select_list/{other part of query that is evaluated after grouping is done} has references to a table column TC, it t needs to look not at TC but at its image in the grouping table (aka "result field").

This wasn't coded correctly.

First,
- The above "if (end_select == ..." ignores the location of the subquery predicate. What if the subquery predicate is in the WHERE, and so is evaluated before the grouping? In that case we won't need to access the "result field", while with this code we'll still do.

Second, I don't see a reason why would Item_in_subselect::init_left_expr_cache() and co. bother at all with the question of whether they should use the "result field"? That question is abstracted away by wrapping needed items inside Item_ref(...) which does val_int() -> val_result() conversions when necessary.
I suspect that if we just removed real_item() calls from new_Cached_item(), then it would use Item_ref objects that would automatically do the right thing and we won't have any need for the above quoted "if (end_select ==..." statement.

Anybody who has an idea about this code please give some feedback.

Changed in maria:
status: New → Confirmed
Revision history for this message
Philip Stoev (pstoev-askmonty) wrote :
Download full text (5.7 KiB)

This is not just a valgrind warning, but can be a full-blown crash. The unsimplified test case is pasted below and will be shortly simplified further.

--disable_warnings
DROP TABLE /*! IF EXISTS */ CC;
DROP TABLE /*! IF EXISTS */ C;
DROP TABLE /*! IF EXISTS */ BB;
DROP TABLE /*! IF EXISTS */ B;
--enable_warnings

CREATE TABLE `CC` (
  `pk` int(11) NOT NULL AUTO_INCREMENT,
  `col_int_key` int(11) DEFAULT NULL,
  `col_date_key` date DEFAULT NULL,
  `col_varchar_key` varchar(1) DEFAULT NULL,
  `col_varchar_nokey` varchar(1) DEFAULT NULL,
  PRIMARY KEY (`pk`),
  KEY `col_int_key` (`col_int_key`),
  KEY `col_date_key` (`col_date_key`),
  KEY `col_varchar_key` (`col_varchar_key`,`col_int_key`)
) ENGINE=MyISAM AUTO_INCREMENT=30 DEFAULT CHARSET=latin1;
INSERT INTO `CC` VALUES (10,8,NULL,'v','v');
INSERT INTO `CC` VALUES (11,9,'2006-06-14','r','r');
INSERT INTO `CC` VALUES (12,9,'2002-09-12','a','a');
INSERT INTO `CC` VALUES (13,186,'2005-02-15','m','m');
INSERT INTO `CC` VALUES (14,NULL,NULL,'y','y');
INSERT INTO `CC` VALUES (15,2,'2008-11-04','j','j');
INSERT INTO `CC` VALUES (16,3,'2004-09-04','d','d');
INSERT INTO `CC` VALUES (17,0,'2006-06-05','z','z');
INSERT INTO `CC` VALUES (18,133,'1900-01-01','e','e');
INSERT INTO `CC` VALUES (19,1,'1900-01-01','h','h');
INSERT INTO `CC` VALUES (20,8,'1900-01-01','b','b');
INSERT INTO `CC` VALUES (21,5,'2005-01-13','s','s');
INSERT INTO `CC` VALUES (22,5,'2006-05-21','e','e');
INSERT INTO `CC` VALUES (23,8,'2003-09-08','j','j');
INSERT INTO `CC` VALUES (24,6,'2006-12-23','e','e');
INSERT INTO `CC` VALUES (25,51,'2006-10-15','f','f');
INSERT INTO `CC` VALUES (26,4,'2005-04-06','v','v');
INSERT INTO `CC` VALUES (27,7,'2008-04-07','x','x');
INSERT INTO `CC` VALUES (28,6,'2006-10-10','m','m');
INSERT INTO `CC` VALUES (29,4,'1900-01-01','c','c');
CREATE TABLE `C` (
  `pk` int(11) NOT NULL AUTO_INCREMENT,
  `col_int_key` int(11) DEFAULT NULL,
  `col_date_key` date DEFAULT NULL,
  `col_varchar_key` varchar(1) DEFAULT NULL,
  `col_varchar_nokey` varchar(1) DEFAULT NULL,
  PRIMARY KEY (`pk`),
  KEY `col_int_key` (`col_int_key`),
  KEY `col_date_key` (`col_date_key`),
  KEY `col_varchar_key` (`col_varchar_key`,`col_int_key`)
) ENGINE=MyISAM AUTO_INCREMENT=21 DEFAULT CHARSET=latin1;
INSERT INTO `C` VALUES (1,2,NULL,'w','w');
INSERT INTO `C` VALUES (2,9,'2001-09-19','m','m');
INSERT INTO `C` VALUES (3,3,'2004-09-12','m','m');
INSERT INTO `C` VALUES (4,9,NULL,'k','k');
INSERT INTO `C` VALUES (5,NULL,'2002-07-19','r','r');
INSERT INTO `C` VALUES (6,9,'2002-12-16','t','t');
INSERT INTO `C` VALUES (7,3,'2006-02-08','j','j');
INSERT INTO `C` VALUES (8,8,'2006-08-28','u','u');
INSERT INTO `C` VALUES (9,8,'2001-04-14','h','h');
INSERT INTO `C` VALUES (10,53,'2000-01-05','o','o');
INSERT INTO `C` VALUES (11,0,'2003-12-06',NULL,NULL);
INSERT INTO `C` VALUES (12,5,'1900-01-01','k','k');
INSERT INTO `C` VALUES (13,166,'2002-11-27','e','e');
INSERT INTO `C` VALUES (14,3,NULL,'n','n');
INSERT INTO `C` VALUES (15,0,'2003-05-27','t','t');
INSERT INTO `C` VALUES (16,1,'2005-05-03','c','c');
INSERT INTO `C` VALUES (17,9,'2001-04-18','m','m');
INSERT INTO `C` VALUES (18,5,'2005-12-27','y','y');
INSERT INTO `C` VALUES (19,6,'2004-08-20',...

Read more...

summary: - Valgrind failure in having.test in
+ Valgrind failure and crash in having.test in
Item_in_subselect::init_left_expr_cache()
Revision history for this message
Philip Stoev (pstoev-askmonty) wrote :

Automatically simplified test case. Further manual simplification, e.g. replacing the two-field IN with a single-field IN results in bug #600958

--disable_warnings
DROP TABLE /*! IF EXISTS */ CC;
DROP TABLE /*! IF EXISTS */ BB;
DROP TABLE /*! IF EXISTS */ B;
--enable_warnings

CREATE TABLE `CC` (
  `pk` int(11) NOT NULL AUTO_INCREMENT,
  `col_int_key` int(11) DEFAULT NULL,
  `col_date_key` date DEFAULT NULL,
  `col_varchar_key` varchar(1) DEFAULT NULL,
  PRIMARY KEY (`pk`),
  KEY `col_int_key` (`col_int_key`),
  KEY `col_date_key` (`col_date_key`),
  KEY `col_varchar_key` (`col_varchar_key`,`col_int_key`)
) ENGINE=MyISAM AUTO_INCREMENT=30 DEFAULT CHARSET=latin1;
INSERT INTO `CC` VALUES (10,8,NULL,'v');
INSERT INTO `CC` VALUES (11,9,'2006-06-14','r');
INSERT INTO `CC` VALUES (12,9,'2002-09-12','a');
INSERT INTO `CC` VALUES (13,186,'2005-02-15','m');
INSERT INTO `CC` VALUES (14,NULL,NULL,'y');
INSERT INTO `CC` VALUES (15,2,'2008-11-04','j');
INSERT INTO `CC` VALUES (16,3,'2004-09-04','d');
INSERT INTO `CC` VALUES (17,0,'2006-06-05','z');
INSERT INTO `CC` VALUES (18,133,'1900-01-01','e');
INSERT INTO `CC` VALUES (19,1,'1900-01-01','h');
INSERT INTO `CC` VALUES (20,8,'1900-01-01','b');
INSERT INTO `CC` VALUES (21,5,'2005-01-13','s');
INSERT INTO `CC` VALUES (22,5,'2006-05-21','e');
INSERT INTO `CC` VALUES (23,8,'2003-09-08','j');
INSERT INTO `CC` VALUES (24,6,'2006-12-23','e');
INSERT INTO `CC` VALUES (25,51,'2006-10-15','f');
INSERT INTO `CC` VALUES (26,4,'2005-04-06','v');
INSERT INTO `CC` VALUES (27,7,'2008-04-07','x');
INSERT INTO `CC` VALUES (28,6,'2006-10-10','m');
INSERT INTO `CC` VALUES (29,4,'1900-01-01','c');
CREATE TABLE `BB` (
  `pk` int(11) NOT NULL AUTO_INCREMENT,
  `col_int_key` int(11) DEFAULT NULL,
  `col_date_key` date DEFAULT NULL,
  `col_varchar_key` varchar(1) DEFAULT NULL,
  PRIMARY KEY (`pk`),
  KEY `col_int_key` (`col_int_key`),
  KEY `col_date_key` (`col_date_key`),
  KEY `col_varchar_key` (`col_varchar_key`,`col_int_key`)
) ENGINE=MyISAM AUTO_INCREMENT=11 DEFAULT CHARSET=latin1;
INSERT INTO `BB` VALUES (10,8,'2002-02-21',NULL);
CREATE TABLE `B` (
  `pk` int(11) NOT NULL AUTO_INCREMENT,
  `col_int_key` int(11) DEFAULT NULL,
  `col_date_key` date DEFAULT NULL,
  `col_varchar_key` varchar(1) DEFAULT NULL,
  PRIMARY KEY (`pk`),
  KEY `col_int_key` (`col_int_key`),
  KEY `col_date_key` (`col_date_key`),
  KEY `col_varchar_key` (`col_varchar_key`,`col_int_key`)
) ENGINE=MyISAM AUTO_INCREMENT=2 DEFAULT CHARSET=latin1;
INSERT INTO `B` VALUES (1,7,'1900-01-01','f');

SELECT `col_date_key`
FROM BB
WHERE ( 5 , 3 ) IN (
SELECT SUBQUERY3_t1 .`col_int_key` , SUM( SUBQUERY3_t2 .`pk` )
FROM B SUBQUERY3_t1 LEFT JOIN BB SUBQUERY3_t2 ON SUBQUERY3_t2 .`col_varchar_key` = SUBQUERY3_t1 .`col_varchar_key` AND SUBQUERY3_t2 .`pk` IN (
SELECT `pk` CHILD_SUBQUERY2_field1
FROM CC
GROUP BY child_subquery2_field1 ) ) ;

Revision history for this message
Sergey Petrunia (sergefp) wrote :

The valgrind warning part has been fixed as part of 5.2->5.3 merge.

The RQG testcase probably wasn't (didn't check), as it is a manifestation of "outer join + semi-join" problem.

Michael Widenius (monty)
Changed in maria:
importance: Undecided → High
milestone: none → 5.3
Revision history for this message
Timour Katchaounov (timour) wrote :

The bug is not reproducible with any combination of subquery-related optimizer switches.
Tried with the latest 5.3 debug build as of 14-06-2011, tip:
------------------------------------------------------------
revno: 3041
committer: Sergey Petrunya <email address hidden>
branch nick: 5.3-push3
timestamp: Mon 2011-06-13 12:41:19 +0400
message:
  Remove redundant code that is a result of a wrong merge.
  (Changeset <email address hidden> moved this loop from one place
  to another, then the merge of <email address hidden> have
  kept both copies).
------------------------------------------------------------

Changed in maria:
status: Confirmed → Invalid
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers