From 7fb17e42cf2f6f309f43907f2db84389d8d895e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 10 Jan 2011 15:34:45 +0200 Subject: [PATCH 01/20] Bug#59181 InnoDB compilation failure on the Sun Studio compiler Define UNIV_PREFETCH_R(add) as sun_prefetch_read_many((void*) addr), because apparently some versions of the Sun library omit the const qualifier. --- storage/innodb_plugin/include/univ.i | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/innodb_plugin/include/univ.i b/storage/innodb_plugin/include/univ.i index bbff8ddf1e3..4425950748b 100644 --- a/storage/innodb_plugin/include/univ.i +++ b/storage/innodb_plugin/include/univ.i @@ -412,7 +412,7 @@ it is read or written. */ /* Use sun_prefetch when compile with Sun Studio */ # define UNIV_EXPECT(expr,value) (expr) # define UNIV_LIKELY_NULL(expr) (expr) -# define UNIV_PREFETCH_R(addr) sun_prefetch_read_many(addr) +# define UNIV_PREFETCH_R(addr) sun_prefetch_read_many((void*) addr) # define UNIV_PREFETCH_RW(addr) sun_prefetch_write_many(addr) #else /* Dummy versions of the macros */ From 634fe860562f7249c55a0f946a1fb50972b9f9ff Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Wed, 12 Jan 2011 17:53:05 +0200 Subject: [PATCH 02/20] Suppress InnoDB warning about long semaphore wait if running under Valgrind Sometimes Valgrind could be extremely slow and could trigger the InnoDB diagnostic message making the test to fail. --- mysql-test/suite/innodb/t/innodb_bug56143.test | 5 +++++ mysql-test/suite/innodb_plugin/t/innodb_bug56143.test | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/mysql-test/suite/innodb/t/innodb_bug56143.test b/mysql-test/suite/innodb/t/innodb_bug56143.test index 1218ae6621c..b69d0048ee8 100644 --- a/mysql-test/suite/innodb/t/innodb_bug56143.test +++ b/mysql-test/suite/innodb/t/innodb_bug56143.test @@ -8,6 +8,11 @@ -- disable_query_log -- disable_result_log +if ($VALGRIND_TEST) +{ + call mtr.add_suppression("InnoDB: Warning: a long semaphore wait:"); +} + SET foreign_key_checks=0; DROP TABLE IF EXISTS bug56143; CREATE TABLE `bug56143` ( diff --git a/mysql-test/suite/innodb_plugin/t/innodb_bug56143.test b/mysql-test/suite/innodb_plugin/t/innodb_bug56143.test index 7c7472303db..0f135a44f5d 100644 --- a/mysql-test/suite/innodb_plugin/t/innodb_bug56143.test +++ b/mysql-test/suite/innodb_plugin/t/innodb_bug56143.test @@ -8,6 +8,11 @@ -- disable_query_log -- disable_result_log +if ($VALGRIND_TEST) +{ + call mtr.add_suppression("InnoDB: Warning: a long semaphore wait:"); +} + SET foreign_key_checks=0; DROP TABLE IF EXISTS bug56143_1; From 9cd4d4984025857782e12e53d32cea5e4b7684e5 Mon Sep 17 00:00:00 2001 From: Jimmy Yang Date: Fri, 14 Jan 2011 09:02:28 -0800 Subject: [PATCH 03/20] Fix Bug#30423 "InnoDBs treatment of NULL in index stats causes bad "rows examined" estimates". This change implements "innodb_stats_method" with options of "nulls_equal", "nulls_unequal" and "null_ignored". rb://553 approved by Marko --- .../suite/innodb/r/innodb_bug30423.result | 95 ++++++++ .../suite/innodb/t/innodb_bug30423.test | 211 ++++++++++++++++++ .../innodb_plugin/r/innodb_bug30423.result | 95 ++++++++ .../innodb_plugin/t/innodb_bug30423.test | 211 ++++++++++++++++++ storage/innobase/btr/btr0cur.c | 146 +++++++++--- storage/innobase/dict/dict0dict.c | 10 + storage/innobase/handler/ha_innodb.cc | 95 +++++++- storage/innobase/include/btr0cur.h | 5 +- storage/innobase/include/dict0mem.h | 6 + storage/innobase/include/rem0cmp.h | 4 + storage/innobase/include/rem0cmp.ic | 2 +- storage/innobase/include/srv0srv.h | 18 ++ storage/innobase/rem/rem0cmp.c | 14 +- storage/innobase/srv/srv0srv.c | 5 + storage/innodb_plugin/ChangeLog | 8 + storage/innodb_plugin/btr/btr0cur.c | 150 ++++++++++--- storage/innodb_plugin/dict/dict0dict.c | 10 + storage/innodb_plugin/handler/ha_innodb.cc | 95 +++++++- storage/innodb_plugin/include/btr0cur.h | 5 +- storage/innodb_plugin/include/dict0mem.h | 6 + storage/innodb_plugin/include/rem0cmp.h | 4 + storage/innodb_plugin/include/rem0cmp.ic | 2 +- storage/innodb_plugin/include/srv0srv.h | 18 ++ storage/innodb_plugin/rem/rem0cmp.c | 14 +- storage/innodb_plugin/srv/srv0srv.c | 5 + 25 files changed, 1157 insertions(+), 77 deletions(-) create mode 100644 mysql-test/suite/innodb/r/innodb_bug30423.result create mode 100644 mysql-test/suite/innodb/t/innodb_bug30423.test create mode 100644 mysql-test/suite/innodb_plugin/r/innodb_bug30423.result create mode 100644 mysql-test/suite/innodb_plugin/t/innodb_bug30423.test diff --git a/mysql-test/suite/innodb/r/innodb_bug30423.result b/mysql-test/suite/innodb/r/innodb_bug30423.result new file mode 100644 index 00000000000..a19809366ae --- /dev/null +++ b/mysql-test/suite/innodb/r/innodb_bug30423.result @@ -0,0 +1,95 @@ +set global innodb_stats_method = default; +select @@innodb_stats_method; +@@innodb_stats_method +nulls_equal +select count(*) from bug30243_3 where org_id is not NULL; +count(*) +20 +select count(*) from bug30243_3 where org_id is NULL; +count(*) +16384 +select count(*) from bug30243_2 where org_id is not NULL; +count(*) +224 +select count(*) from bug30243_2 where org_id is NULL; +count(*) +65536 +select @@innodb_stats_method; +@@innodb_stats_method +nulls_equal +analyze table bug30243_1; +Table Op Msg_type Msg_text +test.bug30243_1 analyze status OK +analyze table bug30243_2; +Table Op Msg_type Msg_text +test.bug30243_2 analyze status OK +analyze table bug30243_3; +Table Op Msg_type Msg_text +test.bug30243_3 analyze status OK +set global innodb_stats_method = "NULL"; +ERROR 42000: Variable 'stats_method' can't be set to the value of 'NULL' +set global innodb_stats_method = "nulls_ignored"; +select @@innodb_stats_method; +@@innodb_stats_method +nulls_ignored +analyze table bug30243_1; +Table Op Msg_type Msg_text +test.bug30243_1 analyze status OK +analyze table bug30243_2; +Table Op Msg_type Msg_text +test.bug30243_2 analyze status OK +analyze table bug30243_3; +Table Op Msg_type Msg_text +test.bug30243_3 analyze status OK +explain SELECT COUNT(*), 0 +FROM bug30243_1 orgs +LEFT JOIN bug30243_3 sa_opportunities +ON orgs.org_id=sa_opportunities.org_id +LEFT JOIN bug30243_2 contacts +ON orgs.org_id=contacts.org_id ; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE orgs index NULL org_id 4 NULL 128 Using index +1 SIMPLE sa_opportunities ref org_id org_id 5 test.orgs.org_id 1 Using index +1 SIMPLE contacts ref contacts$org_id contacts$org_id 5 test.orgs.org_id 1 Using index +select @@innodb_stats_method; +@@innodb_stats_method +nulls_ignored +set global innodb_stats_method = "nulls_unequal"; +select @@innodb_stats_method; +@@innodb_stats_method +nulls_unequal +analyze table bug30243_1; +Table Op Msg_type Msg_text +test.bug30243_1 analyze status OK +analyze table bug30243_2; +Table Op Msg_type Msg_text +test.bug30243_2 analyze status OK +analyze table bug30243_3; +Table Op Msg_type Msg_text +test.bug30243_3 analyze status OK +explain SELECT COUNT(*), 0 +FROM bug30243_1 orgs +LEFT JOIN bug30243_3 sa_opportunities +ON orgs.org_id=sa_opportunities.org_id +LEFT JOIN bug30243_2 contacts +ON orgs.org_id=contacts.org_id; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE orgs index NULL org_id 4 NULL 128 Using index +1 SIMPLE sa_opportunities ref org_id org_id 5 test.orgs.org_id 1 Using index +1 SIMPLE contacts ref contacts$org_id contacts$org_id 5 test.orgs.org_id 1 Using index +SELECT COUNT(*) FROM table_bug30423 WHERE org_id IS NULL; +COUNT(*) +1024 +set global innodb_stats_method = "nulls_unequal"; +analyze table table_bug30423; +Table Op Msg_type Msg_text +test.table_bug30423 analyze status OK +set global innodb_stats_method = "nulls_ignored"; +analyze table table_bug30423; +Table Op Msg_type Msg_text +test.table_bug30423 analyze status OK +set global innodb_stats_method = nulls_equal; +drop table bug30243_2; +drop table bug30243_1; +drop table bug30243_3; +drop table table_bug30423; diff --git a/mysql-test/suite/innodb/t/innodb_bug30423.test b/mysql-test/suite/innodb/t/innodb_bug30423.test new file mode 100644 index 00000000000..f2a3ee8d099 --- /dev/null +++ b/mysql-test/suite/innodb/t/innodb_bug30423.test @@ -0,0 +1,211 @@ +# Test for Bug #30423, InnoDBs treatment of NULL in index stats causes +# bad "rows examined" estimates. +# Implemented InnoDB system variable "innodb_stats_method" with +# "nulls_equal" (default), "nulls_unequal", and "nulls_ignored" options. + +-- source include/have_innodb.inc + +let $innodb_stats_method_orig = `select @@innodb_stats_method`; + +# default setting for innodb_stats_method is "nulls_equal" +set global innodb_stats_method = default; + +select @@innodb_stats_method; + +# create three tables, bug30243_1, bug30243_2 and bug30243_3. +# The test scenario is adopted from original bug #30423 report. +# table bug30243_1 and bug30243_3 have many NULL values + +-- disable_result_log +-- disable_query_log + +DROP TABLE IF EXISTS bug30243_1; +CREATE TABLE bug30243_1 ( + org_id int(11) NOT NULL default '0', + UNIQUE KEY (org_id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +LOCK TABLES bug30243_1 WRITE; +INSERT INTO bug30243_1 VALUES (11),(15),(16),(17),(19),(20),(21),(23),(24), +(25),(26),(27),(28),(29),(30),(31),(32),(33),(34),(35),(37),(38),(40),(41), +(42),(43),(44),(45),(46),(47),(48),(49),(50),(51),(52),(53),(54),(55),(56), +(57),(58),(59),(60),(61),(62),(63),(64),(65),(66),(67),(68),(69),(70),(71), +(72),(73),(74),(75),(76),(77),(78),(79),(80),(81),(82),(83),(84),(85),(86), +(87),(88),(89),(90),(91),(92),(93),(94),(95),(96),(97),(98),(99),(100),(101), +(102),(103),(104),(105),(106),(107),(108),(109),(110),(111),(112),(113),(114), +(115),(116),(117),(118),(119),(120),(121),(122),(123),(124),(125),(126),(127), +(128),(129),(130),(131),(132),(133),(134),(135),(136),(137),(138),(139),(140), +(141),(142),(143),(144),(145); +UNLOCK TABLES; + +DROP TABLE IF EXISTS bug30243_3; +CREATE TABLE bug30243_3 ( + org_id int(11) default NULL, + KEY (org_id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +INSERT INTO bug30243_3 VALUES (NULL); + +begin; +let $i=14; +while ($i) +{ + INSERT INTO bug30243_3 SELECT NULL FROM bug30243_3; + dec $i; +} + +INSERT INTO bug30243_3 VALUES (34),(34),(35),(56),(58),(62),(62),(64),(65),(66),(80),(135),(137),(138),(139),(140),(142),(143),(144),(145); +commit; + +DROP TABLE IF EXISTS bug30243_2; +CREATE TABLE bug30243_2 ( + org_id int(11) default NULL, + KEY `contacts$org_id` (org_id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +INSERT INTO bug30243_2 VALUES (NULL); + +begin; +let $i=16; +while ($i) +{ + INSERT INTO bug30243_2 SELECT NULL FROM bug30243_2; + dec $i; +} + +INSERT INTO bug30243_2 VALUES (11),(15),(16),(17),(20),(21),(23),(24),(25), +(26),(27),(28),(29),(30),(31),(32),(33),(34),(37),(38),(40),(41),(42),(43), +(44),(45),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46), +(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46), +(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46), +(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46), +(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46), +(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(48), +(48),(50),(51),(52),(52),(53),(54),(55),(57),(60),(61),(62),(62),(62),(62), +(62),(63),(64),(64),(65),(66),(66),(67),(68),(69),(70),(71),(72),(73),(74), +(75),(76),(77),(78),(79),(80),(80),(81),(82),(83),(84),(85),(86),(87),(88), +(89),(90),(91),(92),(93),(94),(95),(96),(97),(98),(99),(100),(101),(102), +(103),(104),(105),(106),(107),(108),(109),(110),(111),(112),(113),(114), +(115),(116),(117),(118),(119),(120),(121),(122),(123),(124),(125),(126), +(127),(128),(129),(130),(131),(132),(133),(133),(135),(135),(135),(135), +(136),(136),(138),(138),(139),(139),(139),(140),(141),(141),(142),(143), +(143),(145),(145); +commit; + + +-- enable_result_log +-- enable_query_log + +# check tables's value +select count(*) from bug30243_3 where org_id is not NULL; +select count(*) from bug30243_3 where org_id is NULL; + +select count(*) from bug30243_2 where org_id is not NULL; +select count(*) from bug30243_2 where org_id is NULL; + +select @@innodb_stats_method; + +analyze table bug30243_1; +analyze table bug30243_2; +analyze table bug30243_3; + +# Following query plan shows that we over estimate the rows per +# unique value (since there are many NULLs). +# Skip this query log since the stats estimate could vary from runs +-- disable_query_log +-- disable_result_log +explain SELECT COUNT(*), 0 + FROM bug30243_1 orgs + LEFT JOIN bug30243_3 sa_opportunities + ON orgs.org_id=sa_opportunities.org_id + LEFT JOIN bug30243_2 contacts + ON orgs.org_id=contacts.org_id ; +-- enable_query_log +-- enable_result_log + +# following set operation will fail +#--error ER_WRONG_VALUE_FOR_VAR +--error 1231 +set global innodb_stats_method = "NULL"; + +set global innodb_stats_method = "nulls_ignored"; + +select @@innodb_stats_method; + +# Regenerate the stats with "nulls_ignored" option + +analyze table bug30243_1; +analyze table bug30243_2; +analyze table bug30243_3; + +# Following query plan shows that we get the correct rows per +# unique value (should be approximately 1 row per value) +explain SELECT COUNT(*), 0 + FROM bug30243_1 orgs + LEFT JOIN bug30243_3 sa_opportunities + ON orgs.org_id=sa_opportunities.org_id + LEFT JOIN bug30243_2 contacts + ON orgs.org_id=contacts.org_id ; + +select @@innodb_stats_method; + +# Try the "nulls_unequal" option +set global innodb_stats_method = "nulls_unequal"; + +select @@innodb_stats_method; + +analyze table bug30243_1; +analyze table bug30243_2; +analyze table bug30243_3; + +# Following query plan shows that we get the correct rows per +# unique value (~1) +explain SELECT COUNT(*), 0 + FROM bug30243_1 orgs + LEFT JOIN bug30243_3 sa_opportunities + ON orgs.org_id=sa_opportunities.org_id + LEFT JOIN bug30243_2 contacts + ON orgs.org_id=contacts.org_id; + + +# Create a table with all NULL values, make sure the stats calculation +# does not crash with table of all NULL values +-- disable_query_log +CREATE TABLE table_bug30423 ( + org_id int(11) default NULL, + KEY(org_id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +INSERT INTO `table_bug30423` VALUES (NULL); + +begin; +let $i=10; +while ($i) +{ + INSERT INTO table_bug30423 SELECT NULL FROM table_bug30423; + dec $i; +} +commit; + +-- enable_query_log + +SELECT COUNT(*) FROM table_bug30423 WHERE org_id IS NULL; + +# calculate the statistics for the table for "nulls_ignored" and +# "nulls_unequal" option +set global innodb_stats_method = "nulls_unequal"; +analyze table table_bug30423; + +set global innodb_stats_method = "nulls_ignored"; +analyze table table_bug30423; + + +eval set global innodb_stats_method = $innodb_stats_method_orig; + +drop table bug30243_2; + +drop table bug30243_1; + +drop table bug30243_3; + +drop table table_bug30423; diff --git a/mysql-test/suite/innodb_plugin/r/innodb_bug30423.result b/mysql-test/suite/innodb_plugin/r/innodb_bug30423.result new file mode 100644 index 00000000000..a19809366ae --- /dev/null +++ b/mysql-test/suite/innodb_plugin/r/innodb_bug30423.result @@ -0,0 +1,95 @@ +set global innodb_stats_method = default; +select @@innodb_stats_method; +@@innodb_stats_method +nulls_equal +select count(*) from bug30243_3 where org_id is not NULL; +count(*) +20 +select count(*) from bug30243_3 where org_id is NULL; +count(*) +16384 +select count(*) from bug30243_2 where org_id is not NULL; +count(*) +224 +select count(*) from bug30243_2 where org_id is NULL; +count(*) +65536 +select @@innodb_stats_method; +@@innodb_stats_method +nulls_equal +analyze table bug30243_1; +Table Op Msg_type Msg_text +test.bug30243_1 analyze status OK +analyze table bug30243_2; +Table Op Msg_type Msg_text +test.bug30243_2 analyze status OK +analyze table bug30243_3; +Table Op Msg_type Msg_text +test.bug30243_3 analyze status OK +set global innodb_stats_method = "NULL"; +ERROR 42000: Variable 'stats_method' can't be set to the value of 'NULL' +set global innodb_stats_method = "nulls_ignored"; +select @@innodb_stats_method; +@@innodb_stats_method +nulls_ignored +analyze table bug30243_1; +Table Op Msg_type Msg_text +test.bug30243_1 analyze status OK +analyze table bug30243_2; +Table Op Msg_type Msg_text +test.bug30243_2 analyze status OK +analyze table bug30243_3; +Table Op Msg_type Msg_text +test.bug30243_3 analyze status OK +explain SELECT COUNT(*), 0 +FROM bug30243_1 orgs +LEFT JOIN bug30243_3 sa_opportunities +ON orgs.org_id=sa_opportunities.org_id +LEFT JOIN bug30243_2 contacts +ON orgs.org_id=contacts.org_id ; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE orgs index NULL org_id 4 NULL 128 Using index +1 SIMPLE sa_opportunities ref org_id org_id 5 test.orgs.org_id 1 Using index +1 SIMPLE contacts ref contacts$org_id contacts$org_id 5 test.orgs.org_id 1 Using index +select @@innodb_stats_method; +@@innodb_stats_method +nulls_ignored +set global innodb_stats_method = "nulls_unequal"; +select @@innodb_stats_method; +@@innodb_stats_method +nulls_unequal +analyze table bug30243_1; +Table Op Msg_type Msg_text +test.bug30243_1 analyze status OK +analyze table bug30243_2; +Table Op Msg_type Msg_text +test.bug30243_2 analyze status OK +analyze table bug30243_3; +Table Op Msg_type Msg_text +test.bug30243_3 analyze status OK +explain SELECT COUNT(*), 0 +FROM bug30243_1 orgs +LEFT JOIN bug30243_3 sa_opportunities +ON orgs.org_id=sa_opportunities.org_id +LEFT JOIN bug30243_2 contacts +ON orgs.org_id=contacts.org_id; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE orgs index NULL org_id 4 NULL 128 Using index +1 SIMPLE sa_opportunities ref org_id org_id 5 test.orgs.org_id 1 Using index +1 SIMPLE contacts ref contacts$org_id contacts$org_id 5 test.orgs.org_id 1 Using index +SELECT COUNT(*) FROM table_bug30423 WHERE org_id IS NULL; +COUNT(*) +1024 +set global innodb_stats_method = "nulls_unequal"; +analyze table table_bug30423; +Table Op Msg_type Msg_text +test.table_bug30423 analyze status OK +set global innodb_stats_method = "nulls_ignored"; +analyze table table_bug30423; +Table Op Msg_type Msg_text +test.table_bug30423 analyze status OK +set global innodb_stats_method = nulls_equal; +drop table bug30243_2; +drop table bug30243_1; +drop table bug30243_3; +drop table table_bug30423; diff --git a/mysql-test/suite/innodb_plugin/t/innodb_bug30423.test b/mysql-test/suite/innodb_plugin/t/innodb_bug30423.test new file mode 100644 index 00000000000..458c2967e19 --- /dev/null +++ b/mysql-test/suite/innodb_plugin/t/innodb_bug30423.test @@ -0,0 +1,211 @@ +# Test for Bug #30423, InnoDBs treatment of NULL in index stats causes +# bad "rows examined" estimates. +# Implemented InnoDB system variable "innodb_stats_method" with +# "nulls_equal" (default), "nulls_unequal", and "nulls_ignored" options. + +-- source include/have_innodb_plugin.inc + +let $innodb_stats_method_orig = `select @@innodb_stats_method`; + +# default setting for innodb_stats_method is "nulls_equal" +set global innodb_stats_method = default; + +select @@innodb_stats_method; + +# create three tables, bug30243_1, bug30243_2 and bug30243_3. +# The test scenario is adopted from original bug #30423 report. +# table bug30243_1 and bug30243_3 have many NULL values + +-- disable_result_log +-- disable_query_log + +DROP TABLE IF EXISTS bug30243_1; +CREATE TABLE bug30243_1 ( + org_id int(11) NOT NULL default '0', + UNIQUE KEY (org_id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +LOCK TABLES bug30243_1 WRITE; +INSERT INTO bug30243_1 VALUES (11),(15),(16),(17),(19),(20),(21),(23),(24), +(25),(26),(27),(28),(29),(30),(31),(32),(33),(34),(35),(37),(38),(40),(41), +(42),(43),(44),(45),(46),(47),(48),(49),(50),(51),(52),(53),(54),(55),(56), +(57),(58),(59),(60),(61),(62),(63),(64),(65),(66),(67),(68),(69),(70),(71), +(72),(73),(74),(75),(76),(77),(78),(79),(80),(81),(82),(83),(84),(85),(86), +(87),(88),(89),(90),(91),(92),(93),(94),(95),(96),(97),(98),(99),(100),(101), +(102),(103),(104),(105),(106),(107),(108),(109),(110),(111),(112),(113),(114), +(115),(116),(117),(118),(119),(120),(121),(122),(123),(124),(125),(126),(127), +(128),(129),(130),(131),(132),(133),(134),(135),(136),(137),(138),(139),(140), +(141),(142),(143),(144),(145); +UNLOCK TABLES; + +DROP TABLE IF EXISTS bug30243_3; +CREATE TABLE bug30243_3 ( + org_id int(11) default NULL, + KEY (org_id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +INSERT INTO bug30243_3 VALUES (NULL); + +begin; +let $i=14; +while ($i) +{ + INSERT INTO bug30243_3 SELECT NULL FROM bug30243_3; + dec $i; +} + +INSERT INTO bug30243_3 VALUES (34),(34),(35),(56),(58),(62),(62),(64),(65),(66),(80),(135),(137),(138),(139),(140),(142),(143),(144),(145); +commit; + +DROP TABLE IF EXISTS bug30243_2; +CREATE TABLE bug30243_2 ( + org_id int(11) default NULL, + KEY `contacts$org_id` (org_id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +INSERT INTO bug30243_2 VALUES (NULL); + +begin; +let $i=16; +while ($i) +{ + INSERT INTO bug30243_2 SELECT NULL FROM bug30243_2; + dec $i; +} + +INSERT INTO bug30243_2 VALUES (11),(15),(16),(17),(20),(21),(23),(24),(25), +(26),(27),(28),(29),(30),(31),(32),(33),(34),(37),(38),(40),(41),(42),(43), +(44),(45),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46), +(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46), +(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46), +(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46), +(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46), +(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(46),(48), +(48),(50),(51),(52),(52),(53),(54),(55),(57),(60),(61),(62),(62),(62),(62), +(62),(63),(64),(64),(65),(66),(66),(67),(68),(69),(70),(71),(72),(73),(74), +(75),(76),(77),(78),(79),(80),(80),(81),(82),(83),(84),(85),(86),(87),(88), +(89),(90),(91),(92),(93),(94),(95),(96),(97),(98),(99),(100),(101),(102), +(103),(104),(105),(106),(107),(108),(109),(110),(111),(112),(113),(114), +(115),(116),(117),(118),(119),(120),(121),(122),(123),(124),(125),(126), +(127),(128),(129),(130),(131),(132),(133),(133),(135),(135),(135),(135), +(136),(136),(138),(138),(139),(139),(139),(140),(141),(141),(142),(143), +(143),(145),(145); +commit; + + +-- enable_result_log +-- enable_query_log + +# check tables's value +select count(*) from bug30243_3 where org_id is not NULL; +select count(*) from bug30243_3 where org_id is NULL; + +select count(*) from bug30243_2 where org_id is not NULL; +select count(*) from bug30243_2 where org_id is NULL; + +select @@innodb_stats_method; + +analyze table bug30243_1; +analyze table bug30243_2; +analyze table bug30243_3; + +# Following query plan shows that we over estimate the rows per +# unique value (since there are many NULLs). +# Skip this query log since the stats estimate could vary from runs +-- disable_query_log +-- disable_result_log +explain SELECT COUNT(*), 0 + FROM bug30243_1 orgs + LEFT JOIN bug30243_3 sa_opportunities + ON orgs.org_id=sa_opportunities.org_id + LEFT JOIN bug30243_2 contacts + ON orgs.org_id=contacts.org_id ; +-- enable_query_log +-- enable_result_log + +# following set operation will fail +#--error ER_WRONG_VALUE_FOR_VAR +--error 1231 +set global innodb_stats_method = "NULL"; + +set global innodb_stats_method = "nulls_ignored"; + +select @@innodb_stats_method; + +# Regenerate the stats with "nulls_ignored" option + +analyze table bug30243_1; +analyze table bug30243_2; +analyze table bug30243_3; + +# Following query plan shows that we get the correct rows per +# unique value (should be approximately 1 row per value) +explain SELECT COUNT(*), 0 + FROM bug30243_1 orgs + LEFT JOIN bug30243_3 sa_opportunities + ON orgs.org_id=sa_opportunities.org_id + LEFT JOIN bug30243_2 contacts + ON orgs.org_id=contacts.org_id ; + +select @@innodb_stats_method; + +# Try the "nulls_unequal" option +set global innodb_stats_method = "nulls_unequal"; + +select @@innodb_stats_method; + +analyze table bug30243_1; +analyze table bug30243_2; +analyze table bug30243_3; + +# Following query plan shows that we get the correct rows per +# unique value (~1) +explain SELECT COUNT(*), 0 + FROM bug30243_1 orgs + LEFT JOIN bug30243_3 sa_opportunities + ON orgs.org_id=sa_opportunities.org_id + LEFT JOIN bug30243_2 contacts + ON orgs.org_id=contacts.org_id; + + +# Create a table with all NULL values, make sure the stats calculation +# does not crash with table of all NULL values +-- disable_query_log +CREATE TABLE table_bug30423 ( + org_id int(11) default NULL, + KEY(org_id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +INSERT INTO `table_bug30423` VALUES (NULL); + +begin; +let $i=10; +while ($i) +{ + INSERT INTO table_bug30423 SELECT NULL FROM table_bug30423; + dec $i; +} +commit; + +-- enable_query_log + +SELECT COUNT(*) FROM table_bug30423 WHERE org_id IS NULL; + +# calculate the statistics for the table for "nulls_ignored" and +# "nulls_unequal" option +set global innodb_stats_method = "nulls_unequal"; +analyze table table_bug30423; + +set global innodb_stats_method = "nulls_ignored"; +analyze table table_bug30423; + + +eval set global innodb_stats_method = $innodb_stats_method_orig; + +drop table bug30243_2; + +drop table bug30243_1; + +drop table bug30243_3; + +drop table table_bug30423; diff --git a/storage/innobase/btr/btr0cur.c b/storage/innobase/btr/btr0cur.c index a7160d74a32..9f4babfaae6 100644 --- a/storage/innobase/btr/btr0cur.c +++ b/storage/innobase/btr/btr0cur.c @@ -66,6 +66,13 @@ this many index pages */ /*--------------------------------------*/ #define BTR_BLOB_HDR_SIZE 8 +/* Estimated table level stats from sampled value. */ +#define BTR_TABLE_STATS_FROM_SAMPLE(value, index, ext_size, not_empty) \ + ((value * (ib_longlong) index->stat_n_leaf_pages \ + + BTR_KEY_VAL_ESTIMATE_N_PAGES - 1 + ext_size \ + + not_empty) \ + / (BTR_KEY_VAL_ESTIMATE_N_PAGES + ext_size)) + /*********************************************************************** Marks all extern fields in a record as owned by the record. This function should be called if the delete mark of a record is removed: a not delete @@ -2834,10 +2841,55 @@ btr_estimate_n_rows_in_range( } } +/*********************************************************************** +Record the number of non_null key values in a given index for +each n-column prefix of the index where n < dict_index_get_n_unique(index). +The estimates are eventually stored in the array: +index->stat_n_non_null_key_vals. */ +static +void +btr_record_not_null_field_in_rec( +/*=============================*/ + rec_t* rec, /* in: physical record */ + ulint n_unique, /* in: dict_index_get_n_unique(index), + number of columns uniquely determine + an index entry */ + const ulint* offsets, /* in: rec_get_offsets(rec, index), + its size could be for all fields or + that of "n_unique" */ + ib_longlong* n_not_null) /* in/out: array to record number of + not null rows for n-column prefix */ +{ + ulint i; + + ut_ad(rec_offs_n_fields(offsets) >= n_unique); + + if (n_not_null == NULL) { + return; + } + + for (i = 0; i < n_unique; i++) { + ulint rec_len; + byte* field; + + field = rec_get_nth_field(rec, offsets, i, &rec_len); + + if (rec_len != UNIV_SQL_NULL) { + n_not_null[i]++; + } else { + /* Break if we hit the first NULL value */ + break; + } + } +} + /*********************************************************************** Estimates the number of different key values in a given index, for each n-column prefix of the index where n <= dict_index_get_n_unique(index). -The estimates are stored in the array index->stat_n_diff_key_vals. */ +The estimates are stored in the array index->stat_n_diff_key_vals. +If innodb_stats_method is "nulls_ignored", we also record the number of +non-null values for each prefix and store the estimates in +array index->stat_n_non_null_key_vals. */ void btr_estimate_number_of_different_key_vals( @@ -2851,6 +2903,8 @@ btr_estimate_number_of_different_key_vals( ulint matched_fields; ulint matched_bytes; ib_longlong* n_diff; + ib_longlong* n_not_null; + ibool stats_null_not_equal; ulint not_empty_flag = 0; ulint total_external_size = 0; ulint i; @@ -2858,24 +2912,47 @@ btr_estimate_number_of_different_key_vals( ulint add_on; mtr_t mtr; mem_heap_t* heap = NULL; - ulint offsets_rec_[REC_OFFS_NORMAL_SIZE]; - ulint offsets_next_rec_[REC_OFFS_NORMAL_SIZE]; - ulint* offsets_rec = offsets_rec_; - ulint* offsets_next_rec= offsets_next_rec_; - *offsets_rec_ = (sizeof offsets_rec_) / sizeof *offsets_rec_; - *offsets_next_rec_ - = (sizeof offsets_next_rec_) / sizeof *offsets_next_rec_; + ulint* offsets_rec = NULL; + ulint* offsets_next_rec = NULL; n_cols = dict_index_get_n_unique(index); - n_diff = mem_alloc((n_cols + 1) * sizeof(ib_longlong)); + heap = mem_heap_create((sizeof *n_diff + sizeof *n_not_null) + * (n_cols + 1) + + dict_index_get_n_fields(index) + * (sizeof *offsets_rec + + sizeof *offsets_next_rec)); - memset(n_diff, 0, (n_cols + 1) * sizeof(ib_longlong)); + n_diff = mem_heap_zalloc(heap, (n_cols + 1) * sizeof(ib_longlong)); + + n_not_null = NULL; + + /* Check srv_innodb_stats_method setting, and decide whether we + need to record non-null value and also decide if NULL is + considered equal (by setting stats_null_not_equal value) */ + switch (srv_innodb_stats_method) { + case SRV_STATS_NULLS_IGNORED: + n_not_null = mem_heap_zalloc(heap, (n_cols + 1) + * sizeof *n_not_null); + /* fall through */ + + case SRV_STATS_NULLS_UNEQUAL: + /* for both SRV_STATS_NULLS_IGNORED and SRV_STATS_NULLS_UNEQUAL + case, we will treat NULLs as unequal value */ + stats_null_not_equal = TRUE; + break; + + case SRV_STATS_NULLS_EQUAL: + stats_null_not_equal = FALSE; + break; + + default: + ut_error; + } /* We sample some pages in the index to get an estimate */ for (i = 0; i < BTR_KEY_VAL_ESTIMATE_N_PAGES; i++) { - rec_t* supremum; mtr_start(&mtr); btr_cur_open_at_rnd_pos(index, BTR_SEARCH_LEAF, &cursor, &mtr); @@ -2888,18 +2965,22 @@ btr_estimate_number_of_different_key_vals( page = btr_cur_get_page(&cursor); - supremum = page_get_supremum_rec(page); rec = page_rec_get_next(page_get_infimum_rec(page)); - if (rec != supremum) { + if (!page_rec_is_supremum(rec)) { not_empty_flag = 1; offsets_rec = rec_get_offsets(rec, index, offsets_rec, ULINT_UNDEFINED, &heap); + + if (n_not_null) { + btr_record_not_null_field_in_rec( + rec, n_cols, offsets_rec, n_not_null); + } } - while (rec != supremum) { + while (!page_rec_is_supremum(rec)) { rec_t* next_rec = page_rec_get_next(rec); - if (next_rec == supremum) { + if (page_rec_is_supremum(next_rec)) { break; } @@ -2911,7 +2992,8 @@ btr_estimate_number_of_different_key_vals( cmp_rec_rec_with_match(rec, next_rec, offsets_rec, offsets_next_rec, - index, &matched_fields, + index, stats_null_not_equal, + &matched_fields, &matched_bytes); for (j = matched_fields + 1; j <= n_cols; j++) { @@ -2921,6 +3003,12 @@ btr_estimate_number_of_different_key_vals( n_diff[j]++; } + if (n_not_null) { + btr_record_not_null_field_in_rec( + next_rec, n_cols, offsets_next_rec, + n_not_null); + } + total_external_size += btr_rec_get_externally_stored_len( rec, offsets_rec); @@ -2971,14 +3059,8 @@ btr_estimate_number_of_different_key_vals( included in index->stat_n_leaf_pages) */ for (j = 0; j <= n_cols; j++) { - index->stat_n_diff_key_vals[j] - = ((n_diff[j] - * (ib_longlong)index->stat_n_leaf_pages - + BTR_KEY_VAL_ESTIMATE_N_PAGES - 1 - + total_external_size - + not_empty_flag) - / (BTR_KEY_VAL_ESTIMATE_N_PAGES - + total_external_size)); + index->stat_n_diff_key_vals[j] = BTR_TABLE_STATS_FROM_SAMPLE( + n_diff[j], index, total_external_size, not_empty_flag); /* If the tree is small, smaller than 10 * BTR_KEY_VAL_ESTIMATE_N_PAGES + total_external_size, then @@ -2997,12 +3079,20 @@ btr_estimate_number_of_different_key_vals( } index->stat_n_diff_key_vals[j] += add_on; + + /* Update the stat_n_non_null_key_vals[] with our + sampled result. stat_n_non_null_key_vals[] is created + and initialized to zero in dict_index_add_to_cache(), + along with stat_n_diff_key_vals[] array */ + if (n_not_null != NULL && (j < n_cols)) { + index->stat_n_non_null_key_vals[j] = + BTR_TABLE_STATS_FROM_SAMPLE( + n_not_null[j], index, + total_external_size, not_empty_flag); + } } - mem_free(n_diff); - if (UNIV_LIKELY_NULL(heap)) { - mem_heap_free(heap); - } + mem_heap_free(heap); } /*================== EXTERNAL STORAGE OF BIG FIELDS ===================*/ diff --git a/storage/innobase/dict/dict0dict.c b/storage/innobase/dict/dict0dict.c index fda6555e082..beea0a2f411 100644 --- a/storage/innobase/dict/dict0dict.c +++ b/storage/innobase/dict/dict0dict.c @@ -1358,6 +1358,12 @@ dict_index_add_to_cache( new_index->heap, (1 + dict_index_get_n_unique(new_index)) * sizeof(ib_longlong)); + + new_index->stat_n_non_null_key_vals = mem_heap_zalloc( + new_index->heap, + (1 + dict_index_get_n_unique(new_index)) + * sizeof(*new_index->stat_n_non_null_key_vals)); + /* Give some sensible values to stat_n_... in case we do not calculate statistics quickly enough */ @@ -3817,6 +3823,10 @@ dict_update_statistics_low( for (i = dict_index_get_n_unique(index); i; ) { index->stat_n_diff_key_vals[i--] = 1; } + + memset(index->stat_n_non_null_key_vals, 0, + (1 + dict_index_get_n_unique(index)) + * sizeof(*index->stat_n_non_null_key_vals)); } index = dict_table_get_next_index(index); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 4c52326a58a..6f58fd70fbd 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -130,6 +130,25 @@ static my_bool innobase_adaptive_hash_index = TRUE; static char* internal_innobase_data_file_path = NULL; +/* Possible values for system variable "innodb_stats_method". The values +are defined the same as its corresponding MyISAM system variable +"myisam_stats_method"(see "myisam_stats_method_names"), for better usability */ +static const char* innodb_stats_method_names[] = { + "nulls_equal", + "nulls_unequal", + "nulls_ignored", + NullS +}; + +/* Used to define an enumerate type of the system variable innodb_stats_method. +This is the same as "myisam_stats_method_typelib" */ +static TYPELIB innodb_stats_method_typelib = { + array_elements(innodb_stats_method_names) - 1, + "innodb_stats_method_typelib", + innodb_stats_method_names, + NULL +}; + /* The following counter is used to convey information to InnoDB about server activity: in selects it is not sensible to call srv_active_wake_master_thread after each fetch or search, we only do @@ -6362,6 +6381,65 @@ ha_innobase::read_time( return(ranges + (double) rows / (double) total_rows * time_for_scan); } +/************************************************************************* +Calculate Record Per Key value. Need to exclude the NULL value if +innodb_stats_method is set to "nulls_ignored" */ +static +ha_rows +innodb_rec_per_key( +/*===============*/ + /* out: estimated record per key + value */ + dict_index_t* index, /* in: dict_index_t structure */ + ulint i, /* in: the column we are + calculating rec per key */ + ha_rows records) /* in: estimated total records */ +{ + ha_rows rec_per_key; + + ut_ad(i < dict_index_get_n_unique(index)); + + /* Note the stat_n_diff_key_vals[] stores the diff value with + n-prefix indexing, so it is always stat_n_diff_key_vals[i + 1] */ + if (index->stat_n_diff_key_vals[i + 1] == 0) { + + rec_per_key = records; + } else if (srv_innodb_stats_method == SRV_STATS_NULLS_IGNORED) { + ib_longlong num_null; + + /* Number of rows with NULL value in this + field */ + num_null = records - index->stat_n_non_null_key_vals[i]; + + /* In theory, index->stat_n_non_null_key_vals[i] + should always be less than the number of records. + Since this is statistics value, the value could + have slight discrepancy. But we will make sure + the number of null values is not a negative number. */ + num_null = (num_null < 0) ? 0 : num_null; + + /* If the number of NULL values is the same as or + large than that of the distinct values, we could + consider that the table consists mostly of NULL value. + Set rec_per_key to 1. */ + if (index->stat_n_diff_key_vals[i + 1] <= num_null) { + rec_per_key = 1; + } else { + /* Need to exclude rows with NULL values from + rec_per_key calculation */ + rec_per_key = (ha_rows)( + (records - num_null) + / (index->stat_n_diff_key_vals[i + 1] + - num_null)); + } + } else { + rec_per_key = (ha_rows) + (records / index->stat_n_diff_key_vals[i + 1]); + } + + return(rec_per_key); +} + /************************************************************************* Returns statistics information of the table to the MySQL interpreter, in various fields of the handle object. */ @@ -6568,13 +6646,8 @@ ha_innobase::info_low( break; } - if (index->stat_n_diff_key_vals[j + 1] == 0) { - - rec_per_key = stats.records; - } else { - rec_per_key = (ha_rows)(stats.records / - index->stat_n_diff_key_vals[j + 1]); - } + rec_per_key = innodb_rec_per_key( + index, j, stats.records); /* Since MySQL seems to favor table scans too much over index searches, we pretend @@ -8990,6 +9063,13 @@ static MYSQL_SYSVAR_LONG(autoinc_lock_mode, innobase_autoinc_lock_mode, AUTOINC_OLD_STYLE_LOCKING, /* Minimum value */ AUTOINC_NO_LOCKING, 0); /* Maximum value */ +static MYSQL_SYSVAR_ENUM(stats_method, srv_innodb_stats_method, + PLUGIN_VAR_RQCMDARG, + "Specifies how InnoDB index statistics collection code should " + "treat NULLs. Possible values are NULLS_EQUAL (default), " + "NULLS_UNEQUAL and NULLS_IGNORED", + NULL, NULL, SRV_STATS_NULLS_EQUAL, &innodb_stats_method_typelib); + #if defined UNIV_DEBUG || defined UNIV_IBUF_DEBUG static MYSQL_SYSVAR_UINT(change_buffering_debug, ibuf_debug, PLUGIN_VAR_RQCMDARG, @@ -9031,6 +9111,7 @@ static struct st_mysql_sys_var* innobase_system_variables[]= { MYSQL_SYSVAR(stats_on_metadata), MYSQL_SYSVAR(use_legacy_cardinality_algorithm), MYSQL_SYSVAR(adaptive_hash_index), + MYSQL_SYSVAR(stats_method), MYSQL_SYSVAR(status_file), MYSQL_SYSVAR(support_xa), MYSQL_SYSVAR(sync_spin_loops), diff --git a/storage/innobase/include/btr0cur.h b/storage/innobase/include/btr0cur.h index 213dcb7f568..20235c55f22 100644 --- a/storage/innobase/include/btr0cur.h +++ b/storage/innobase/include/btr0cur.h @@ -404,7 +404,10 @@ btr_estimate_n_rows_in_range( /*********************************************************************** Estimates the number of different key values in a given index, for each n-column prefix of the index where n <= dict_index_get_n_unique(index). -The estimates are stored in the array index->stat_n_diff_key_vals. */ +The estimates are stored in the array index->stat_n_diff_key_vals. +If innodb_stats_method is nulls_ignored, we also record the number of +non-null values for each prefix and stored the estimates in +array index->stat_n_non_null_key_vals. */ void btr_estimate_number_of_different_key_vals( diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index 2f2a7441478..83dbf65ea41 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -222,6 +222,12 @@ struct dict_index_struct{ for this index, for each n-column prefix where n <= dict_get_n_unique(index); we periodically calculate new estimates */ + ib_longlong* stat_n_non_null_key_vals; + /* approximate number of non-null key values + for this index, for each column where + n < dict_get_n_unique(index); This + is used when innodb_stats_method is + "nulls_ignored". */ ulint stat_index_size; /* approximate index size in database pages */ ulint stat_n_leaf_pages; diff --git a/storage/innobase/include/rem0cmp.h b/storage/innobase/include/rem0cmp.h index c6a6e5de4db..22a22d13e17 100644 --- a/storage/innobase/include/rem0cmp.h +++ b/storage/innobase/include/rem0cmp.h @@ -141,6 +141,10 @@ cmp_rec_rec_with_match( const ulint* offsets1,/* in: rec_get_offsets(rec1, index) */ const ulint* offsets2,/* in: rec_get_offsets(rec2, index) */ dict_index_t* index, /* in: data dictionary index */ + ibool nulls_unequal, + /* in: TRUE if this is for index statistics + cardinality estimation, and innodb_stats_method + is "nulls_unequal" or "nulls_ignored" */ ulint* matched_fields, /* in/out: number of already completely matched fields; when the function returns, contains the value the for current diff --git a/storage/innobase/include/rem0cmp.ic b/storage/innobase/include/rem0cmp.ic index 52dc7ff5dc9..45e12301a3c 100644 --- a/storage/innobase/include/rem0cmp.ic +++ b/storage/innobase/include/rem0cmp.ic @@ -72,5 +72,5 @@ cmp_rec_rec( ulint match_b = 0; return(cmp_rec_rec_with_match(rec1, rec2, offsets1, offsets2, index, - &match_f, &match_b)); + FALSE, &match_f, &match_b)); } diff --git a/storage/innobase/include/srv0srv.h b/storage/innobase/include/srv0srv.h index 3dd4bb961f9..811074b2be8 100644 --- a/storage/innobase/include/srv0srv.h +++ b/storage/innobase/include/srv0srv.h @@ -91,6 +91,11 @@ extern ulint srv_lock_table_size; extern ulint srv_n_file_io_threads; +/* The "innodb_stats_method" setting, decides how InnoDB is going +to treat NULL value when collecting statistics. It is not defined +as enum type because the configure option takes unsigned integer type. */ +extern ulong srv_innodb_stats_method; + #ifdef UNIV_LOG_ARCHIVE extern ibool srv_log_archive_on; extern ibool srv_archive_recovery; @@ -286,6 +291,19 @@ of lower numbers are included. */ #define SRV_FORCE_NO_LOG_REDO 6 /* do not do the log roll-forward in connection with recovery */ +/* Alternatives for srv_innodb_stats_method, which could be changed by +setting innodb_stats_method */ +enum srv_stats_method_name_enum { + SRV_STATS_NULLS_EQUAL, /* All NULL values are treated as + equal. This is the default setting + for innodb_stats_method */ + SRV_STATS_NULLS_UNEQUAL, /* All NULL values are treated as + NOT equal. */ + SRV_STATS_NULLS_IGNORED /* NULL values are ignored */ +}; + +typedef enum srv_stats_method_name_enum srv_stats_method_name_t; + /************************************************************************* Boots Innobase server. */ diff --git a/storage/innobase/rem/rem0cmp.c b/storage/innobase/rem/rem0cmp.c index ca0ec663548..2939c119e2e 100644 --- a/storage/innobase/rem/rem0cmp.c +++ b/storage/innobase/rem/rem0cmp.c @@ -720,6 +720,10 @@ cmp_rec_rec_with_match( const ulint* offsets1,/* in: rec_get_offsets(rec1, index) */ const ulint* offsets2,/* in: rec_get_offsets(rec2, index) */ dict_index_t* index, /* in: data dictionary index */ + ibool nulls_unequal, + /* in: TRUE if this is for index statistics + cardinality estimation, and innodb_stats_method + is "nulls_unequal" or "nulls_ignored" */ ulint* matched_fields, /* in/out: number of already completely matched fields; when the function returns, contains the value the for current @@ -821,9 +825,13 @@ cmp_rec_rec_with_match( || rec2_f_len == UNIV_SQL_NULL) { if (rec1_f_len == rec2_f_len) { - - goto next_field; - + /* This is limited to stats collection, + cannot use it for regular search */ + if (nulls_unequal) { + ret = -1; + } else { + goto next_field; + } } else if (rec2_f_len == UNIV_SQL_NULL) { /* We define the SQL null to be the diff --git a/storage/innobase/srv/srv0srv.c b/storage/innobase/srv/srv0srv.c index 5b1184fb416..9c34e73109c 100644 --- a/storage/innobase/srv/srv0srv.c +++ b/storage/innobase/srv/srv0srv.c @@ -218,6 +218,11 @@ ulong srv_max_buf_pool_modified_pct = 90; /* variable counts amount of data read in total (in bytes) */ ulint srv_data_read = 0; +/* Internal setting for "innodb_stats_method". Decides how InnoDB treats +NULL value when collecting statistics. By default, it is set to +SRV_STATS_NULLS_EQUAL(0), ie. all NULL value are treated equal */ +ulong srv_innodb_stats_method = SRV_STATS_NULLS_EQUAL; + /* here we count the amount of data written in total (in bytes) */ ulint srv_data_written = 0; diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 8eb63fe8c78..43ffa762ddb 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,11 @@ +2011-01-14 The InnoDB Team + * btr/btr0cur.c, dict/dict0dict.c, handler/ha_innodb.cc, + include/btr0cur.h, include/dict0mem.h, include/rem0cmp.h, + include/rem0cmp.ic, include/srv0srv.h, rem/rem0cmp.c, + srv/srv0srv.c, innodb_bug30423.test: + Fix Bug#30423 InnoDBs treatment of NULL in index stats causes + bad "rows examined" estimates + 2011-01-06 The InnoDB Team * handler/i_s.cc, include/trx0i_s.h, trx/trx0i_s.c: Fix Bug#55397 cannot select from innodb_trx when trx_query contains diff --git a/storage/innodb_plugin/btr/btr0cur.c b/storage/innodb_plugin/btr/btr0cur.c index c57255a25ae..1fb0bc39933 100644 --- a/storage/innodb_plugin/btr/btr0cur.c +++ b/storage/innodb_plugin/btr/btr0cur.c @@ -100,6 +100,18 @@ can be released by page reorganize, then it is reorganized */ /*--------------------------------------*/ #define BTR_BLOB_HDR_SIZE 8 /*!< Size of a BLOB part header, in bytes */ + +/** Estimated table level stats from sampled value. +@param value sampled stats +@param index index being sampled +@param sample number of sampled rows +@param ext_size external stored data size +@param not_empty table not empty +@return estimated table wide stats from sampled value */ +#define BTR_TABLE_STATS_FROM_SAMPLE(value, index, sample, ext_size, not_empty)\ + (((value) * (ib_int64_t) index->stat_n_leaf_pages \ + + (sample) - 1 + (ext_size) + (not_empty)) / ((sample) + (ext_size))) + /* @} */ #endif /* !UNIV_HOTBACKUP */ @@ -3200,10 +3212,55 @@ btr_estimate_n_rows_in_range( } } +/*******************************************************************//** +Record the number of non_null key values in a given index for +each n-column prefix of the index where n < dict_index_get_n_unique(index). +The estimates are eventually stored in the array: +index->stat_n_non_null_key_vals. */ +static +void +btr_record_not_null_field_in_rec( +/*=============================*/ + rec_t* rec, /*!< in: physical record */ + ulint n_unique, /*!< in: dict_index_get_n_unique(index), + number of columns uniquely determine + an index entry */ + const ulint* offsets, /*!< in: rec_get_offsets(rec, index), + its size could be for all fields or + that of "n_unique" */ + ib_int64_t* n_not_null) /*!< in/out: array to record number of + not null rows for n-column prefix */ +{ + ulint i; + + ut_ad(rec_offs_n_fields(offsets) >= n_unique); + + if (n_not_null == NULL) { + return; + } + + for (i = 0; i < n_unique; i++) { + ulint rec_len; + byte* field; + + field = rec_get_nth_field(rec, offsets, i, &rec_len); + + if (rec_len != UNIV_SQL_NULL) { + n_not_null[i]++; + } else { + /* Break if we hit the first NULL value */ + break; + } + } +} + /*******************************************************************//** Estimates the number of different key values in a given index, for each n-column prefix of the index where n <= dict_index_get_n_unique(index). -The estimates are stored in the array index->stat_n_diff_key_vals. */ +The estimates are stored in the array index->stat_n_diff_key_vals. +If innodb_stats_method is "nulls_ignored", we also record the number of +non-null values for each prefix and store the estimates in +array index->stat_n_non_null_key_vals. */ UNIV_INTERN void btr_estimate_number_of_different_key_vals( @@ -3217,6 +3274,8 @@ btr_estimate_number_of_different_key_vals( ulint matched_fields; ulint matched_bytes; ib_int64_t* n_diff; + ib_int64_t* n_not_null; + ibool stats_null_not_equal; ullint n_sample_pages; /* number of pages to sample */ ulint not_empty_flag = 0; ulint total_external_size = 0; @@ -3225,16 +3284,43 @@ btr_estimate_number_of_different_key_vals( ullint add_on; mtr_t mtr; mem_heap_t* heap = NULL; - ulint offsets_rec_[REC_OFFS_NORMAL_SIZE]; - ulint offsets_next_rec_[REC_OFFS_NORMAL_SIZE]; - ulint* offsets_rec = offsets_rec_; - ulint* offsets_next_rec= offsets_next_rec_; - rec_offs_init(offsets_rec_); - rec_offs_init(offsets_next_rec_); + ulint* offsets_rec = NULL; + ulint* offsets_next_rec = NULL; n_cols = dict_index_get_n_unique(index); - n_diff = mem_zalloc((n_cols + 1) * sizeof(ib_int64_t)); + heap = mem_heap_create((sizeof *n_diff + sizeof *n_not_null) + * (n_cols + 1) + + dict_index_get_n_fields(index) + * (sizeof *offsets_rec + + sizeof *offsets_next_rec)); + + n_diff = mem_heap_zalloc(heap, (n_cols + 1) * sizeof(ib_int64_t)); + + n_not_null = NULL; + + /* Check srv_innodb_stats_method setting, and decide whether we + need to record non-null value and also decide if NULL is + considered equal (by setting stats_null_not_equal value) */ + switch (srv_innodb_stats_method) { + case SRV_STATS_NULLS_IGNORED: + n_not_null = mem_heap_zalloc(heap, (n_cols + 1) + * sizeof *n_not_null); + /* fall through */ + + case SRV_STATS_NULLS_UNEQUAL: + /* for both SRV_STATS_NULLS_IGNORED and SRV_STATS_NULLS_UNEQUAL + case, we will treat NULLs as unequal value */ + stats_null_not_equal = TRUE; + break; + + case SRV_STATS_NULLS_EQUAL: + stats_null_not_equal = FALSE; + break; + + default: + ut_error; + } /* It makes no sense to test more pages than are contained in the index, thus we lower the number if it is too high */ @@ -3251,7 +3337,6 @@ btr_estimate_number_of_different_key_vals( /* We sample some pages in the index to get an estimate */ for (i = 0; i < n_sample_pages; i++) { - rec_t* supremum; mtr_start(&mtr); btr_cur_open_at_rnd_pos(index, BTR_SEARCH_LEAF, &cursor, &mtr); @@ -3264,18 +3349,22 @@ btr_estimate_number_of_different_key_vals( page = btr_cur_get_page(&cursor); - supremum = page_get_supremum_rec(page); rec = page_rec_get_next(page_get_infimum_rec(page)); - if (rec != supremum) { + if (!page_rec_is_supremum(rec)) { not_empty_flag = 1; offsets_rec = rec_get_offsets(rec, index, offsets_rec, ULINT_UNDEFINED, &heap); + + if (n_not_null) { + btr_record_not_null_field_in_rec( + rec, n_cols, offsets_rec, n_not_null); + } } - while (rec != supremum) { + while (!page_rec_is_supremum(rec)) { rec_t* next_rec = page_rec_get_next(rec); - if (next_rec == supremum) { + if (page_rec_is_supremum(next_rec)) { break; } @@ -3287,7 +3376,8 @@ btr_estimate_number_of_different_key_vals( cmp_rec_rec_with_match(rec, next_rec, offsets_rec, offsets_next_rec, - index, &matched_fields, + index, stats_null_not_equal, + &matched_fields, &matched_bytes); for (j = matched_fields + 1; j <= n_cols; j++) { @@ -3297,6 +3387,12 @@ btr_estimate_number_of_different_key_vals( n_diff[j]++; } + if (n_not_null) { + btr_record_not_null_field_in_rec( + next_rec, n_cols, offsets_next_rec, + n_not_null); + } + total_external_size += btr_rec_get_externally_stored_len( rec, offsets_rec); @@ -3348,13 +3444,9 @@ btr_estimate_number_of_different_key_vals( for (j = 0; j <= n_cols; j++) { index->stat_n_diff_key_vals[j] - = ((n_diff[j] - * (ib_int64_t)index->stat_n_leaf_pages - + n_sample_pages - 1 - + total_external_size - + not_empty_flag) - / (n_sample_pages - + total_external_size)); + = BTR_TABLE_STATS_FROM_SAMPLE( + n_diff[j], index, n_sample_pages, + total_external_size, not_empty_flag); /* If the tree is small, smaller than 10 * n_sample_pages + total_external_size, then @@ -3373,12 +3465,20 @@ btr_estimate_number_of_different_key_vals( } index->stat_n_diff_key_vals[j] += add_on; + + /* Update the stat_n_non_null_key_vals[] with our + sampled result. stat_n_non_null_key_vals[] is created + and initialized to zero in dict_index_add_to_cache(), + along with stat_n_diff_key_vals[] array */ + if (n_not_null != NULL && (j < n_cols)) { + index->stat_n_non_null_key_vals[j] = + BTR_TABLE_STATS_FROM_SAMPLE( + n_not_null[j], index, n_sample_pages, + total_external_size, not_empty_flag); + } } - mem_free(n_diff); - if (UNIV_LIKELY_NULL(heap)) { - mem_heap_free(heap); - } + mem_heap_free(heap); } /*================== EXTERNAL STORAGE OF BIG FIELDS ===================*/ diff --git a/storage/innodb_plugin/dict/dict0dict.c b/storage/innodb_plugin/dict/dict0dict.c index 67765555658..ff56e9cb76a 100644 --- a/storage/innodb_plugin/dict/dict0dict.c +++ b/storage/innodb_plugin/dict/dict0dict.c @@ -1669,6 +1669,12 @@ undo_size_ok: new_index->heap, (1 + dict_index_get_n_unique(new_index)) * sizeof(ib_int64_t)); + + new_index->stat_n_non_null_key_vals = mem_heap_zalloc( + new_index->heap, + (1 + dict_index_get_n_unique(new_index)) + * sizeof(*new_index->stat_n_non_null_key_vals)); + /* Give some sensible values to stat_n_... in case we do not calculate statistics quickly enough */ @@ -4291,6 +4297,10 @@ dict_update_statistics( for (i = dict_index_get_n_unique(index); i; ) { index->stat_n_diff_key_vals[i--] = 1; } + + memset(index->stat_n_non_null_key_vals, 0, + (1 + dict_index_get_n_unique(index)) + * sizeof(*index->stat_n_non_null_key_vals)); } index = dict_table_get_next_index(index); diff --git a/storage/innodb_plugin/handler/ha_innodb.cc b/storage/innodb_plugin/handler/ha_innodb.cc index 86168e2bc9b..2d60c7397b0 100644 --- a/storage/innodb_plugin/handler/ha_innodb.cc +++ b/storage/innodb_plugin/handler/ha_innodb.cc @@ -174,6 +174,25 @@ static char* internal_innobase_data_file_path = NULL; static char* innodb_version_str = (char*) INNODB_VERSION_STR; +/** Possible values for system variable "innodb_stats_method". The values +are defined the same as its corresponding MyISAM system variable +"myisam_stats_method"(see "myisam_stats_method_names"), for better usability */ +static const char* innodb_stats_method_names[] = { + "nulls_equal", + "nulls_unequal", + "nulls_ignored", + NullS +}; + +/** Used to define an enumerate type of the system variable innodb_stats_method. +This is the same as "myisam_stats_method_typelib" */ +static TYPELIB innodb_stats_method_typelib = { + array_elements(innodb_stats_method_names) - 1, + "innodb_stats_method_typelib", + innodb_stats_method_names, + NULL +}; + /* The following counter is used to convey information to InnoDB about server activity: in selects it is not sensible to call srv_active_wake_master_thread after each fetch or search, we only do @@ -7507,6 +7526,65 @@ innobase_get_mysql_key_number_for_index( return(0); } + +/*********************************************************************//** +Calculate Record Per Key value. Need to exclude the NULL value if +innodb_stats_method is set to "nulls_ignored" +@return estimated record per key value */ +static +ha_rows +innodb_rec_per_key( +/*===============*/ + dict_index_t* index, /*!< in: dict_index_t structure */ + ulint i, /*!< in: the column we are + calculating rec per key */ + ha_rows records) /*!< in: estimated total records */ +{ + ha_rows rec_per_key; + + ut_ad(i < dict_index_get_n_unique(index)); + + /* Note the stat_n_diff_key_vals[] stores the diff value with + n-prefix indexing, so it is always stat_n_diff_key_vals[i + 1] */ + if (index->stat_n_diff_key_vals[i + 1] == 0) { + + rec_per_key = records; + } else if (srv_innodb_stats_method == SRV_STATS_NULLS_IGNORED) { + ib_int64_t num_null; + + /* Number of rows with NULL value in this + field */ + num_null = records - index->stat_n_non_null_key_vals[i]; + + /* In theory, index->stat_n_non_null_key_vals[i] + should always be less than the number of records. + Since this is statistics value, the value could + have slight discrepancy. But we will make sure + the number of null values is not a negative number. */ + num_null = (num_null < 0) ? 0 : num_null; + + /* If the number of NULL values is the same as or + large than that of the distinct values, we could + consider that the table consists mostly of NULL value. + Set rec_per_key to 1. */ + if (index->stat_n_diff_key_vals[i + 1] <= num_null) { + rec_per_key = 1; + } else { + /* Need to exclude rows with NULL values from + rec_per_key calculation */ + rec_per_key = (ha_rows)( + (records - num_null) + / (index->stat_n_diff_key_vals[i + 1] + - num_null)); + } + } else { + rec_per_key = (ha_rows) + (records / index->stat_n_diff_key_vals[i + 1]); + } + + return(rec_per_key); +} + /*********************************************************************//** Returns statistics information of the table to the MySQL interpreter, in various fields of the handle object. */ @@ -7737,13 +7815,8 @@ ha_innobase::info_low( break; } - if (index->stat_n_diff_key_vals[j + 1] == 0) { - - rec_per_key = stats.records; - } else { - rec_per_key = (ha_rows)(stats.records / - index->stat_n_diff_key_vals[j + 1]); - } + rec_per_key = innodb_rec_per_key( + index, j, stats.records); /* Since MySQL seems to favor table scans too much over index searches, we pretend @@ -10934,6 +11007,13 @@ static MYSQL_SYSVAR_STR(change_buffering, innobase_change_buffering, innodb_change_buffering_validate, innodb_change_buffering_update, "inserts"); +static MYSQL_SYSVAR_ENUM(stats_method, srv_innodb_stats_method, + PLUGIN_VAR_RQCMDARG, + "Specifies how InnoDB index statistics collection code should " + "treat NULLs. Possible values are NULLS_EQUAL (default), " + "NULLS_UNEQUAL and NULLS_IGNORED", + NULL, NULL, SRV_STATS_NULLS_EQUAL, &innodb_stats_method_typelib); + #if defined UNIV_DEBUG || defined UNIV_IBUF_DEBUG static MYSQL_SYSVAR_UINT(change_buffering_debug, ibuf_debug, PLUGIN_VAR_RQCMDARG, @@ -10988,6 +11068,7 @@ static struct st_mysql_sys_var* innobase_system_variables[]= { MYSQL_SYSVAR(stats_on_metadata), MYSQL_SYSVAR(stats_sample_pages), MYSQL_SYSVAR(adaptive_hash_index), + MYSQL_SYSVAR(stats_method), MYSQL_SYSVAR(replication_delay), MYSQL_SYSVAR(status_file), MYSQL_SYSVAR(strict_mode), diff --git a/storage/innodb_plugin/include/btr0cur.h b/storage/innodb_plugin/include/btr0cur.h index b477ad0320a..cb8cb399715 100644 --- a/storage/innodb_plugin/include/btr0cur.h +++ b/storage/innodb_plugin/include/btr0cur.h @@ -478,7 +478,10 @@ btr_estimate_n_rows_in_range( /*******************************************************************//** Estimates the number of different key values in a given index, for each n-column prefix of the index where n <= dict_index_get_n_unique(index). -The estimates are stored in the array index->stat_n_diff_key_vals. */ +The estimates are stored in the array index->stat_n_diff_key_vals. +If innodb_stats_method is nulls_ignored, we also record the number of +non-null values for each prefix and stored the estimates in +array index->stat_n_non_null_key_vals. */ UNIV_INTERN void btr_estimate_number_of_different_key_vals( diff --git a/storage/innodb_plugin/include/dict0mem.h b/storage/innodb_plugin/include/dict0mem.h index 19782c2e76a..09a068ccb93 100644 --- a/storage/innodb_plugin/include/dict0mem.h +++ b/storage/innodb_plugin/include/dict0mem.h @@ -321,6 +321,12 @@ struct dict_index_struct{ dict_get_n_unique(index); we periodically calculate new estimates */ + ib_int64_t* stat_n_non_null_key_vals; + /* approximate number of non-null key values + for this index, for each column where + n < dict_get_n_unique(index); This + is used when innodb_stats_method is + "nulls_ignored". */ ulint stat_index_size; /*!< approximate index size in database pages */ diff --git a/storage/innodb_plugin/include/rem0cmp.h b/storage/innodb_plugin/include/rem0cmp.h index 2f751a38864..a908521c9f7 100644 --- a/storage/innodb_plugin/include/rem0cmp.h +++ b/storage/innodb_plugin/include/rem0cmp.h @@ -165,6 +165,10 @@ cmp_rec_rec_with_match( const ulint* offsets1,/*!< in: rec_get_offsets(rec1, index) */ const ulint* offsets2,/*!< in: rec_get_offsets(rec2, index) */ dict_index_t* index, /*!< in: data dictionary index */ + ibool nulls_unequal, + /* in: TRUE if this is for index statistics + cardinality estimation, and innodb_stats_method + is "nulls_unequal" or "nulls_ignored" */ ulint* matched_fields, /*!< in/out: number of already completely matched fields; when the function returns, contains the value the for current diff --git a/storage/innodb_plugin/include/rem0cmp.ic b/storage/innodb_plugin/include/rem0cmp.ic index 39ef5f4fba3..63415fe7837 100644 --- a/storage/innodb_plugin/include/rem0cmp.ic +++ b/storage/innodb_plugin/include/rem0cmp.ic @@ -87,5 +87,5 @@ cmp_rec_rec( ulint match_b = 0; return(cmp_rec_rec_with_match(rec1, rec2, offsets1, offsets2, index, - &match_f, &match_b)); + FALSE, &match_f, &match_b)); } diff --git a/storage/innodb_plugin/include/srv0srv.h b/storage/innodb_plugin/include/srv0srv.h index 7aa2ce74720..91ae895040c 100644 --- a/storage/innodb_plugin/include/srv0srv.h +++ b/storage/innodb_plugin/include/srv0srv.h @@ -154,6 +154,11 @@ capacity. PCT_IO(5) -> returns the number of IO operations that is 5% of the max where max is srv_io_capacity. */ #define PCT_IO(p) ((ulong) (srv_io_capacity * ((double) p / 100.0))) +/* The "innodb_stats_method" setting, decides how InnoDB is going +to treat NULL value when collecting statistics. It is not defined +as enum type because the configure option takes unsigned integer type. */ +extern ulong srv_innodb_stats_method; + #ifdef UNIV_LOG_ARCHIVE extern ibool srv_log_archive_on; extern ibool srv_archive_recovery; @@ -363,6 +368,19 @@ enum { in connection with recovery */ }; +/* Alternatives for srv_innodb_stats_method, which could be changed by +setting innodb_stats_method */ +enum srv_stats_method_name_enum { + SRV_STATS_NULLS_EQUAL, /* All NULL values are treated as + equal. This is the default setting + for innodb_stats_method */ + SRV_STATS_NULLS_UNEQUAL, /* All NULL values are treated as + NOT equal. */ + SRV_STATS_NULLS_IGNORED /* NULL values are ignored */ +}; + +typedef enum srv_stats_method_name_enum srv_stats_method_name_t; + #ifndef UNIV_HOTBACKUP /** Types of threads existing in the system. */ enum srv_thread_type { diff --git a/storage/innodb_plugin/rem/rem0cmp.c b/storage/innodb_plugin/rem/rem0cmp.c index 35b67992558..04d2c15437b 100644 --- a/storage/innodb_plugin/rem/rem0cmp.c +++ b/storage/innodb_plugin/rem/rem0cmp.c @@ -862,6 +862,10 @@ cmp_rec_rec_with_match( const ulint* offsets1,/*!< in: rec_get_offsets(rec1, index) */ const ulint* offsets2,/*!< in: rec_get_offsets(rec2, index) */ dict_index_t* index, /*!< in: data dictionary index */ + ibool nulls_unequal, + /* in: TRUE if this is for index statistics + cardinality estimation, and innodb_stats_method + is "nulls_unequal" or "nulls_ignored" */ ulint* matched_fields, /*!< in/out: number of already completely matched fields; when the function returns, contains the value the for current @@ -961,9 +965,13 @@ cmp_rec_rec_with_match( || rec2_f_len == UNIV_SQL_NULL) { if (rec1_f_len == rec2_f_len) { - - goto next_field; - + /* This is limited to stats collection, + cannot use it for regular search */ + if (nulls_unequal) { + ret = -1; + } else { + goto next_field; + } } else if (rec2_f_len == UNIV_SQL_NULL) { /* We define the SQL null to be the diff --git a/storage/innodb_plugin/srv/srv0srv.c b/storage/innodb_plugin/srv/srv0srv.c index f7e7e351bdc..3cf17f33c40 100644 --- a/storage/innodb_plugin/srv/srv0srv.c +++ b/storage/innodb_plugin/srv/srv0srv.c @@ -243,6 +243,11 @@ UNIV_INTERN ulong srv_max_buf_pool_modified_pct = 75; /* variable counts amount of data read in total (in bytes) */ UNIV_INTERN ulint srv_data_read = 0; +/* Internal setting for "innodb_stats_method". Decides how InnoDB treats +NULL value when collecting statistics. By default, it is set to +SRV_STATS_NULLS_EQUAL(0), ie. all NULL value are treated equal */ +ulong srv_innodb_stats_method = SRV_STATS_NULLS_EQUAL; + /* here we count the amount of data written in total (in bytes) */ UNIV_INTERN ulint srv_data_written = 0; From 1f3975b4f8b22eef97b2d86b8ecbc17c90c5f1ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 17 Jan 2011 14:06:48 +0200 Subject: [PATCH 04/20] Non-functional changes. Remove the unused data type dict_cluster_t. Remove a bogus comment about latching order. --- storage/innobase/include/dict0types.h | 5 ----- storage/innobase/include/trx0rseg.h | 4 +--- storage/innodb_plugin/include/dict0types.h | 5 ----- storage/innodb_plugin/include/trx0rseg.h | 4 +--- 4 files changed, 2 insertions(+), 16 deletions(-) diff --git a/storage/innobase/include/dict0types.h b/storage/innobase/include/dict0types.h index b90545f2105..6674b5ff397 100644 --- a/storage/innobase/include/dict0types.h +++ b/storage/innobase/include/dict0types.h @@ -16,11 +16,6 @@ typedef struct dict_index_struct dict_index_t; typedef struct dict_table_struct dict_table_t; typedef struct dict_foreign_struct dict_foreign_t; -/* A cluster object is a table object with the type field set to -DICT_CLUSTERED */ - -typedef dict_table_t dict_cluster_t; - typedef struct ind_node_struct ind_node_t; typedef struct tab_node_struct tab_node_t; diff --git a/storage/innobase/include/trx0rseg.h b/storage/innobase/include/trx0rseg.h index 46ba010bd1d..22f8aa89181 100644 --- a/storage/innobase/include/trx0rseg.h +++ b/storage/innobase/include/trx0rseg.h @@ -121,9 +121,7 @@ struct trx_rseg_struct{ ulint id; /* rollback segment id == the index of its slot in the trx system file copy */ mutex_t mutex; /* mutex protecting the fields in this - struct except id; NOTE that the latching - order must always be kernel mutex -> - rseg mutex */ + struct except id, which is constant */ ulint space; /* space where the rollback segment is header is placed */ ulint page_no;/* page number of the rollback segment diff --git a/storage/innodb_plugin/include/dict0types.h b/storage/innodb_plugin/include/dict0types.h index 7ad69193cc9..f14b59a19d4 100644 --- a/storage/innodb_plugin/include/dict0types.h +++ b/storage/innodb_plugin/include/dict0types.h @@ -33,11 +33,6 @@ typedef struct dict_index_struct dict_index_t; typedef struct dict_table_struct dict_table_t; typedef struct dict_foreign_struct dict_foreign_t; -/* A cluster object is a table object with the type field set to -DICT_CLUSTERED */ - -typedef dict_table_t dict_cluster_t; - typedef struct ind_node_struct ind_node_t; typedef struct tab_node_struct tab_node_t; diff --git a/storage/innodb_plugin/include/trx0rseg.h b/storage/innodb_plugin/include/trx0rseg.h index a25d84f1e84..e3674089735 100644 --- a/storage/innodb_plugin/include/trx0rseg.h +++ b/storage/innodb_plugin/include/trx0rseg.h @@ -135,9 +135,7 @@ struct trx_rseg_struct{ ulint id; /*!< rollback segment id == the index of its slot in the trx system file copy */ mutex_t mutex; /*!< mutex protecting the fields in this - struct except id; NOTE that the latching - order must always be kernel mutex -> - rseg mutex */ + struct except id, which is constant */ ulint space; /*!< space where the rollback segment is header is placed */ ulint zip_size;/* compressed page size of space From 359bddbee1a27864a38195e85fceab8a1678081d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 18 Jan 2011 12:25:13 +0200 Subject: [PATCH 05/20] Bug#59579 rw_lock_debug_print outputs to stderr rw_lock_debug_print(): Add parameter FILE* for specifying the output stream. rw_lock_list_print_info(): Invoke rw_lock_debug_print() on file, not stderr. --- storage/innobase/include/sync0rw.h | 3 ++- storage/innobase/sync/sync0arr.c | 4 ++-- storage/innobase/sync/sync0rw.c | 19 ++++++++++--------- storage/innodb_plugin/ChangeLog | 6 ++++++ storage/innodb_plugin/include/sync0rw.h | 3 ++- storage/innodb_plugin/sync/sync0arr.c | 4 ++-- storage/innodb_plugin/sync/sync0rw.c | 19 ++++++++++--------- 7 files changed, 34 insertions(+), 24 deletions(-) diff --git a/storage/innobase/include/sync0rw.h b/storage/innobase/include/sync0rw.h index 008df80a2c7..dd898557d6e 100644 --- a/storage/innobase/include/sync0rw.h +++ b/storage/innobase/include/sync0rw.h @@ -1,7 +1,7 @@ /****************************************************** The read-write lock (for threads, not for database transactions) -(c) 1995 Innobase Oy +Copyright (c) 1995, 2011, Oracle and/or its affiliates. All Rights Reserved. Created 9/11/1995 Heikki Tuuri *******************************************************/ @@ -409,6 +409,7 @@ Prints info of a debug struct. */ void rw_lock_debug_print( /*================*/ + FILE* f, /* in: output stream */ rw_lock_debug_t* info); /* in: debug struct */ #endif /* UNIV_SYNC_DEBUG */ diff --git a/storage/innobase/sync/sync0arr.c b/storage/innobase/sync/sync0arr.c index 154593a9035..41d3492c8c9 100644 --- a/storage/innobase/sync/sync0arr.c +++ b/storage/innobase/sync/sync0arr.c @@ -1,7 +1,7 @@ /****************************************************** The wait array used in synchronization primitives -(c) 1995 Innobase Oy +Copyright (c) 1995, 2011, Oracle and/or its affiliates. All Rights Reserved. Created 9/5/1995 Heikki Tuuri *******************************************************/ @@ -709,7 +709,7 @@ print: fprintf(stderr, "rw-lock %p ", (void*) lock); sync_array_cell_print(stderr, cell); - rw_lock_debug_print(debug); + rw_lock_debug_print(stderr, debug); return(TRUE); } } diff --git a/storage/innobase/sync/sync0rw.c b/storage/innobase/sync/sync0rw.c index 0b05fb826ac..ef4c07e8c26 100644 --- a/storage/innobase/sync/sync0rw.c +++ b/storage/innobase/sync/sync0rw.c @@ -1,7 +1,7 @@ /****************************************************** The read-write lock (for thread synchronization) -(c) 1995 Innobase Oy +Copyright (c) 1995, 2011, Oracle and/or its affiliates. All Rights Reserved. Created 9/11/1995 Heikki Tuuri *******************************************************/ @@ -830,7 +830,7 @@ rw_lock_list_print_info( info = UT_LIST_GET_FIRST(lock->debug_list); while (info != NULL) { - rw_lock_debug_print(info); + rw_lock_debug_print(file, info); info = UT_LIST_GET_NEXT(list, info); } } @@ -870,7 +870,7 @@ rw_lock_print( info = UT_LIST_GET_FIRST(lock->debug_list); while (info != NULL) { - rw_lock_debug_print(info); + rw_lock_debug_print(stderr, info); info = UT_LIST_GET_NEXT(list, info); } } @@ -882,28 +882,29 @@ Prints info of a debug struct. */ void rw_lock_debug_print( /*================*/ + FILE* f, /* in: output stream */ rw_lock_debug_t* info) /* in: debug struct */ { ulint rwt; rwt = info->lock_type; - fprintf(stderr, "Locked: thread %lu file %s line %lu ", + fprintf(f, "Locked: thread %lu file %s line %lu ", (ulong) os_thread_pf(info->thread_id), info->file_name, (ulong) info->line); if (rwt == RW_LOCK_SHARED) { - fputs("S-LOCK", stderr); + fputs("S-LOCK", f); } else if (rwt == RW_LOCK_EX) { - fputs("X-LOCK", stderr); + fputs("X-LOCK", f); } else if (rwt == RW_LOCK_WAIT_EX) { - fputs("WAIT X-LOCK", stderr); + fputs("WAIT X-LOCK", f); } else { ut_error; } if (info->pass != 0) { - fprintf(stderr, " pass value %lu", (ulong) info->pass); + fprintf(f, " pass value %lu", (ulong) info->pass); } - putc('\n', stderr); + putc('\n', f); } /******************************************************************* diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 43ffa762ddb..4d35bcff4a1 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,9 @@ +2011-01-18 The InnoDB Team + + * include/sync0rw.h, sync/sync0arr.c, sync/sync0rw.c: + Fix Bug#59579 rw_lock_debug_print outputs to stderr, not to + SHOW ENGINE INNODB STATUS + 2011-01-14 The InnoDB Team * btr/btr0cur.c, dict/dict0dict.c, handler/ha_innodb.cc, include/btr0cur.h, include/dict0mem.h, include/rem0cmp.h, diff --git a/storage/innodb_plugin/include/sync0rw.h b/storage/innodb_plugin/include/sync0rw.h index 175f3deb77c..47f7dbfe0eb 100644 --- a/storage/innodb_plugin/include/sync0rw.h +++ b/storage/innodb_plugin/include/sync0rw.h @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 1995, 2010, Innobase Oy. All Rights Reserved. +Copyright (c) 1995, 2011, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2008, Google Inc. Portions of this file contain modifications contributed and copyrighted by @@ -490,6 +490,7 @@ UNIV_INTERN void rw_lock_debug_print( /*================*/ + FILE* f, /*!< in: output stream */ rw_lock_debug_t* info); /*!< in: debug struct */ #endif /* UNIV_SYNC_DEBUG */ diff --git a/storage/innodb_plugin/sync/sync0arr.c b/storage/innodb_plugin/sync/sync0arr.c index 3c825e2202b..ad29b90d344 100644 --- a/storage/innodb_plugin/sync/sync0arr.c +++ b/storage/innodb_plugin/sync/sync0arr.c @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 1995, 2009, Innobase Oy. All Rights Reserved. +Copyright (c) 1995, 2011, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2008, Google Inc. Portions of this file contain modifications contributed and copyrighted by @@ -715,7 +715,7 @@ print: fprintf(stderr, "rw-lock %p ", (void*) lock); sync_array_cell_print(stderr, cell); - rw_lock_debug_print(debug); + rw_lock_debug_print(stderr, debug); return(TRUE); } } diff --git a/storage/innodb_plugin/sync/sync0rw.c b/storage/innodb_plugin/sync/sync0rw.c index 572c3690a7f..00e0324becd 100644 --- a/storage/innodb_plugin/sync/sync0rw.c +++ b/storage/innodb_plugin/sync/sync0rw.c @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 1995, 2009, Innobase Oy. All Rights Reserved. +Copyright (c) 1995, 2011, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2008, Google Inc. Portions of this file contain modifications contributed and copyrighted by @@ -925,7 +925,7 @@ rw_lock_list_print_info( info = UT_LIST_GET_FIRST(lock->debug_list); while (info != NULL) { - rw_lock_debug_print(info); + rw_lock_debug_print(file, info); info = UT_LIST_GET_NEXT(list, info); } } @@ -973,7 +973,7 @@ rw_lock_print( info = UT_LIST_GET_FIRST(lock->debug_list); while (info != NULL) { - rw_lock_debug_print(info); + rw_lock_debug_print(stderr, info); info = UT_LIST_GET_NEXT(list, info); } } @@ -985,28 +985,29 @@ UNIV_INTERN void rw_lock_debug_print( /*================*/ + FILE* f, /*!< in: output stream */ rw_lock_debug_t* info) /*!< in: debug struct */ { ulint rwt; rwt = info->lock_type; - fprintf(stderr, "Locked: thread %lu file %s line %lu ", + fprintf(f, "Locked: thread %lu file %s line %lu ", (ulong) os_thread_pf(info->thread_id), info->file_name, (ulong) info->line); if (rwt == RW_LOCK_SHARED) { - fputs("S-LOCK", stderr); + fputs("S-LOCK", f); } else if (rwt == RW_LOCK_EX) { - fputs("X-LOCK", stderr); + fputs("X-LOCK", f); } else if (rwt == RW_LOCK_WAIT_EX) { - fputs("WAIT X-LOCK", stderr); + fputs("WAIT X-LOCK", f); } else { ut_error; } if (info->pass != 0) { - fprintf(stderr, " pass value %lu", (ulong) info->pass); + fprintf(f, " pass value %lu", (ulong) info->pass); } - putc('\n', stderr); + putc('\n', f); } /***************************************************************//** From a9d18719f639c61632ccc1feb059e927ab8e6b10 Mon Sep 17 00:00:00 2001 From: Sandeep Doddaballapur Date: Tue, 25 Jan 2011 12:14:28 +0530 Subject: [PATCH 06/20] --- mysql-test/r/csv_not_null.result | 3 +++ mysql-test/t/csv_not_null.test | 6 ++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/csv_not_null.result b/mysql-test/r/csv_not_null.result index af583a36837..aed9bcb1587 100644 --- a/mysql-test/r/csv_not_null.result +++ b/mysql-test/r/csv_not_null.result @@ -19,13 +19,16 @@ INSERT INTO t1 VALUES(); SELECT * FROM t1; a b c d e f 0 foo 0000-00-00 +INSERT INTO t1 VALUES(default,default,default,default,default,default); SELECT * FROM t1; a b c d e f 0 foo 0000-00-00 +0 foo 0000-00-00 INSERT INTO t1 VALUES(0,'abc','def','ghi','bar','1999-12-31'); SELECT * FROM t1; a b c d e f 0 foo 0000-00-00 +0 foo 0000-00-00 0 abc def ghi bar 1999-12-31 # === insert failures === INSERT INTO t1 VALUES(NULL,'ab','a','b','foo','2007-01-01'); diff --git a/mysql-test/t/csv_not_null.test b/mysql-test/t/csv_not_null.test index 03ed566fb22..bebea53b2f7 100644 --- a/mysql-test/t/csv_not_null.test +++ b/mysql-test/t/csv_not_null.test @@ -55,10 +55,8 @@ INSERT INTO t1 VALUES(); SELECT * FROM t1; -- disable_warnings -# NOTE - Test disabled due to enum crash for this INSERT -# See Bug#33717 - INSERT...(default) fails for enum. -# Crashes CSV tables, loads spaces for MyISAM -#INSERT INTO t1 VALUES(default,default,default,default,default,default); +# Bug#33717 - INSERT...(default) fails for enum. +INSERT INTO t1 VALUES(default,default,default,default,default,default); -- enable_warnings SELECT * FROM t1; From 60a622d1c1940f80829a4df312ff49a6feae265e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 25 Jan 2011 09:56:18 +0200 Subject: [PATCH 07/20] Bug#59707 Unused compression-related parameters in buffer pool functions buf_block_alloc(): ulint zip_size is always 0. buf_LRU_get_free_block(): ulint zip_size is always 0. buf_LRU_free_block(): ibool* buf_pool_mutex_released is always NULL. Remove these parameters. buf_LRU_get_free_block(): Simplify the initialization of block->page.zip and release buf_pool_mutex() earlier. --- storage/innodb_plugin/ChangeLog | 9 +++++ storage/innodb_plugin/btr/btr0btr.c | 2 +- storage/innodb_plugin/btr/btr0cur.c | 5 ++- storage/innodb_plugin/btr/btr0sea.c | 2 +- storage/innodb_plugin/buf/buf0buddy.c | 2 +- storage/innodb_plugin/buf/buf0buf.c | 14 ++++---- storage/innodb_plugin/buf/buf0lru.c | 44 +++++------------------- storage/innodb_plugin/include/buf0buf.h | 6 ++-- storage/innodb_plugin/include/buf0buf.ic | 8 ++--- storage/innodb_plugin/include/buf0lru.h | 14 +++----- storage/innodb_plugin/mem/mem0mem.c | 2 +- storage/innodb_plugin/page/page0zip.c | 2 +- 12 files changed, 40 insertions(+), 70 deletions(-) diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 4d35bcff4a1..24cac7ac2be 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,12 @@ +2011-01-25 The InnoDB Team + + * btr/btr0btr.c, btr/btr0cur.c, btr/btr0sea.c, + buf/buf0buddy.c, buf/buf0buf.c, buf/buf0lru.c, + include/buf0buf.h, include/buf0buf.ic, include/buf0lru.h, + mem/mem0mem.c, page/page0zip.c: + Fix Bug#59707 Unused compression-related parameters + in buffer pool functions + 2011-01-18 The InnoDB Team * include/sync0rw.h, sync/sync0arr.c, sync/sync0rw.c: diff --git a/storage/innodb_plugin/btr/btr0btr.c b/storage/innodb_plugin/btr/btr0btr.c index 32e2caecdb8..3d8d6048603 100644 --- a/storage/innodb_plugin/btr/btr0btr.c +++ b/storage/innodb_plugin/btr/btr0btr.c @@ -979,7 +979,7 @@ btr_page_reorganize_low( log_mode = mtr_set_log_mode(mtr, MTR_LOG_NONE); #ifndef UNIV_HOTBACKUP - temp_block = buf_block_alloc(0); + temp_block = buf_block_alloc(); #else /* !UNIV_HOTBACKUP */ ut_ad(block == back_block1); temp_block = back_block2; diff --git a/storage/innodb_plugin/btr/btr0cur.c b/storage/innodb_plugin/btr/btr0cur.c index 1fb0bc39933..f41b125b281 100644 --- a/storage/innodb_plugin/btr/btr0cur.c +++ b/storage/innodb_plugin/btr/btr0cur.c @@ -3767,13 +3767,12 @@ btr_blob_free( && buf_block_get_space(block) == space && buf_block_get_page_no(block) == page_no) { - if (buf_LRU_free_block(&block->page, all, NULL) - != BUF_LRU_FREED + if (buf_LRU_free_block(&block->page, all) != BUF_LRU_FREED && all && block->page.zip.data) { /* Attempt to deallocate the uncompressed page if the whole block cannot be deallocted. */ - buf_LRU_free_block(&block->page, FALSE, NULL); + buf_LRU_free_block(&block->page, FALSE); } } diff --git a/storage/innodb_plugin/btr/btr0sea.c b/storage/innodb_plugin/btr/btr0sea.c index 035fdbb61d2..9835efcf712 100644 --- a/storage/innodb_plugin/btr/btr0sea.c +++ b/storage/innodb_plugin/btr/btr0sea.c @@ -141,7 +141,7 @@ btr_search_check_free_space_in_heap(void) be enough free space in the hash table. */ if (heap->free_block == NULL) { - buf_block_t* block = buf_block_alloc(0); + buf_block_t* block = buf_block_alloc(); rw_lock_x_lock(&btr_search_latch); diff --git a/storage/innodb_plugin/buf/buf0buddy.c b/storage/innodb_plugin/buf/buf0buddy.c index ee5a569c3ff..63c99571510 100644 --- a/storage/innodb_plugin/buf/buf0buddy.c +++ b/storage/innodb_plugin/buf/buf0buddy.c @@ -327,7 +327,7 @@ buf_buddy_alloc_low( /* Try replacing an uncompressed page in the buffer pool. */ buf_pool_mutex_exit(); - block = buf_LRU_get_free_block(0); + block = buf_LRU_get_free_block(); *lru = TRUE; buf_pool_mutex_enter(); diff --git a/storage/innodb_plugin/buf/buf0buf.c b/storage/innodb_plugin/buf/buf0buf.c index dac416f9472..6e76e4c65be 100644 --- a/storage/innodb_plugin/buf/buf0buf.c +++ b/storage/innodb_plugin/buf/buf0buf.c @@ -1283,7 +1283,7 @@ shrink_again: buf_LRU_make_block_old(&block->page); dirty++; - } else if (buf_LRU_free_block(&block->page, TRUE, NULL) + } else if (buf_LRU_free_block(&block->page, TRUE) != BUF_LRU_FREED) { nonfree++; } @@ -1729,8 +1729,7 @@ err_exit: mutex_enter(block_mutex); /* Discard the uncompressed page frame if possible. */ - if (buf_LRU_free_block(bpage, FALSE, NULL) - == BUF_LRU_FREED) { + if (buf_LRU_free_block(bpage, FALSE) == BUF_LRU_FREED) { mutex_exit(block_mutex); goto lookup; @@ -2165,7 +2164,7 @@ wait_until_unfixed: buf_pool_mutex_exit(); mutex_exit(&buf_pool_zip_mutex); - block = buf_LRU_get_free_block(0); + block = buf_LRU_get_free_block(); ut_a(block); buf_pool_mutex_enter(); @@ -2291,8 +2290,7 @@ wait_until_unfixed: /* Try to evict the block from the buffer pool, to use the insert buffer as much as possible. */ - if (buf_LRU_free_block(&block->page, TRUE, NULL) - == BUF_LRU_FREED) { + if (buf_LRU_free_block(&block->page, TRUE) == BUF_LRU_FREED) { buf_pool_mutex_exit(); mutex_exit(&block->mutex); fprintf(stderr, @@ -2829,7 +2827,7 @@ buf_page_init_for_read( && UNIV_LIKELY(!recv_recovery_is_on())) { block = NULL; } else { - block = buf_LRU_get_free_block(0); + block = buf_LRU_get_free_block(); ut_ad(block); } @@ -3001,7 +2999,7 @@ buf_page_create( ut_ad(mtr->state == MTR_ACTIVE); ut_ad(space || !zip_size); - free_block = buf_LRU_get_free_block(0); + free_block = buf_LRU_get_free_block(); buf_pool_mutex_enter(); diff --git a/storage/innodb_plugin/buf/buf0lru.c b/storage/innodb_plugin/buf/buf0lru.c index e4cf218bf2e..39feb06ff23 100644 --- a/storage/innodb_plugin/buf/buf0lru.c +++ b/storage/innodb_plugin/buf/buf0lru.c @@ -575,7 +575,7 @@ buf_LRU_free_from_unzip_LRU_list( ut_ad(block->page.in_LRU_list); mutex_enter(&block->mutex); - freed = buf_LRU_free_block(&block->page, FALSE, NULL); + freed = buf_LRU_free_block(&block->page, FALSE); mutex_exit(&block->mutex); switch (freed) { @@ -636,7 +636,7 @@ buf_LRU_free_from_common_LRU_list( mutex_enter(block_mutex); accessed = buf_page_is_accessed(bpage); - freed = buf_LRU_free_block(bpage, TRUE, NULL); + freed = buf_LRU_free_block(bpage, TRUE); mutex_exit(block_mutex); switch (freed) { @@ -798,10 +798,8 @@ LRU list to the free list. @return the free control block, in state BUF_BLOCK_READY_FOR_USE */ UNIV_INTERN buf_block_t* -buf_LRU_get_free_block( -/*===================*/ - ulint zip_size) /*!< in: compressed page size in bytes, - or 0 if uncompressed tablespace */ +buf_LRU_get_free_block(void) +/*========================*/ { buf_block_t* block = NULL; ibool freed; @@ -877,26 +875,10 @@ loop: /* If there is a block in the free list, take it */ block = buf_LRU_get_free_only(); + buf_pool_mutex_exit(); + if (block) { - -#ifdef UNIV_DEBUG - block->page.zip.m_start = -#endif /* UNIV_DEBUG */ - block->page.zip.m_end = - block->page.zip.m_nonempty = - block->page.zip.n_blobs = 0; - - if (UNIV_UNLIKELY(zip_size)) { - ibool lru; - page_zip_set_size(&block->page.zip, zip_size); - block->page.zip.data = buf_buddy_alloc(zip_size, &lru); - UNIV_MEM_DESC(block->page.zip.data, zip_size, block); - } else { - page_zip_set_size(&block->page.zip, 0); - block->page.zip.data = NULL; - } - - buf_pool_mutex_exit(); + memset(&block->page.zip, 0, sizeof block->page.zip); if (started_monitor) { srv_print_innodb_monitor = mon_value_was; @@ -908,8 +890,6 @@ loop: /* If no block was in the free list, search from the end of the LRU list and try to free a block there */ - buf_pool_mutex_exit(); - freed = buf_LRU_search_and_free_block(n_iterations); if (freed > 0) { @@ -1378,12 +1358,8 @@ enum buf_lru_free_block_status buf_LRU_free_block( /*===============*/ buf_page_t* bpage, /*!< in: block to be freed */ - ibool zip, /*!< in: TRUE if should remove also the + ibool zip) /*!< in: TRUE if should remove also the compressed page of an uncompressed page */ - ibool* buf_pool_mutex_released) - /*!< in: pointer to a variable that will - be assigned TRUE if buf_pool_mutex - was temporarily released, or NULL */ { buf_page_t* b = NULL; mutex_t* block_mutex = buf_page_get_mutex(bpage); @@ -1554,10 +1530,6 @@ alloc: b->io_fix = BUF_IO_READ; } - if (buf_pool_mutex_released) { - *buf_pool_mutex_released = TRUE; - } - buf_pool_mutex_exit(); mutex_exit(block_mutex); diff --git a/storage/innodb_plugin/include/buf0buf.h b/storage/innodb_plugin/include/buf0buf.h index cd4ee5906f0..d903b443920 100644 --- a/storage/innodb_plugin/include/buf0buf.h +++ b/storage/innodb_plugin/include/buf0buf.h @@ -165,10 +165,8 @@ Allocates a buffer block. @return own: the allocated block, in state BUF_BLOCK_MEMORY */ UNIV_INLINE buf_block_t* -buf_block_alloc( -/*============*/ - ulint zip_size); /*!< in: compressed page size in bytes, - or 0 if uncompressed tablespace */ +buf_block_alloc(void); +/*=================*/ /********************************************************************//** Frees a buffer block which does not contain a file page. */ UNIV_INLINE diff --git a/storage/innodb_plugin/include/buf0buf.ic b/storage/innodb_plugin/include/buf0buf.ic index 23db684806c..0025bef5aac 100644 --- a/storage/innodb_plugin/include/buf0buf.ic +++ b/storage/innodb_plugin/include/buf0buf.ic @@ -719,14 +719,12 @@ Allocates a buffer block. @return own: the allocated block, in state BUF_BLOCK_MEMORY */ UNIV_INLINE buf_block_t* -buf_block_alloc( -/*============*/ - ulint zip_size) /*!< in: compressed page size in bytes, - or 0 if uncompressed tablespace */ +buf_block_alloc(void) +/*=================*/ { buf_block_t* block; - block = buf_LRU_get_free_block(zip_size); + block = buf_LRU_get_free_block(); buf_block_set_state(block, BUF_BLOCK_MEMORY); diff --git a/storage/innodb_plugin/include/buf0lru.h b/storage/innodb_plugin/include/buf0lru.h index 5a9cfd059f3..d543bce53cd 100644 --- a/storage/innodb_plugin/include/buf0lru.h +++ b/storage/innodb_plugin/include/buf0lru.h @@ -110,12 +110,9 @@ enum buf_lru_free_block_status buf_LRU_free_block( /*===============*/ buf_page_t* bpage, /*!< in: block to be freed */ - ibool zip, /*!< in: TRUE if should remove also the + ibool zip) /*!< in: TRUE if should remove also the compressed page of an uncompressed page */ - ibool* buf_pool_mutex_released); - /*!< in: pointer to a variable that will - be assigned TRUE if buf_pool_mutex - was temporarily released, or NULL */ + __attribute__((nonnull)); /******************************************************************//** Try to free a replaceable block. @return TRUE if found and freed */ @@ -146,10 +143,9 @@ LRU list to the free list. @return the free control block, in state BUF_BLOCK_READY_FOR_USE */ UNIV_INTERN buf_block_t* -buf_LRU_get_free_block( -/*===================*/ - ulint zip_size); /*!< in: compressed page size in bytes, - or 0 if uncompressed tablespace */ +buf_LRU_get_free_block(void) +/*========================*/ + __attribute__((warn_unused_result)); /******************************************************************//** Puts a block back to the free list. */ diff --git a/storage/innodb_plugin/mem/mem0mem.c b/storage/innodb_plugin/mem/mem0mem.c index 1dd4db30841..86100b04fd6 100644 --- a/storage/innodb_plugin/mem/mem0mem.c +++ b/storage/innodb_plugin/mem/mem0mem.c @@ -347,7 +347,7 @@ mem_heap_create_block( return(NULL); } } else { - buf_block = buf_block_alloc(0); + buf_block = buf_block_alloc(); } block = (mem_block_t*) buf_block->frame; diff --git a/storage/innodb_plugin/page/page0zip.c b/storage/innodb_plugin/page/page0zip.c index d3b1edefc6b..bb9b0995c72 100644 --- a/storage/innodb_plugin/page/page0zip.c +++ b/storage/innodb_plugin/page/page0zip.c @@ -4439,7 +4439,7 @@ page_zip_reorganize( log_mode = mtr_set_log_mode(mtr, MTR_LOG_NONE); #ifndef UNIV_HOTBACKUP - temp_block = buf_block_alloc(0); + temp_block = buf_block_alloc(); btr_search_drop_page_hash_index(block); block->check_index_page_at_flush = TRUE; #else /* !UNIV_HOTBACKUP */ From 46b7ef69916635ca0575ea4898ed8980f4bf6f5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 25 Jan 2011 11:54:50 +0200 Subject: [PATCH 08/20] Bug#59464 Race condition in row_vers_build_for_semi_consistent_read row_vers_build_for_semi_consistent_read(): Dereference version_trx before releasing kernel_mutex, but not thereafter. --- storage/innobase/row/row0vers.c | 10 +++++++--- storage/innodb_plugin/ChangeLog | 5 +++++ storage/innodb_plugin/row/row0vers.c | 10 +++++++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/storage/innobase/row/row0vers.c b/storage/innobase/row/row0vers.c index f4adfa855df..23aca8c3f2e 100644 --- a/storage/innobase/row/row0vers.c +++ b/storage/innobase/row/row0vers.c @@ -593,11 +593,15 @@ row_vers_build_for_semi_consistent_read( mutex_enter(&kernel_mutex); version_trx = trx_get_on_id(version_trx_id); + if (version_trx + && (version_trx->conc_state == TRX_COMMITTED_IN_MEMORY + || version_trx->conc_state == TRX_NOT_STARTED)) { + + version_trx = NULL; + } mutex_exit(&kernel_mutex); - if (!version_trx - || version_trx->conc_state == TRX_NOT_STARTED - || version_trx->conc_state == TRX_COMMITTED_IN_MEMORY) { + if (!version_trx) { /* We found a version that belongs to a committed transaction: return it. */ diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 24cac7ac2be..d5e9a6bc825 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,8 @@ +2011-01-25 The InnoDB Team + + * row/row0vers.c: + Fix Bug#59464 Race condition in row_vers_build_for_semi_consistent_read + 2011-01-25 The InnoDB Team * btr/btr0btr.c, btr/btr0cur.c, btr/btr0sea.c, diff --git a/storage/innodb_plugin/row/row0vers.c b/storage/innodb_plugin/row/row0vers.c index b6d35363f08..d4fde0b939b 100644 --- a/storage/innodb_plugin/row/row0vers.c +++ b/storage/innodb_plugin/row/row0vers.c @@ -669,11 +669,15 @@ row_vers_build_for_semi_consistent_read( mutex_enter(&kernel_mutex); version_trx = trx_get_on_id(version_trx_id); + if (version_trx + && (version_trx->conc_state == TRX_COMMITTED_IN_MEMORY + || version_trx->conc_state == TRX_NOT_STARTED)) { + + version_trx = NULL; + } mutex_exit(&kernel_mutex); - if (!version_trx - || version_trx->conc_state == TRX_NOT_STARTED - || version_trx->conc_state == TRX_COMMITTED_IN_MEMORY) { + if (!version_trx) { /* We found a version that belongs to a committed transaction: return it. */ From 896e0ba4e0304fbd1b056022d8e27f6ce146a83e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 25 Jan 2011 12:17:28 +0200 Subject: [PATCH 09/20] Bug#59486 Incorrect usage of UNIV_UNLIKELY() in mlog_parse_string() mlog_parse_string(): Enclose the comparison in UNIV_UNLIKELY, not the comparand. --- storage/innodb_plugin/ChangeLog | 5 +++++ storage/innodb_plugin/mtr/mtr0log.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index d5e9a6bc825..cac72fd3075 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,8 @@ +2011-01-25 The InnoDB Team + + * mtr/mtr0log.c: + Bug#59486 Incorrect usage of UNIV_UNLIKELY() in mlog_parse_string() + 2011-01-25 The InnoDB Team * row/row0vers.c: diff --git a/storage/innodb_plugin/mtr/mtr0log.c b/storage/innodb_plugin/mtr/mtr0log.c index 3f3dab36b76..3349036b5b3 100644 --- a/storage/innodb_plugin/mtr/mtr0log.c +++ b/storage/innodb_plugin/mtr/mtr0log.c @@ -408,7 +408,7 @@ mlog_parse_string( ptr += 2; if (UNIV_UNLIKELY(offset >= UNIV_PAGE_SIZE) - || UNIV_UNLIKELY(len + offset) > UNIV_PAGE_SIZE) { + || UNIV_UNLIKELY(len + offset > UNIV_PAGE_SIZE)) { recv_sys->found_corrupt_log = TRUE; return(NULL); From e44703db76e87fccbcc2e51606f04b18b55a0544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 25 Jan 2011 15:43:08 +0200 Subject: [PATCH 10/20] Bug#59585 Fix 58912 introduces compiler warning due to potentially uninitialized variable row_upd_changes_ord_field_binary(): Initialize dfield_len to suppress the warning. The compiler cannot know that row_ext_lookup() does initialize dfield_len for us, as it is defined in a different module. --- storage/innodb_plugin/ChangeLog | 6 ++++++ storage/innodb_plugin/row/row0upd.c | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index cac72fd3075..e2fdeecfcc1 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,9 @@ +2011-01-25 The InnoDB Team + + * row/row0upd.c: + Bug#59585 Fix 58912 introduces compiler warning + due to potentially uninitialized variable + 2011-01-25 The InnoDB Team * mtr/mtr0log.c: diff --git a/storage/innodb_plugin/row/row0upd.c b/storage/innodb_plugin/row/row0upd.c index 4aa1474a25b..691d263e6ed 100644 --- a/storage/innodb_plugin/row/row0upd.c +++ b/storage/innodb_plugin/row/row0upd.c @@ -1252,6 +1252,10 @@ row_upd_changes_ord_field_binary( || dfield_is_null(dfield)) { /* do nothing special */ } else if (UNIV_LIKELY_NULL(ext)) { + /* Silence a compiler warning without + silencing a Valgrind error. */ + dfield_len = 0; + UNIV_MEM_INVALID(&dfield_len, sizeof dfield_len); /* See if the column is stored externally. */ buf = row_ext_lookup(ext, col_no, &dfield_len); From 786ac62c82038ed42278b3699b0661f0bb3c80ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 27 Jan 2011 13:27:29 +0200 Subject: [PATCH 11/20] Bug#59440 Race condition in XA ROLLBACK and XA COMMIT after server restart trx_get_trx_by_xid(): Invalidate trx->xid after a successful lookup, so that subsequent callers will not find the same transaction. The only callers of trx_get_trx_by_xid() will be invoking innobase_commit_low() or innobase_rollback_trx(), and those code paths should not depend on trx->xid. rb://584 approved by Jimmy Yang --- storage/innobase/include/trx0trx.h | 5 +++-- storage/innobase/trx/trx0trx.c | 26 ++++++++++++------------- storage/innodb_plugin/ChangeLog | 6 ++++++ storage/innodb_plugin/include/trx0trx.h | 4 ++-- storage/innodb_plugin/trx/trx0trx.c | 25 +++++++++++------------- 5 files changed, 34 insertions(+), 32 deletions(-) diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h index 97a47d9f46e..4652f45892e 100644 --- a/storage/innobase/include/trx0trx.h +++ b/storage/innobase/include/trx0trx.h @@ -198,8 +198,9 @@ which is in the prepared state */ trx_t * trx_get_trx_by_xid( /*===============*/ - /* out: trx or NULL */ - XID* xid); /* in: X/Open XA transaction identification */ + /* out: trx or NULL; + on match, the trx->xid will be invalidated */ + const XID* xid); /* in: X/Open XA transaction identifier */ /************************************************************************** If required, flushes the log to disk if we called trx_commit_for_mysql() with trx->flush_log_later == TRUE. */ diff --git a/storage/innobase/trx/trx0trx.c b/storage/innobase/trx/trx0trx.c index 21f75e0818f..a82d7f452fc 100644 --- a/storage/innobase/trx/trx0trx.c +++ b/storage/innobase/trx/trx0trx.c @@ -2041,14 +2041,15 @@ which is in the prepared state */ trx_t* trx_get_trx_by_xid( /*===============*/ - /* out: trx or NULL */ - XID* xid) /* in: X/Open XA transaction identification */ + /* out: trx or NULL; + on match, the trx->xid will be invalidated */ + const XID* xid) /* in: X/Open XA transaction identifier */ { trx_t* trx; if (xid == NULL) { - return (NULL); + return(NULL); } mutex_enter(&kernel_mutex); @@ -2061,10 +2062,16 @@ trx_get_trx_by_xid( of gtrid_lenght+bqual_length bytes should be the same */ - if (xid->gtrid_length == trx->xid.gtrid_length + if (trx->conc_state == TRX_PREPARED + && xid->gtrid_length == trx->xid.gtrid_length && xid->bqual_length == trx->xid.bqual_length && memcmp(xid->data, trx->xid.data, xid->gtrid_length + xid->bqual_length) == 0) { + + /* Invalidate the XID, so that subsequent calls + will not find it. */ + memset(&trx->xid, 0, sizeof(trx->xid)); + trx->xid.formatID = -1; break; } @@ -2073,14 +2080,5 @@ trx_get_trx_by_xid( mutex_exit(&kernel_mutex); - if (trx) { - if (trx->conc_state != TRX_PREPARED) { - - return(NULL); - } - - return(trx); - } else { - return(NULL); - } + return(trx); } diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index e2fdeecfcc1..3e14b0052e7 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,9 @@ +2011-01-27 The InnoDB Team + + * include/trx0trx.h, trx/trx0trx.c: + Bug#59440 Race condition in XA ROLLBACK and XA COMMIT + after server restart + 2011-01-25 The InnoDB Team * row/row0upd.c: diff --git a/storage/innodb_plugin/include/trx0trx.h b/storage/innodb_plugin/include/trx0trx.h index abd175d365b..833bae4a4ff 100644 --- a/storage/innodb_plugin/include/trx0trx.h +++ b/storage/innodb_plugin/include/trx0trx.h @@ -214,12 +214,12 @@ trx_recover_for_mysql( /*******************************************************************//** This function is used to find one X/Open XA distributed transaction which is in the prepared state -@return trx or NULL */ +@return trx or NULL; on match, the trx->xid will be invalidated */ UNIV_INTERN trx_t * trx_get_trx_by_xid( /*===============*/ - XID* xid); /*!< in: X/Open XA transaction identification */ + const XID* xid); /*!< in: X/Open XA transaction identifier */ /**********************************************************************//** If required, flushes the log to disk if we called trx_commit_for_mysql() with trx->flush_log_later == TRUE. diff --git a/storage/innodb_plugin/trx/trx0trx.c b/storage/innodb_plugin/trx/trx0trx.c index ee744fd58b1..f0bbf220815 100644 --- a/storage/innodb_plugin/trx/trx0trx.c +++ b/storage/innodb_plugin/trx/trx0trx.c @@ -2010,18 +2010,18 @@ trx_recover_for_mysql( /*******************************************************************//** This function is used to find one X/Open XA distributed transaction which is in the prepared state -@return trx or NULL */ +@return trx or NULL; on match, the trx->xid will be invalidated */ UNIV_INTERN trx_t* trx_get_trx_by_xid( /*===============*/ - XID* xid) /*!< in: X/Open XA transaction identification */ + const XID* xid) /*!< in: X/Open XA transaction identifier */ { trx_t* trx; if (xid == NULL) { - return (NULL); + return(NULL); } mutex_enter(&kernel_mutex); @@ -2034,10 +2034,16 @@ trx_get_trx_by_xid( of gtrid_lenght+bqual_length bytes should be the same */ - if (xid->gtrid_length == trx->xid.gtrid_length + if (trx->conc_state == TRX_PREPARED + && xid->gtrid_length == trx->xid.gtrid_length && xid->bqual_length == trx->xid.bqual_length && memcmp(xid->data, trx->xid.data, xid->gtrid_length + xid->bqual_length) == 0) { + + /* Invalidate the XID, so that subsequent calls + will not find it. */ + memset(&trx->xid, 0, sizeof(trx->xid)); + trx->xid.formatID = -1; break; } @@ -2046,14 +2052,5 @@ trx_get_trx_by_xid( mutex_exit(&kernel_mutex); - if (trx) { - if (trx->conc_state != TRX_PREPARED) { - - return(NULL); - } - - return(trx); - } else { - return(NULL); - } + return(trx); } From 235e10d987222b8cc42e1f3a9a79834912d45026 Mon Sep 17 00:00:00 2001 From: Alfranio Correia Date: Fri, 28 Jan 2011 01:25:26 +0000 Subject: [PATCH 12/20] BUG#55675 rpl.rpl_log_pos fails sporadically with error binlog truncated in the middle There are two calls to read_log_event() on master in mysql_binlog_send(). Each call reads 19 bytes in this test case and the error of the second read_log_event() is reported to the slave. The second read_log_event() starts from position 94 (75 + 19) to 113 (75 + 19 + 19). Usually, there are two events in the binary log: . 0 - 3 - Header . 4 - 105 - Format Descriptor Event . 106 - 304 - Query Event and both reads fail because operations are reading from invalid positions as expected. However, mysql_binlog_send() does not use the same IO_CACHE that is used to write into binary log (i.e. mysql_bin_log.log_file) for the hot binary log. It opens the binary log file directly by calling open_binlog() and creates a separated IO_CACHE. So there is a possibly that after a master has flushed the binary log file, the content has been cached by the filesystem, and has not updated the disk file. If this happens, then a slave will only see part of the file, and thus the second read_log_event() will report event truncated error. To fix the problem, if the first read_log_event() has failed, we ensure that the second one will try to read from the same position. --- mysql-test/suite/rpl/t/disabled.def | 1 - sql/sql_repl.cc | 9 ++++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/mysql-test/suite/rpl/t/disabled.def b/mysql-test/suite/rpl/t/disabled.def index 3b3a55fe4cd..33f65ff3ecc 100644 --- a/mysql-test/suite/rpl/t/disabled.def +++ b/mysql-test/suite/rpl/t/disabled.def @@ -11,7 +11,6 @@ ############################################################################## rpl_row_create_table : Bug#51574 Feb 27 2010 andrei failed different way than earlier with bug#45576 -rpl_log_pos : BUG#55675 Sep 10 2010 27 2010 alfranio rpl.rpl_log_pos fails sporadically with error binlog truncated in the middle rpl_get_master_version_and_clock : Bug#59178 Jan 05 2011 joro Valgrind warnings rpl_get_master_version_and_clock rpl_row_until : BUG#59543 Jan 26 2011 alfranio Replication test from eits suite rpl_row_until times out rpl_stm_until : BUG#59543 Jan 26 2011 alfranio Replication test from eits suite rpl_row_until times out diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc index 8c769ce6acf..2a1efab13a9 100644 --- a/sql/sql_repl.cc +++ b/sql/sql_repl.cc @@ -545,8 +545,10 @@ impossible position"; while (!net->error && net->vio != 0 && !thd->killed) { + my_off_t prev_pos= pos; while (!(error = Log_event::read_log_event(&log, packet, log_lock))) { + prev_pos= my_b_tell(&log); #ifndef DBUG_OFF if (max_binlog_dump_events && !left_events--) { @@ -613,8 +615,13 @@ impossible position"; here we were reading binlog that was not closed properly (as a result of a crash ?). treat any corruption as EOF */ - if (binlog_can_be_corrupted && error != LOG_READ_MEM) + if (binlog_can_be_corrupted && + error != LOG_READ_MEM && error != LOG_READ_EOF) + { + my_b_seek(&log, prev_pos); error=LOG_READ_EOF; + } + /* TODO: now that we are logging the offset, check to make sure the recorded offset and the actual match. From 71e8043bae2071ba875b18326504b1058b8deb98 Mon Sep 17 00:00:00 2001 From: Jimmy Yang Date: Fri, 28 Jan 2011 00:50:10 -0800 Subject: [PATCH 13/20] Fix Bug #59465 btr_estimate_number_of_different_key_vals use incorrect offset for external_size rb://581 approved by Marko --- storage/innobase/btr/btr0cur.c | 10 +++++----- storage/innodb_plugin/ChangeLog | 7 +++++++ storage/innodb_plugin/btr/btr0cur.c | 10 +++++----- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/storage/innobase/btr/btr0cur.c b/storage/innobase/btr/btr0cur.c index 9f4babfaae6..6c0497cbd41 100644 --- a/storage/innobase/btr/btr0cur.c +++ b/storage/innobase/btr/btr0cur.c @@ -2981,6 +2981,9 @@ btr_estimate_number_of_different_key_vals( while (!page_rec_is_supremum(rec)) { rec_t* next_rec = page_rec_get_next(rec); if (page_rec_is_supremum(next_rec)) { + total_external_size += + btr_rec_get_externally_stored_len( + rec, offsets_rec); break; } @@ -2988,7 +2991,8 @@ btr_estimate_number_of_different_key_vals( matched_bytes = 0; offsets_next_rec = rec_get_offsets(next_rec, index, offsets_next_rec, - n_cols, &heap); + ULINT_UNDEFINED, + &heap); cmp_rec_rec_with_match(rec, next_rec, offsets_rec, offsets_next_rec, @@ -3043,10 +3047,6 @@ btr_estimate_number_of_different_key_vals( } } - offsets_rec = rec_get_offsets(rec, index, offsets_rec, - ULINT_UNDEFINED, &heap); - total_external_size += btr_rec_get_externally_stored_len( - rec, offsets_rec); mtr_commit(&mtr); } diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 3e14b0052e7..7a901fc1fa1 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,9 @@ +2011-01-27 The InnoDB Team + + * btr/btr0cur.c: + Bug#59465 btr_estimate_number_of_different_key_vals use + incorrect offset for external_size + 2011-01-27 The InnoDB Team * include/trx0trx.h, trx/trx0trx.c: @@ -29,6 +35,7 @@ Fix Bug#59707 Unused compression-related parameters in buffer pool functions +>>>>>>> MERGE-SOURCE 2011-01-18 The InnoDB Team * include/sync0rw.h, sync/sync0arr.c, sync/sync0rw.c: diff --git a/storage/innodb_plugin/btr/btr0cur.c b/storage/innodb_plugin/btr/btr0cur.c index f41b125b281..874db3066b5 100644 --- a/storage/innodb_plugin/btr/btr0cur.c +++ b/storage/innodb_plugin/btr/btr0cur.c @@ -3365,6 +3365,9 @@ btr_estimate_number_of_different_key_vals( while (!page_rec_is_supremum(rec)) { rec_t* next_rec = page_rec_get_next(rec); if (page_rec_is_supremum(next_rec)) { + total_external_size += + btr_rec_get_externally_stored_len( + rec, offsets_rec); break; } @@ -3372,7 +3375,8 @@ btr_estimate_number_of_different_key_vals( matched_bytes = 0; offsets_next_rec = rec_get_offsets(next_rec, index, offsets_next_rec, - n_cols, &heap); + ULINT_UNDEFINED, + &heap); cmp_rec_rec_with_match(rec, next_rec, offsets_rec, offsets_next_rec, @@ -3427,10 +3431,6 @@ btr_estimate_number_of_different_key_vals( } } - offsets_rec = rec_get_offsets(rec, index, offsets_rec, - ULINT_UNDEFINED, &heap); - total_external_size += btr_rec_get_externally_stored_len( - rec, offsets_rec); mtr_commit(&mtr); } From 8ce9b992462f0d2ef5e435e4ecda4700fa7db652 Mon Sep 17 00:00:00 2001 From: Alfranio Correia Date: Fri, 28 Jan 2011 12:09:15 +0000 Subject: [PATCH 14/20] BUG#59338 Inconsistency in binlog for statements that don't change any rows STATEMENT SBR In SBR, if a statement does not fail, it is always written to the binary log, regardless if rows are changed or not. If there is a failure, a statement is only written to the binary log if a non-transactional (.e.g. MyIsam) engine is updated. INSERT ON DUPLICATE KEY UPDATE and INSERT IGNORE were not following the rule above and were not written to the binary log, if then engine was Innodb. mysql-test/extra/rpl_tests/rpl_insert_duplicate.test: Added test case. mysql-test/extra/rpl_tests/rpl_insert_ignore.test: Updated test case. mysql-test/include/commit.inc: Updated test case as the calls to the binary log have changed for INSERT ON DUPLICATE and INSERT IGNORE. mysql-test/r/commit_1innodb.result: Updated result file. mysql-test/suite/rpl/r/rpl_insert_duplicate.result: Added test case. mysql-test/suite/rpl/r/rpl_insert_ignore.result: Updated result file. mysql-test/suite/rpl/t/rpl_insert_duplicate.test: Added test case. mysql-test/suite/rpl/t/rpl_insert_ignore.test: Improved test case. --- .../extra/rpl_tests/rpl_insert_duplicate.test | 64 +++++++++++++++++ .../extra/rpl_tests/rpl_insert_ignore.test | 70 +++++++++++-------- mysql-test/include/commit.inc | 8 +-- mysql-test/r/commit_1innodb.result | 8 +-- .../suite/rpl/r/rpl_insert_duplicate.result | 29 ++++++++ .../suite/rpl/r/rpl_insert_ignore.result | 64 +++++++---------- .../suite/rpl/t/rpl_insert_duplicate.test | 14 ++++ mysql-test/suite/rpl/t/rpl_insert_ignore.test | 8 ++- sql/sql_insert.cc | 2 +- 9 files changed, 189 insertions(+), 78 deletions(-) create mode 100644 mysql-test/extra/rpl_tests/rpl_insert_duplicate.test create mode 100644 mysql-test/suite/rpl/r/rpl_insert_duplicate.result create mode 100644 mysql-test/suite/rpl/t/rpl_insert_duplicate.test diff --git a/mysql-test/extra/rpl_tests/rpl_insert_duplicate.test b/mysql-test/extra/rpl_tests/rpl_insert_duplicate.test new file mode 100644 index 00000000000..77140174f4b --- /dev/null +++ b/mysql-test/extra/rpl_tests/rpl_insert_duplicate.test @@ -0,0 +1,64 @@ +# BUG#59338 Inconsistency in binlog for statements that don't change any rows STATEMENT SBR +# In SBR, if a statement does not fail, it is always written to the binary log, +# regardless if rows are changed or not. If there is a failure, a statement is +# only written to the binary log if a non-transactional (.e.g. MyIsam) engine +# is updated. INSERT ON DUPLICATE KEY UPDATE was not following the rule above +# and was not written to the binary log, if then engine was Innodb. +# +# In this test case, we check if INSERT ON DUPLICATE KEY UPDATE that does not +# change anything is still written to the binary log. + +# Prepare environment +--connection master + +eval CREATE TABLE t1 ( + a INT UNSIGNED NOT NULL PRIMARY KEY +) ENGINE=$engine_type; + +eval CREATE TABLE t2 ( + a INT UNSIGNED +) ENGINE=$engine_type; + +INSERT INTO t1 VALUES (1); +INSERT INTO t2 VALUES (1); + +# An insert duplicate that does not update anything must be written to the binary +# log in SBR and MIXED modes. We check this property by summing a before and after +# the update and comparing the binlog positions. The sum should be the same at both +# points and the statement should be in the binary log. +--let $binlog_file= query_get_value("SHOW MASTER STATUS", File, 1) +--let $binlog_start= query_get_value("SHOW MASTER STATUS", Position, 1) +--let $statement_file=INSERT INTO t1 SELECT t2.a FROM t2 ORDER BY t2.a ON DUPLICATE KEY UPDATE t1.a= t1.a +--eval $statement_file + +--let $assert_cond= SUM(a) = 1 FROM t1 +--let $assert_text= Sum of elements in t1 should be 1. +--source include/assert.inc + +if (`SELECT @@BINLOG_FORMAT = 'ROW'`) +{ + --let $binlog_position_cmp= = + --let $assert_cond= [SHOW MASTER STATUS, Position, 1] $binlog_position_cmp $binlog_start + --let $assert_text= In SBR or MIXED modes, the event in the binlog should be the same that was executed. In RBR mode, binlog position should stay unchanged. +} +if (`SELECT @@BINLOG_FORMAT != 'ROW' && UPPER('$engine_type') = UPPER('Innodb')`) +{ + --let $assert_cond= \'[\'SHOW BINLOG EVENTS IN "$binlog_file" FROM $binlog_start LIMIT 1, 1\', Info, 1]\' LIKE \'%$statement_file\' + --let $assert_text= In SBR or MIXED modes, the event in the binlog should be the same that was executed. In RBR mode, binlog position should stay unchanged. +} +if (`SELECT @@BINLOG_FORMAT != 'ROW' && UPPER('$engine_type') = UPPER('MyIsam')`) +{ + --let $assert_cond= \'[\'SHOW BINLOG EVENTS IN "$binlog_file" FROM $binlog_start LIMIT 0, 1\', Info, 1]\' LIKE \'%$statement_file\' + --let $assert_text= In SBR or MIXED modes, the event in the binlog should be the same that was executed. In RBR mode, binlog position should stay unchanged. +} +--source include/assert.inc + +# Compare master and slave +--sync_slave_with_master +--let $diff_tables= master:test.t1 , slave:test.t1 +--source include/diff_tables.inc + +# Clean up +--connection master +drop table t1, t2; +--sync_slave_with_master diff --git a/mysql-test/extra/rpl_tests/rpl_insert_ignore.test b/mysql-test/extra/rpl_tests/rpl_insert_ignore.test index 43d45ef6c60..1eb66358423 100644 --- a/mysql-test/extra/rpl_tests/rpl_insert_ignore.test +++ b/mysql-test/extra/rpl_tests/rpl_insert_ignore.test @@ -5,6 +5,7 @@ # Slave needs to be started with --innodb to store table in InnoDB. # Same test for MyISAM (which had no bug). +--connection master eval CREATE TABLE t1 ( a int unsigned not null auto_increment primary key, b int unsigned, @@ -32,38 +33,49 @@ INSERT INTO t2 VALUES (5, 4); INSERT INTO t2 VALUES (6, 6); INSERT IGNORE INTO t1 SELECT NULL, t2.b FROM t2 ORDER BY t2.a; +--let $assert_cond= COUNT(*) = 6 FROM t1 +--let $assert_text= Count of elements in t1 should be 6. +--source include/assert.inc -# Compare results +# Compare master and slave +--sync_slave_with_master +--let $diff_tables= master:test.t1 , slave:test.t1 +--source include/diff_tables.inc -SELECT * FROM t1 ORDER BY a; +# BUG#59338 Inconsistency in binlog for statements that don't change any rows STATEMENT SBR +# An insert ignore that does not update anything must be written to the binary log in SBR +# and MIXED modes. We check this property by counting occurrences in t1 before and after +# the insert and comparing the binlog positions. The count should be the same in both points +# and the statement should be in the binary log. +--connection master +--let $binlog_file= query_get_value("SHOW MASTER STATUS", File, 1) +--let $binlog_start= query_get_value("SHOW MASTER STATUS", Position, 1) +--let $statement_file=INSERT IGNORE INTO t1 SELECT NULL, t2.b FROM t2 ORDER BY t2.a +--eval $statement_file -sync_slave_with_master; -SELECT * FROM t1 ORDER BY a; +--let $assert_cond= COUNT(*) = 6 FROM t1 +--let $assert_text= Count of elements in t1 should be 6. +--source include/assert.inc -# Now do the same for MyISAM +if (`SELECT @@BINLOG_FORMAT = 'ROW'`) +{ + --let $binlog_position_cmp= = + --let $assert_cond= [SHOW MASTER STATUS, Position, 1] $binlog_position_cmp $binlog_start + --let $assert_text= In SBR or MIXED modes, the event in the binlog should be the same that was executed. In RBR mode, binlog position should stay unchanged. +} +if (`SELECT @@BINLOG_FORMAT != 'ROW' && UPPER('$engine_type') = UPPER('Innodb')`) +{ + --let $assert_cond= \'[\'SHOW BINLOG EVENTS IN "$binlog_file" FROM $binlog_start LIMIT 2, 1\', Info, 1]\' LIKE \'%$statement_file\' + --let $assert_text= In SBR or MIXED modes, the event in the binlog should be the same that was executed. In RBR mode, binlog position should stay unchanged. +} +if (`SELECT @@BINLOG_FORMAT != 'ROW' && UPPER('$engine_type') = UPPER('MyIsam')`) +{ + --let $assert_cond= \'[\'SHOW BINLOG EVENTS IN "$binlog_file" FROM $binlog_start LIMIT 1, 1\', Info, 1]\' LIKE \'%$statement_file\' + --let $assert_text= In SBR or MIXED modes, the event in the binlog should be the same that was executed. In RBR mode, binlog position should stay unchanged. +} +--source include/assert.inc -connection master; -drop table t1; -eval CREATE TABLE t1 ( - a int unsigned not null auto_increment primary key, - b int unsigned, - unique (b) -) ENGINE=$engine_type2; - -INSERT INTO t1 VALUES (1, 1); -INSERT INTO t1 VALUES (2, 2); -INSERT INTO t1 VALUES (3, 3); -INSERT INTO t1 VALUES (4, 4); - -INSERT IGNORE INTO t1 SELECT NULL, t2.b FROM t2 ORDER BY t2.a; - -SELECT * FROM t1 ORDER BY a; - -sync_slave_with_master; -SELECT * FROM t1 ORDER BY a; - -connection master; +# Clean up +--connection master drop table t1, t2; -sync_slave_with_master; - -# End of 4.1 tests +--sync_slave_with_master diff --git a/mysql-test/include/commit.inc b/mysql-test/include/commit.inc index d412eae8364..7924f9bc96f 100644 --- a/mysql-test/include/commit.inc +++ b/mysql-test/include/commit.inc @@ -502,16 +502,16 @@ call p_verify_status_increment(2, 2, 2, 2); --echo # 12. Read-write statement: IODKU, change 0 rows. --echo # insert t1 set a=2 on duplicate key update a=2; -call p_verify_status_increment(1, 0, 1, 0); +call p_verify_status_increment(2, 2, 1, 0); commit; -call p_verify_status_increment(1, 0, 1, 0); +call p_verify_status_increment(2, 2, 1, 0); --echo # 13. Read-write statement: INSERT IGNORE, change 0 rows. --echo # insert ignore t1 set a=2; -call p_verify_status_increment(1, 0, 1, 0); +call p_verify_status_increment(2, 2, 1, 0); commit; -call p_verify_status_increment(1, 0, 1, 0); +call p_verify_status_increment(2, 2, 1, 0); --echo # 14. Read-write statement: INSERT IGNORE, change 1 row. --echo # diff --git a/mysql-test/r/commit_1innodb.result b/mysql-test/r/commit_1innodb.result index 1f0b2c8019b..d21af5ef555 100644 --- a/mysql-test/r/commit_1innodb.result +++ b/mysql-test/r/commit_1innodb.result @@ -518,21 +518,21 @@ SUCCESS # 12. Read-write statement: IODKU, change 0 rows. # insert t1 set a=2 on duplicate key update a=2; -call p_verify_status_increment(1, 0, 1, 0); +call p_verify_status_increment(2, 2, 1, 0); SUCCESS commit; -call p_verify_status_increment(1, 0, 1, 0); +call p_verify_status_increment(2, 2, 1, 0); SUCCESS # 13. Read-write statement: INSERT IGNORE, change 0 rows. # insert ignore t1 set a=2; -call p_verify_status_increment(1, 0, 1, 0); +call p_verify_status_increment(2, 2, 1, 0); SUCCESS commit; -call p_verify_status_increment(1, 0, 1, 0); +call p_verify_status_increment(2, 2, 1, 0); SUCCESS # 14. Read-write statement: INSERT IGNORE, change 1 row. diff --git a/mysql-test/suite/rpl/r/rpl_insert_duplicate.result b/mysql-test/suite/rpl/r/rpl_insert_duplicate.result new file mode 100644 index 00000000000..61ebbaaa5a9 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_insert_duplicate.result @@ -0,0 +1,29 @@ +include/master-slave.inc +[connection master] +CREATE TABLE t1 ( +a INT UNSIGNED NOT NULL PRIMARY KEY +) ENGINE=innodb; +CREATE TABLE t2 ( +a INT UNSIGNED +) ENGINE=innodb; +INSERT INTO t1 VALUES (1); +INSERT INTO t2 VALUES (1); +INSERT INTO t1 SELECT t2.a FROM t2 ORDER BY t2.a ON DUPLICATE KEY UPDATE t1.a= t1.a; +include/assert.inc [Sum of elements in t1 should be 1.] +include/assert.inc [In SBR or MIXED modes, the event in the binlog should be the same that was executed. In RBR mode, binlog position should stay unchanged.] +include/diff_tables.inc [master:test.t1 , slave:test.t1] +drop table t1, t2; +CREATE TABLE t1 ( +a INT UNSIGNED NOT NULL PRIMARY KEY +) ENGINE=myisam; +CREATE TABLE t2 ( +a INT UNSIGNED +) ENGINE=myisam; +INSERT INTO t1 VALUES (1); +INSERT INTO t2 VALUES (1); +INSERT INTO t1 SELECT t2.a FROM t2 ORDER BY t2.a ON DUPLICATE KEY UPDATE t1.a= t1.a; +include/assert.inc [Sum of elements in t1 should be 1.] +include/assert.inc [In SBR or MIXED modes, the event in the binlog should be the same that was executed. In RBR mode, binlog position should stay unchanged.] +include/diff_tables.inc [master:test.t1 , slave:test.t1] +drop table t1, t2; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_insert_ignore.result b/mysql-test/suite/rpl/r/rpl_insert_ignore.result index 6937c3d0987..5d9c4aec9f7 100644 --- a/mysql-test/suite/rpl/r/rpl_insert_ignore.result +++ b/mysql-test/suite/rpl/r/rpl_insert_ignore.result @@ -20,48 +20,36 @@ INSERT INTO t2 VALUES (4, 3); INSERT INTO t2 VALUES (5, 4); INSERT INTO t2 VALUES (6, 6); INSERT IGNORE INTO t1 SELECT NULL, t2.b FROM t2 ORDER BY t2.a; -SELECT * FROM t1 ORDER BY a; -a b -1 1 -2 2 -3 3 -4 4 -5 5 -6 6 -SELECT * FROM t1 ORDER BY a; -a b -1 1 -2 2 -3 3 -4 4 -5 5 -6 6 -drop table t1; +include/assert.inc [Count of elements in t1 should be 6.] +include/diff_tables.inc [master:test.t1 , slave:test.t1] +INSERT IGNORE INTO t1 SELECT NULL, t2.b FROM t2 ORDER BY t2.a; +include/assert.inc [Count of elements in t1 should be 6.] +include/assert.inc [In SBR or MIXED modes, the event in the binlog should be the same that was executed. In RBR mode, binlog position should stay unchanged.] +drop table t1, t2; CREATE TABLE t1 ( a int unsigned not null auto_increment primary key, b int unsigned, unique (b) -) ENGINE=myisam; -INSERT INTO t1 VALUES (1, 1); -INSERT INTO t1 VALUES (2, 2); -INSERT INTO t1 VALUES (3, 3); -INSERT INTO t1 VALUES (4, 4); +) ENGINE=innodb; +CREATE TABLE t2 ( +a int unsigned, # to force INSERT SELECT to have a certain order +b int unsigned +) ENGINE=innodb; +INSERT INTO t1 VALUES (NULL, 1); +INSERT INTO t1 VALUES (NULL, 2); +INSERT INTO t1 VALUES (NULL, 3); +INSERT INTO t1 VALUES (NULL, 4); +INSERT INTO t2 VALUES (1, 1); +INSERT INTO t2 VALUES (2, 2); +INSERT INTO t2 VALUES (3, 5); +INSERT INTO t2 VALUES (4, 3); +INSERT INTO t2 VALUES (5, 4); +INSERT INTO t2 VALUES (6, 6); INSERT IGNORE INTO t1 SELECT NULL, t2.b FROM t2 ORDER BY t2.a; -SELECT * FROM t1 ORDER BY a; -a b -1 1 -2 2 -3 3 -4 4 -5 5 -6 6 -SELECT * FROM t1 ORDER BY a; -a b -1 1 -2 2 -3 3 -4 4 -5 5 -6 6 +include/assert.inc [Count of elements in t1 should be 6.] +include/diff_tables.inc [master:test.t1 , slave:test.t1] +INSERT IGNORE INTO t1 SELECT NULL, t2.b FROM t2 ORDER BY t2.a; +include/assert.inc [Count of elements in t1 should be 6.] +include/assert.inc [In SBR or MIXED modes, the event in the binlog should be the same that was executed. In RBR mode, binlog position should stay unchanged.] drop table t1, t2; include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_insert_duplicate.test b/mysql-test/suite/rpl/t/rpl_insert_duplicate.test new file mode 100644 index 00000000000..7dd38a696ae --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_insert_duplicate.test @@ -0,0 +1,14 @@ +######################################### +# Wrapper for rpl_insert_duplicate.test # +######################################### +-- source include/master-slave.inc +-- source include/have_innodb.inc +#-- source include/have_binlog_format_mixed_or_statement.inc + +let $engine_type=innodb; +-- source extra/rpl_tests/rpl_insert_duplicate.test + +let $engine_type=myisam; +-- source extra/rpl_tests/rpl_insert_duplicate.test + +--source include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_insert_ignore.test b/mysql-test/suite/rpl/t/rpl_insert_ignore.test index 1d6c8e7168e..265a52fb51d 100644 --- a/mysql-test/suite/rpl/t/rpl_insert_ignore.test +++ b/mysql-test/suite/rpl/t/rpl_insert_ignore.test @@ -4,7 +4,11 @@ -- source include/not_ndb_default.inc -- source include/have_innodb.inc -- source include/master-slave.inc -let $engine_type=innodb; -let $engine_type2=myisam; + +-- let $engine_type=innodb -- source extra/rpl_tests/rpl_insert_ignore.test + +-- let $engine_type=innodb +-- source extra/rpl_tests/rpl_insert_ignore.test + --source include/rpl_end.inc diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index f0735a9e093..e2f93ee4de5 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -881,7 +881,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, */ query_cache_invalidate3(thd, table_list, 1); } - if ((changed && error <= 0) || + if (error <= 0 || thd->transaction.stmt.modified_non_trans_table || was_insert_delayed) { From 2d2f80e07e858f157a0823c5e886c3589cd1a435 Mon Sep 17 00:00:00 2001 From: Alfranio Correia Date: Mon, 31 Jan 2011 14:31:33 +0000 Subject: [PATCH 15/20] Post-fix for BUG#59338. --- mysql-test/suite/rpl/r/rpl_insert_ignore.result | 4 ++-- mysql-test/suite/rpl/t/rpl_insert_ignore.test | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mysql-test/suite/rpl/r/rpl_insert_ignore.result b/mysql-test/suite/rpl/r/rpl_insert_ignore.result index 5d9c4aec9f7..63b9244a40d 100644 --- a/mysql-test/suite/rpl/r/rpl_insert_ignore.result +++ b/mysql-test/suite/rpl/r/rpl_insert_ignore.result @@ -30,11 +30,11 @@ CREATE TABLE t1 ( a int unsigned not null auto_increment primary key, b int unsigned, unique (b) -) ENGINE=innodb; +) ENGINE=myisam; CREATE TABLE t2 ( a int unsigned, # to force INSERT SELECT to have a certain order b int unsigned -) ENGINE=innodb; +) ENGINE=myisam; INSERT INTO t1 VALUES (NULL, 1); INSERT INTO t1 VALUES (NULL, 2); INSERT INTO t1 VALUES (NULL, 3); diff --git a/mysql-test/suite/rpl/t/rpl_insert_ignore.test b/mysql-test/suite/rpl/t/rpl_insert_ignore.test index 265a52fb51d..4129d4d553e 100644 --- a/mysql-test/suite/rpl/t/rpl_insert_ignore.test +++ b/mysql-test/suite/rpl/t/rpl_insert_ignore.test @@ -8,7 +8,7 @@ -- let $engine_type=innodb -- source extra/rpl_tests/rpl_insert_ignore.test --- let $engine_type=innodb +-- let $engine_type=myisam -- source extra/rpl_tests/rpl_insert_ignore.test --source include/rpl_end.inc From 59269b1da1552ae21e935c04588844c573e9c0c3 Mon Sep 17 00:00:00 2001 From: Ole John Aske Date: Tue, 1 Feb 2011 13:20:16 +0100 Subject: [PATCH 16/20] Fix for bug#57030: ('BETWEEN' evaluation is incorrect') Root cause for this bug is that the optimizer try to detect& optimize the special case: ' BETWEEN c1 AND c1' and handle this as the condition ' = c1' This was implemented inside add_key_field(.. *field, *value[]...) which assumed field to refer key Field, and value[] to refer a [low...high] constant pair. value[0] and value[1] was then compared for equality. In a 'normal' BETWEEN condition of the form ' BETWEEN val1 and val2' the BETWEEN operation is represented with an argementlist containing the values [, val1, val2] - add_key_field() is then called with parameters field=, *value=val1. However, if the BETWEEN predicate specified: 1) ' BETWEEN AND the 'field' and 'value' arguments to add_key_field() had to be swapped. This was implemented by trying to cheat add_key_field() to handle it like: 2) ' GE AND LE' As we didn't really replace the BETWEEN operation with 'ge' and 'le', add_key_field() still handled it as a 'BETWEEN' and compared the (swapped) arguments and for equality. If they was equal, the condition 1) was incorrectly 'optimized' to: 3) ' EQ ' This fix moves this optimization of ' BETWEEN c1 AND c1' into add_key_fields() which then calls add_key_equal_fields() to collect key equality / comparison for the key fields in the BETWEEN condition. --- mysql-test/r/range.result | 101 ++++++++++++++++++++++++++++++++++++++ mysql-test/t/range.test | 67 +++++++++++++++++++++++++ sql/sql_select.cc | 98 ++++++++++++++++++++++-------------- 3 files changed, 228 insertions(+), 38 deletions(-) diff --git a/mysql-test/r/range.result b/mysql-test/r/range.result index d989896514c..ae63edf87b9 100644 --- a/mysql-test/r/range.result +++ b/mysql-test/r/range.result @@ -1666,4 +1666,105 @@ c_key c_notkey 1 1 3 3 DROP TABLE t1; +# +# Bug #57030: 'BETWEEN' evaluation is incorrect +# +CREATE TABLE t1(pk INT PRIMARY KEY, i4 INT); +CREATE UNIQUE INDEX i4_uq ON t1(i4); +INSERT INTO t1 VALUES (1,10), (2,20), (3,30); +EXPLAIN +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 10; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 const i4_uq i4_uq 5 const 1 +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 10; +pk i4 +1 10 +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND i4; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 const i4_uq i4_uq 5 const 1 +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND i4; +pk i4 +1 10 +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND i4; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 range i4_uq i4_uq 5 NULL 3 Using where +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND i4; +pk i4 +1 10 +2 20 +3 30 +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND 10; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 range i4_uq i4_uq 5 NULL 1 Using where +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND 10; +pk i4 +1 10 +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND 10; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL NULL NULL NULL NULL 3 +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND 10; +pk i4 +1 10 +2 20 +3 30 +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN 11 AND 11; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Impossible WHERE +SELECT * FROM t1 WHERE 10 BETWEEN 11 AND 11; +pk i4 +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN 100 AND 0; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Impossible WHERE +SELECT * FROM t1 WHERE 10 BETWEEN 100 AND 0; +pk i4 +EXPLAIN +SELECT * FROM t1 WHERE i4 BETWEEN 100 AND 0; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Impossible WHERE noticed after reading const tables +SELECT * FROM t1 WHERE i4 BETWEEN 100 AND 0; +pk i4 +EXPLAIN +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 99999999999999999; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 range i4_uq i4_uq 5 NULL 2 Using where +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 99999999999999999; +pk i4 +1 10 +2 20 +3 30 +EXPLAIN +SELECT * FROM t1 WHERE i4 BETWEEN 999999999999999 AND 30; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Impossible WHERE noticed after reading const tables +SELECT * FROM t1 WHERE i4 BETWEEN 999999999999999 AND 30; +pk i4 +EXPLAIN +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND '20'; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 range i4_uq i4_uq 5 NULL 1 Using where +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND '20'; +pk i4 +1 10 +2 20 +EXPLAIN +SELECT * FROM t1, t1 as t2 WHERE t2.pk BETWEEN t1.i4 AND t1.i4; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL i4_uq NULL NULL NULL 3 +1 SIMPLE t2 eq_ref PRIMARY PRIMARY 4 test.t1.i4 1 Using where +SELECT * FROM t1, t1 as t2 WHERE t2.pk BETWEEN t1.i4 AND t1.i4; +pk i4 pk i4 +EXPLAIN +SELECT * FROM t1, t1 as t2 WHERE t1.i4 BETWEEN t2.pk AND t2.pk; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL i4_uq NULL NULL NULL 3 +1 SIMPLE t2 eq_ref PRIMARY PRIMARY 4 test.t1.i4 1 Using where +SELECT * FROM t1, t1 as t2 WHERE t1.i4 BETWEEN t2.pk AND t2.pk; +pk i4 pk i4 +DROP TABLE t1; End of 5.1 tests diff --git a/mysql-test/t/range.test b/mysql-test/t/range.test index 0ad3d3e8504..6c9320b708a 100644 --- a/mysql-test/t/range.test +++ b/mysql-test/t/range.test @@ -1325,4 +1325,71 @@ SELECT * FROM t1 WHERE 2 NOT BETWEEN c_notkey AND c_key; DROP TABLE t1; +--echo # +--echo # Bug #57030: 'BETWEEN' evaluation is incorrect +--echo # + +# Test some BETWEEN predicates which does *not* follow the +# 'normal' pattern of BETWEEN AND + +CREATE TABLE t1(pk INT PRIMARY KEY, i4 INT); +CREATE UNIQUE INDEX i4_uq ON t1(i4); + +INSERT INTO t1 VALUES (1,10), (2,20), (3,30); + +EXPLAIN +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 10; +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 10; + +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND i4; +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND i4; + +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND i4; +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND i4; + +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND 10; +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND 10; + +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND 10; +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND 10; + +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN 11 AND 11; +SELECT * FROM t1 WHERE 10 BETWEEN 11 AND 11; + +EXPLAIN +SELECT * FROM t1 WHERE 10 BETWEEN 100 AND 0; +SELECT * FROM t1 WHERE 10 BETWEEN 100 AND 0; + +EXPLAIN +SELECT * FROM t1 WHERE i4 BETWEEN 100 AND 0; +SELECT * FROM t1 WHERE i4 BETWEEN 100 AND 0; + +EXPLAIN +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 99999999999999999; +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 99999999999999999; + +EXPLAIN +SELECT * FROM t1 WHERE i4 BETWEEN 999999999999999 AND 30; +SELECT * FROM t1 WHERE i4 BETWEEN 999999999999999 AND 30; + +EXPLAIN +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND '20'; +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND '20'; + +#Should detect the EQ_REF 't2.pk=t1.i4' +EXPLAIN +SELECT * FROM t1, t1 as t2 WHERE t2.pk BETWEEN t1.i4 AND t1.i4; +SELECT * FROM t1, t1 as t2 WHERE t2.pk BETWEEN t1.i4 AND t1.i4; + +EXPLAIN +SELECT * FROM t1, t1 as t2 WHERE t1.i4 BETWEEN t2.pk AND t2.pk; +SELECT * FROM t1, t1 as t2 WHERE t1.i4 BETWEEN t2.pk AND t2.pk; + +DROP TABLE t1; + --echo End of 5.1 tests diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 8cc2ec6a0f8..e080dc9370c 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -3349,26 +3349,7 @@ add_key_field(KEY_FIELD **key_fields,uint and_level, Item_func *cond, eq_func is NEVER true when num_values > 1 */ if (!eq_func) - { - /* - Additional optimization: if we're processing - "t.key BETWEEN c1 AND c1" then proceed as if we were processing - "t.key = c1". - TODO: This is a very limited fix. A more generic fix is possible. - There are 2 options: - A) Make equality propagation code be able to handle BETWEEN - (including cases like t1.key BETWEEN t2.key AND t3.key) - B) Make range optimizer to infer additional "t.key = c" equalities - and use them in equality propagation process (see details in - OptimizerKBAndTodo) - */ - if ((cond->functype() != Item_func::BETWEEN) || - ((Item_func_between*) cond)->negated || - !value[0]->eq(value[1], field->binary())) - return; - eq_func= TRUE; - } - + return; if (field->result_type() == STRING_RESULT) { if ((*value)->result_type() != STRING_RESULT) @@ -3564,9 +3545,65 @@ add_key_fields(JOIN *join, KEY_FIELD **key_fields, uint *and_level, case Item_func::OPTIMIZE_KEY: { Item **values; - // BETWEEN, IN, NE - if (is_local_field (cond_func->key_item()) && - !(cond_func->used_tables() & OUTER_REF_TABLE_BIT)) + /* + Build list of possible keys for 'a BETWEEN low AND high'. + It is handled similar to the equivalent condition + 'a >= low AND a <= high': + */ + if (cond_func->functype() == Item_func::BETWEEN) + { + Item_field *field_item; + bool equal_func= FALSE; + uint num_values= 2; + values= cond_func->arguments(); + + bool binary_cmp= (values[0]->real_item()->type() == Item::FIELD_ITEM) + ? ((Item_field*)values[0]->real_item())->field->binary() + : TRUE; + + /* + Additional optimization: If 'low = high': + Handle as if the condition was "t.key = low". + */ + if (!((Item_func_between*)cond_func)->negated && + values[1]->eq(values[2], binary_cmp)) + { + equal_func= TRUE; + num_values= 1; + } + + /* + Append keys for 'field value[]' if the + condition is of the form:: + ' BETWEEN value[1] AND value[2]' + */ + if (is_local_field (values[0])) + { + field_item= (Item_field *) (values[0]->real_item()); + add_key_equal_fields(key_fields, *and_level, cond_func, + field_item, equal_func, &values[1], + num_values, usable_tables, sargables); + } + /* + Append keys for 'value[0] field' if the + condition is of the form: + 'value[0] BETWEEN field1 AND field2' + */ + for (uint i= 1; i <= num_values; i++) + { + if (is_local_field (values[i])) + { + field_item= (Item_field *) (values[i]->real_item()); + add_key_equal_fields(key_fields, *and_level, cond_func, + field_item, equal_func, values, + 1, usable_tables, sargables); + } + } + } // if ( ... Item_func::BETWEEN) + + // IN, NE + else if (is_local_field (cond_func->key_item()) && + !(cond_func->used_tables() & OUTER_REF_TABLE_BIT)) { values= cond_func->arguments()+1; if (cond_func->functype() == Item_func::NE_FUNC && @@ -3580,21 +3617,6 @@ add_key_fields(JOIN *join, KEY_FIELD **key_fields, uint *and_level, cond_func->argument_count()-1, usable_tables, sargables); } - if (cond_func->functype() == Item_func::BETWEEN) - { - values= cond_func->arguments(); - for (uint i= 1 ; i < cond_func->argument_count() ; i++) - { - Item_field *field_item; - if (is_local_field (cond_func->arguments()[i])) - { - field_item= (Item_field *) (cond_func->arguments()[i]->real_item()); - add_key_equal_fields(key_fields, *and_level, cond_func, - field_item, 0, values, 1, usable_tables, - sargables); - } - } - } break; } case Item_func::OPTIMIZE_OP: From e805a0fd9de24cf1a9a6fb7bd10a336dbcd46fe9 Mon Sep 17 00:00:00 2001 From: Dmitry Lenev Date: Wed, 2 Feb 2011 16:17:48 +0300 Subject: [PATCH 17/20] Fix for bug #58650 "Failing assertion: primary_key_no == -1 || primary_key_no == 0". Attempt to create InnoDB table with non-nullable column of geometry type having an unique key with length 12 on it and with some other candidate key led to server crash due to assertion failure in both non-debug and debug builds. The problem was that such a non-candidate key could have been sorted as the first key in table/.FRM, before any legit candidate keys. This resulted in assertion failure in InnoDB engine which assumes that primary key should either be the first key in table/.FRM or should not exist at all. The reason behind such an incorrect sorting was an wrong value of Create_field::key_length member for geometry field (which was set to its pack_length == 12) which confused code in mysql_prepare_create_table(), so it would skip marking such key as a key with partial segments. This patch fixes the problem by ensuring that this member gets the same value of Create_field::key_length member as for other blob fields (from which geometry field class is inherited), and as result unique keys on geometry fields are correctly marked as having partial segments. mysql-test/include/gis_keys.inc: Added test case for bug #58650 "Failing assertion: primary_key_no == -1 || primary_key_no == 0". mysql-test/r/gis.result: Added test case for bug #58650 "Failing assertion: primary_key_no == -1 || primary_key_no == 0". mysql-test/suite/innodb/r/innodb_gis.result: Added test case for bug #58650 "Failing assertion: primary_key_no == -1 || primary_key_no == 0". mysql-test/suite/innodb_plugin/r/innodb_gis.result: Added test case for bug #58650 "Failing assertion: primary_key_no == -1 || primary_key_no == 0". sql/field.cc: Changed Create_field::create_length_to_internal_length() to correctly set Create_field::key_length member for geometry fields. Similar to the blob types key_length for such fields should be the same as length and not field's packed length (which is always 12 for geometry). As result of this change code handling table creation now always correctly identifies btree/unique keys on geometry fields as partial keys, so such keys can't be erroneously treated as candidate keys and sorted in keys array in .FRM before legit candidate keys. This fixes bug #58650 "Failing assertion: primary_key_no == -1 || primary_key_no == 0" in which incorrect candidate key sorting led to assertion failure in InnoDB code. --- mysql-test/include/gis_keys.inc | 16 ++++++++++++++++ mysql-test/r/gis.result | 12 ++++++++++++ mysql-test/suite/innodb/r/innodb_gis.result | 12 ++++++++++++ .../suite/innodb_plugin/r/innodb_gis.result | 12 ++++++++++++ sql/field.cc | 1 + 5 files changed, 53 insertions(+) diff --git a/mysql-test/include/gis_keys.inc b/mysql-test/include/gis_keys.inc index c75311f062a..ad00c7e1ef9 100644 --- a/mysql-test/include/gis_keys.inc +++ b/mysql-test/include/gis_keys.inc @@ -44,3 +44,19 @@ SELECT COUNT(*) FROM t2 IGNORE INDEX(p) WHERE p=POINTFROMTEXT('POINT(1 2)'); DROP TABLE t1, t2; --echo End of 5.0 tests + + +--echo # +--echo # Test for bug #58650 "Failing assertion: primary_key_no == -1 || +--echo # primary_key_no == 0". +--echo # +--disable_warnings +drop table if exists t1; +--enable_warnings +--echo # The minimal test case. +create table t1 (a int not null, b linestring not null, unique key b (b(12)), unique key a (a)); +drop table t1; +--echo # The original test case. +create table t1 (a int not null, b linestring not null, unique key b (b(12))); +create unique index a on t1(a); +drop table t1; diff --git a/mysql-test/r/gis.result b/mysql-test/r/gis.result index f4aa361ffcf..a9beb9631ae 100644 --- a/mysql-test/r/gis.result +++ b/mysql-test/r/gis.result @@ -960,6 +960,18 @@ COUNT(*) 2 DROP TABLE t1, t2; End of 5.0 tests +# +# Test for bug #58650 "Failing assertion: primary_key_no == -1 || +# primary_key_no == 0". +# +drop table if exists t1; +# The minimal test case. +create table t1 (a int not null, b linestring not null, unique key b (b(12)), unique key a (a)); +drop table t1; +# The original test case. +create table t1 (a int not null, b linestring not null, unique key b (b(12))); +create unique index a on t1(a); +drop table t1; create table `t1` (`col002` point)engine=myisam; insert into t1 values (),(),(); select min(`col002`) from t1 union select `col002` from t1; diff --git a/mysql-test/suite/innodb/r/innodb_gis.result b/mysql-test/suite/innodb/r/innodb_gis.result index c6c775afc9f..a52a1387b6e 100644 --- a/mysql-test/suite/innodb/r/innodb_gis.result +++ b/mysql-test/suite/innodb/r/innodb_gis.result @@ -585,5 +585,17 @@ COUNT(*) 2 DROP TABLE t1, t2; End of 5.0 tests +# +# Test for bug #58650 "Failing assertion: primary_key_no == -1 || +# primary_key_no == 0". +# +drop table if exists t1; +# The minimal test case. +create table t1 (a int not null, b linestring not null, unique key b (b(12)), unique key a (a)); +drop table t1; +# The original test case. +create table t1 (a int not null, b linestring not null, unique key b (b(12))); +create unique index a on t1(a); +drop table t1; create table t1 (g geometry not null, spatial gk(g)) engine=innodb; ERROR HY000: The used table type doesn't support SPATIAL indexes diff --git a/mysql-test/suite/innodb_plugin/r/innodb_gis.result b/mysql-test/suite/innodb_plugin/r/innodb_gis.result index c6c775afc9f..a52a1387b6e 100644 --- a/mysql-test/suite/innodb_plugin/r/innodb_gis.result +++ b/mysql-test/suite/innodb_plugin/r/innodb_gis.result @@ -585,5 +585,17 @@ COUNT(*) 2 DROP TABLE t1, t2; End of 5.0 tests +# +# Test for bug #58650 "Failing assertion: primary_key_no == -1 || +# primary_key_no == 0". +# +drop table if exists t1; +# The minimal test case. +create table t1 (a int not null, b linestring not null, unique key b (b(12)), unique key a (a)); +drop table t1; +# The original test case. +create table t1 (a int not null, b linestring not null, unique key b (b(12))); +create unique index a on t1(a); +drop table t1; create table t1 (g geometry not null, spatial gk(g)) engine=innodb; ERROR HY000: The used table type doesn't support SPATIAL indexes diff --git a/sql/field.cc b/sql/field.cc index 11edd8bfc1a..1ad5e408e07 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -9476,6 +9476,7 @@ void Create_field::create_length_to_internal_length(void) case MYSQL_TYPE_MEDIUM_BLOB: case MYSQL_TYPE_LONG_BLOB: case MYSQL_TYPE_BLOB: + case MYSQL_TYPE_GEOMETRY: case MYSQL_TYPE_VAR_STRING: case MYSQL_TYPE_STRING: case MYSQL_TYPE_VARCHAR: From a70c34bf0f34703fd330f8cb828e48b303c5296a Mon Sep 17 00:00:00 2001 From: Georgi Kodinov Date: Wed, 2 Feb 2011 18:51:35 +0200 Subject: [PATCH 18/20] Fixes for Bug #55755 and Bug #52315 part 2 Bug #55755 : Date STD variable signness breaks server on FreeBSD and OpenBSD * Added a check to configure on the size of time_t * Created a macro to check for a valid time_t that is safe to use with datetime functions and store in TIMESTAMP columns. * Used the macro consistently instead of the ad-hoc checks introduced by 52315 * Fixed compliation warnings on platforms where the size of time_t is smaller than the size of a long (e.g. OpenBSD 4.8 64 amd64). Bug #52315: utc_date() crashes when system time > year 2037 * Added a correct check for the timestamp range instead of just variable size check to SET TIMESTAMP. * Added overflow checking before converting to time_t. * Using a correct localized error message in this case instead of the generic error. * Added a test suite. * fixed the checks so that they check for unsigned time_t as well. Used the checks consistently across the source code. * fixed the original test case to expect the new error code. --- configure.in | 9 +++++++++ include/config-win.h | 5 +++++ include/my_time.h | 13 +++++++++++++ mysql-test/r/func_time.result | 17 +++++++++++++++++ mysql-test/t/func_time.test | 19 +++++++++++++++++++ mysql-test/t/variables.test | 2 +- sql-common/my_time.c | 2 +- sql/mysqld.cc | 14 +++++++------- sql/set_var.cc | 12 ++++++------ sql/sql_class.h | 2 +- 10 files changed, 79 insertions(+), 16 deletions(-) diff --git a/configure.in b/configure.in index fda56554879..523d36afaea 100644 --- a/configure.in +++ b/configure.in @@ -1956,6 +1956,15 @@ dnl MYSQL_CHECK_TIME_T +dnl +dnl check size of time_t +dnl + +AC_CHECK_SIZEOF(time_t, 8) +if test "$ac_cv_sizeof_time_t" -eq 0 +then + AC_MSG_ERROR("MySQL needs a time_t type.") +fi # do we need #pragma interface/#pragma implementation ? # yes if it's gcc 2.x, and not icc pretending to be gcc, and not cygwin diff --git a/include/config-win.h b/include/config-win.h index 84705809d7a..5e37d9e3a20 100644 --- a/include/config-win.h +++ b/include/config-win.h @@ -167,6 +167,11 @@ typedef uint rf_SetTimer; #define SIZEOF_LONG 4 #define SIZEOF_LONG_LONG 8 #define SIZEOF_OFF_T 8 +/* + The size of time_t depends on the compiler. + But it's 8 for all the supported VC versions. +*/ +#define SIZEOF_TIME_T 8 #ifdef _WIN64 #define SIZEOF_CHARP 8 #else diff --git a/include/my_time.h b/include/my_time.h index 014327d6fd8..5603535e0b7 100644 --- a/include/my_time.h +++ b/include/my_time.h @@ -53,6 +53,19 @@ typedef long my_time_t; #define YY_PART_YEAR 70 +/* + check for valid times only if the range of time_t is greater than + the range of my_time_t +*/ +#if SIZEOF_TIME_T > 4 || defined(TIME_T_UNSIGNED) +# define IS_TIME_T_VALID_FOR_TIMESTAMP(x) \ + ((x) <= TIMESTAMP_MAX_VALUE && \ + (x) >= TIMESTAMP_MIN_VALUE) +#else +# define IS_TIME_T_VALID_FOR_TIMESTAMP(x) \ + ((x) >= TIMESTAMP_MIN_VALUE) +#endif + /* Flags to str_to_datetime */ #define TIME_FUZZY_DATE 1 #define TIME_DATETIME_ONLY 2 diff --git a/mysql-test/r/func_time.result b/mysql-test/r/func_time.result index b3505795494..8638fb09e5a 100644 --- a/mysql-test/r/func_time.result +++ b/mysql-test/r/func_time.result @@ -1323,4 +1323,21 @@ SELECT '2008-02-18' + INTERVAL 1 FRAC_SECOND; ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'FRAC_SECOND' at line 1 SELECT '2008-02-18' - INTERVAL 1 FRAC_SECOND; ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'FRAC_SECOND' at line 1 +# +# Bug #52315 part 2 : utc_date() crashes when system time > year 2037 +# +SET TIMESTAMP=-147490000; +SELECT UTC_TIMESTAMP(); +SET TIMESTAMP=2147483648; +SELECT UTC_TIMESTAMP(); +SET TIMESTAMP=2147483646; +SELECT UTC_TIMESTAMP(); +SET TIMESTAMP=2147483647; +SELECT UTC_TIMESTAMP(); +SET TIMESTAMP=0; +SELECT UTC_TIMESTAMP(); +SET TIMESTAMP=-1; +SELECT UTC_TIMESTAMP(); +SET TIMESTAMP=1; +SELECT UTC_TIMESTAMP(); End of 5.0 tests diff --git a/mysql-test/t/func_time.test b/mysql-test/t/func_time.test index 65d8764f2ce..475938b4458 100644 --- a/mysql-test/t/func_time.test +++ b/mysql-test/t/func_time.test @@ -838,4 +838,23 @@ SELECT '2008-02-18' + INTERVAL 1 FRAC_SECOND; --error ER_PARSE_ERROR SELECT '2008-02-18' - INTERVAL 1 FRAC_SECOND; + +--echo # +--echo # Bug #52315 part 2 : utc_date() crashes when system time > year 2037 +--echo # + +--disable_result_log +--error ER_WRONG_VALUE_FOR_VAR +SET TIMESTAMP=-147490000; SELECT UTC_TIMESTAMP(); +--error ER_WRONG_VALUE_FOR_VAR +SET TIMESTAMP=2147483648; SELECT UTC_TIMESTAMP(); +SET TIMESTAMP=2147483646; SELECT UTC_TIMESTAMP(); +SET TIMESTAMP=2147483647; SELECT UTC_TIMESTAMP(); +SET TIMESTAMP=0; SELECT UTC_TIMESTAMP(); +--error ER_WRONG_VALUE_FOR_VAR +SET TIMESTAMP=-1; SELECT UTC_TIMESTAMP(); +SET TIMESTAMP=1; SELECT UTC_TIMESTAMP(); +--enable_result_log + + --echo End of 5.0 tests diff --git a/mysql-test/t/variables.test b/mysql-test/t/variables.test index 73e5bbd9d87..778fb133045 100644 --- a/mysql-test/t/variables.test +++ b/mysql-test/t/variables.test @@ -776,7 +776,7 @@ SET @@myisam_mmap_size= 500M; --echo # Bug #52315: utc_date() crashes when system time > year 2037 --echo # ---error 0, ER_UNKNOWN_ERROR +--error 0, ER_WRONG_VALUE_FOR_VAR SET TIMESTAMP=2*1024*1024*1024; --echo #Should not crash --disable_result_log diff --git a/sql-common/my_time.c b/sql-common/my_time.c index ed1279f7afb..07cb8b25c46 100644 --- a/sql-common/my_time.c +++ b/sql-common/my_time.c @@ -985,7 +985,7 @@ my_system_gmt_sec(const MYSQL_TIME *t_src, long *my_timezone, with unsigned time_t tmp+= shift*86400L might result in a number, larger then TIMESTAMP_MAX_VALUE, so another check will work. */ - if ((tmp < TIMESTAMP_MIN_VALUE) || (tmp > TIMESTAMP_MAX_VALUE)) + if (!IS_TIME_T_VALID_FOR_TIMESTAMP(tmp)) tmp= 0; return (my_time_t) tmp; diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 5f84d15588d..f026bab1c32 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -2836,13 +2836,6 @@ static int init_common_variables(const char *conf_file_name, int argc, max_system_variables.pseudo_thread_id= (ulong)~0; server_start_time= flush_status_time= time((time_t*) 0); - /* TODO: remove this when my_time_t is 64 bit compatible */ - if (server_start_time >= (time_t) MY_TIME_T_MAX) - { - sql_print_error("This MySQL server doesn't support dates later then 2038"); - return 1; - } - if (init_thread_environment()) return 1; mysql_init_variables(); @@ -2882,6 +2875,13 @@ static int init_common_variables(const char *conf_file_name, int argc, mysql_slow_log.init_pthread_objects(); mysql_bin_log.init_pthread_objects(); + /* TODO: remove this when my_time_t is 64 bit compatible */ + if (!IS_TIME_T_VALID_FOR_TIMESTAMP(server_start_time)) + { + sql_print_error("This MySQL server doesn't support dates later then 2038"); + return 1; + } + if (gethostname(glob_hostname,sizeof(glob_hostname)) < 0) { strmake(glob_hostname, STRING_WITH_LEN("localhost")); diff --git a/sql/set_var.cc b/sql/set_var.cc index 9c327504577..fbc7cdbc232 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -2714,14 +2714,14 @@ int set_var_collation_client::update(THD *thd) bool sys_var_timestamp::check(THD *thd, set_var *var) { - time_t val; + longlong val; var->save_result.ulonglong_value= var->value->val_int(); - val= (time_t) var->save_result.ulonglong_value; - if (val < (time_t) MY_TIME_T_MIN || val > (time_t) MY_TIME_T_MAX) + val= (longlong) var->save_result.ulonglong_value; + if (val != 0 && // this is how you set the default value + (val < TIMESTAMP_MIN_VALUE || val > TIMESTAMP_MAX_VALUE)) { - my_message(ER_UNKNOWN_ERROR, - "This version of MySQL doesn't support dates later than 2038", - MYF(0)); + char buf[64]; + my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), "timestamp", llstr(val, buf)); return TRUE; } return FALSE; diff --git a/sql/sql_class.h b/sql/sql_class.h index 2f4af15c02d..70086e2d944 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1710,7 +1710,7 @@ public: /*TODO: this will be obsolete when we have support for 64 bit my_time_t */ inline bool is_valid_time() { - return (start_time < (time_t) MY_TIME_T_MAX); + return (IS_TIME_T_VALID_FOR_TIMESTAMP(start_time)); } inline void insert_id(ulonglong id_arg) { From 0a8419dfdd6d80b6029c354fb0a82683b2fef617 Mon Sep 17 00:00:00 2001 From: Georgi Kodinov Date: Wed, 2 Feb 2011 20:10:57 +0200 Subject: [PATCH 19/20] Bug #52315 part 2 addendum : reset back the timestamp --- mysql-test/r/func_time.result | 1 + mysql-test/t/func_time.test | 3 +++ 2 files changed, 4 insertions(+) diff --git a/mysql-test/r/func_time.result b/mysql-test/r/func_time.result index 8638fb09e5a..0bca06a55e2 100644 --- a/mysql-test/r/func_time.result +++ b/mysql-test/r/func_time.result @@ -1340,4 +1340,5 @@ SET TIMESTAMP=-1; SELECT UTC_TIMESTAMP(); SET TIMESTAMP=1; SELECT UTC_TIMESTAMP(); +SET TIMESTAMP=0; End of 5.0 tests diff --git a/mysql-test/t/func_time.test b/mysql-test/t/func_time.test index 475938b4458..7ec07dd786a 100644 --- a/mysql-test/t/func_time.test +++ b/mysql-test/t/func_time.test @@ -856,5 +856,8 @@ SET TIMESTAMP=-1; SELECT UTC_TIMESTAMP(); SET TIMESTAMP=1; SELECT UTC_TIMESTAMP(); --enable_result_log +#reset back the timestamp value +SET TIMESTAMP=0; + --echo End of 5.0 tests From 378091e434d66dcc7081f991a07db2244dbb8cda Mon Sep 17 00:00:00 2001 From: Dmitry Shulga Date: Fri, 4 Feb 2011 10:47:46 +0600 Subject: [PATCH 20/20] Fixed bug#58026 - massive recursion and crash in regular expression handling. The problem was that parsing of nested regular expression involved recursive calls. Such recursion didn't take into account the amount of available stack space, which ended up leading to stack overflow crashes. mysql-test/t/not_embedded_server.test: Added test for bug#58026. regex/my_regex.h: added pointer to function as last argument of my_regex_init() for check enough memory in stack. regex/regcomp.c: p_ere() was modified: added call to function for check enough memory in stack. Function for check available stack space specified by global variable my_regex_enough_mem_in_stack. This variable set to NULL for embedded mysqld and to a pointer to function check_enough_stack_size otherwise. regex/reginit.c: my_regex_init was modified: pass a pointer to a function for check enough memory in stack space. Reset this pointer to NULL in my_regex_end. sql/mysqld.cc: Added function check_enough_stack_size() for check enough memory in stack. Passed this function as second argument to my_regex_init. For embedded mysqld passed NULL as second argument. --- mysql-test/r/not_embedded_server.result | 4 ++++ mysql-test/t/not_embedded_server.test | 10 ++++++++++ regex/my_regex.h | 4 +++- regex/regcomp.c | 15 +++++++++++++-- regex/reginit.c | 6 +++++- sql/mysqld.cc | 19 ++++++++++++++++++- 6 files changed, 53 insertions(+), 5 deletions(-) diff --git a/mysql-test/r/not_embedded_server.result b/mysql-test/r/not_embedded_server.result index 60c92bd0196..7219e29b2a6 100644 --- a/mysql-test/r/not_embedded_server.result +++ b/mysql-test/r/not_embedded_server.result @@ -4,3 +4,7 @@ select 1; SHOW VARIABLES like 'slave_skip_errors'; Variable_name Value slave_skip_errors OFF +# +# Bug#58026: massive recursion and crash in regular expression handling +# +SELECT '1' RLIKE RPAD('1', 10000, '('); diff --git a/mysql-test/t/not_embedded_server.test b/mysql-test/t/not_embedded_server.test index fa2b659ec57..43cdb977386 100644 --- a/mysql-test/t/not_embedded_server.test +++ b/mysql-test/t/not_embedded_server.test @@ -42,4 +42,14 @@ select 1; SHOW VARIABLES like 'slave_skip_errors'; +--echo # +--echo # Bug#58026: massive recursion and crash in regular expression handling +--echo # + +--disable_result_log +--error ER_STACK_OVERRUN_NEED_MORE +SELECT '1' RLIKE RPAD('1', 10000, '('); +--enable_result_log + + # End of 5.1 tests diff --git a/regex/my_regex.h b/regex/my_regex.h index 0d1cedf5430..30896e29b91 100644 --- a/regex/my_regex.h +++ b/regex/my_regex.h @@ -28,6 +28,7 @@ typedef struct { /* === regcomp.c === */ +typedef int (*my_regex_stack_check_t)(); extern int my_regcomp(my_regex_t *, const char *, int, CHARSET_INFO *charset); #define REG_BASIC 0000 #define REG_EXTENDED 0001 @@ -76,7 +77,8 @@ extern void my_regfree(my_regex_t *); /* === reginit.c === */ -extern void my_regex_init(CHARSET_INFO *cs); /* Should be called for multithread progs */ +/* Should be called for multithread progs */ +extern void my_regex_init(CHARSET_INFO *cs, my_regex_stack_check_t func); extern void my_regex_end(void); /* If one wants a clean end */ #ifdef __cplusplus diff --git a/regex/regcomp.c b/regex/regcomp.c index 81c435ed552..e163a9ba7f4 100644 --- a/regex/regcomp.c +++ b/regex/regcomp.c @@ -31,6 +31,9 @@ struct parse { CHARSET_INFO *charset; /* for ctype things */ }; +/* Check if there is enough stack space for recursion. */ +my_regex_stack_check_t my_regex_enough_mem_in_stack= NULL; + #include "regcomp.ih" static char nuls[10]; /* place to point scanner in event of error */ @@ -117,7 +120,7 @@ CHARSET_INFO *charset; # define GOODFLAGS(f) ((f)&~REG_DUMP) #endif - my_regex_init(charset); /* Init cclass if neaded */ + my_regex_init(charset, NULL); /* Init cclass if neaded */ preg->charset=charset; cflags = GOODFLAGS(cflags); if ((cflags®_EXTENDED) && (cflags®_NOSPEC)) @@ -222,7 +225,15 @@ int stop; /* character this ERE should end at */ /* do a bunch of concatenated expressions */ conc = HERE(); while (MORE() && (c = PEEK()) != '|' && c != stop) - p_ere_exp(p); + { + if (my_regex_enough_mem_in_stack && + my_regex_enough_mem_in_stack()) + { + SETERROR(REG_ESPACE); + return; + } + p_ere_exp(p); + } if(REQUIRE(HERE() != conc, REG_EMPTY)) {}/* require nonempty */ if (!EAT('|')) diff --git a/regex/reginit.c b/regex/reginit.c index 5980de24030..3d2cd64d1e7 100644 --- a/regex/reginit.c +++ b/regex/reginit.c @@ -4,10 +4,12 @@ #include #include #include "cclass.h" +#include "my_regex.h" static my_bool regex_inited=0; +extern my_regex_stack_check_t my_regex_enough_mem_in_stack; -void my_regex_init(CHARSET_INFO *cs) +void my_regex_init(CHARSET_INFO *cs, my_regex_stack_check_t func) { char buff[CCLASS_LAST][256]; int count[CCLASS_LAST]; @@ -16,6 +18,7 @@ void my_regex_init(CHARSET_INFO *cs) if (!regex_inited) { regex_inited=1; + my_regex_enough_mem_in_stack= func; bzero((uchar*) &count,sizeof(count)); for (i=1 ; i<= 255; i++) @@ -74,6 +77,7 @@ void my_regex_end() int i; for (i=0; i < CCLASS_LAST ; i++) free((char*) cclasses[i].chars); + my_regex_enough_mem_in_stack= NULL; regex_inited=0; } } diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 54f93cccd5d..694d64c81df 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -3034,6 +3034,19 @@ sizeof(load_default_groups)/sizeof(load_default_groups[0]); #endif +#ifndef EMBEDDED_LIBRARY +static +int +check_enough_stack_size() +{ + uchar stack_top; + + return check_stack_overrun(current_thd, STACK_MIN_SIZE, + &stack_top); +} +#endif + + /** Initialize one of the global date/time format variables. @@ -3415,7 +3428,11 @@ static int init_common_variables(const char *conf_file_name, int argc, #endif mysys_uses_curses=0; #ifdef USE_REGEX - my_regex_init(&my_charset_latin1); +#ifndef EMBEDDED_LIBRARY + my_regex_init(&my_charset_latin1, check_enough_stack_size); +#else + my_regex_init(&my_charset_latin1, NULL); +#endif #endif /* Process a comma-separated character set list and choose