maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #04251
Fwd: [Commits] Rev 3104: Fix bug lp:809266 in file:///home/tsk/mprog/src/5.3-mwl89/
Igor,
Could you please review this fix for bug lp:809266.
Timour
------------------------------------------------------------
revno: 3104
revision-id: timour@xxxxxxxxxxxx-20110713211507-j9v80yk2tzae8tq2
parent: timour@xxxxxxxxxxxx-20110713141146-339e3xwz17hogqsb
fixes bug(s): https://launchpad.net/bugs/809266
committer: timour@xxxxxxxxxxxx
branch nick: 5.3-mwl89
timestamp: Thu 2011-07-14 00:15:07 +0300
message:
Fix bug lp:809266
Analysis:
This is a bug in MWL#68, where it was incorrectly assumed
that if there is a match in the only non-null key, then
if there is a covering NULL row on all remaining NULL-able
columns there is a partial match. However, this is not the case,
because even if there is such a null-only sub-row, it is not
guaranteed to be part of the matched sub-row. The matched sub-row
and the NULL-only sub-row may be parts of different rows.
In fact there are two cases:
- there is a complete row with only NULL values, and
- all nullable columns contain only NULL values.
These two cases were incorrectly mixed up in the class member
subselect_partial_match_engine::covering_null_row_width.
Solution:
The solution is to:
- split covering_null_row_width into two members:
has_covering_null_row, and has_covering_null_columns, and
- take into account each state during initialization and
execution.
=== modified file 'mysql-test/r/subselect_partial_match.result'
--- a/mysql-test/r/subselect_partial_match.result 2011-07-13 14:09:09 +0000
+++ b/mysql-test/r/subselect_partial_match.result 2011-07-13 21:15:07 +0000
@@ -99,4 +99,50 @@ FROM t3 LEFT JOIN t2 ON t3.a = t2.a);
d
r
drop table t1, t2, t3;
+#
+# LP BUG#809266 Diverging results with partial_match_rowid_merge=on
+#
+CREATE TABLE t1 (c int) ;
+INSERT INTO t1 VALUES (0),(0);
+CREATE TABLE t2 (a int, b int) ;
+INSERT INTO t2 VALUES (6,3), (9,NULL);
+set @@optimizer_switch='materialization=on,partial_match_rowid_merge=on,partial_match_table_scan=off,in_to_exists=off';
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+id select_type table type possible_keys key key_len ref rows Extra
+1 PRIMARY t1 ALL NULL NULL NULL NULL 2
+2 SUBQUERY t2 ALL NULL NULL NULL NULL 2
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+c
+0
+0
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+id select_type table type possible_keys key key_len ref rows Extra
+1 PRIMARY t1 ALL NULL NULL NULL NULL 2
+2 SUBQUERY t2 ALL NULL NULL NULL NULL 2
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+c
+0
+0
+set @@optimizer_switch='materialization=off,in_to_exists=on';
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+id select_type table type possible_keys key key_len ref rows Extra
+1 PRIMARY t1 ALL NULL NULL NULL NULL 2
+2 DEPENDENT SUBQUERY t2 ALL NULL NULL NULL NULL 2 Using where
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+c
+0
+0
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+id select_type table type possible_keys key key_len ref rows Extra
+1 PRIMARY t1 ALL NULL NULL NULL NULL 2
+2 DEPENDENT SUBQUERY t2 ALL NULL NULL NULL NULL 2 Using where
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+c
+0
+0
+drop table t1, t2;
set @@optimizer_switch=@save_optimizer_switch;
=== modified file 'mysql-test/t/subselect_partial_match.test'
--- a/mysql-test/t/subselect_partial_match.test 2011-07-13 14:09:09 +0000
+++ b/mysql-test/t/subselect_partial_match.test 2011-07-13 21:15:07 +0000
@@ -109,4 +109,36 @@ WHERE (t1.d , t1.d) NOT IN (
drop table t1, t2, t3;
+--echo #
+--echo # LP BUG#809266 Diverging results with partial_match_rowid_merge=on
+--echo #
+
+CREATE TABLE t1 (c int) ;
+INSERT INTO t1 VALUES (0),(0);
+
+CREATE TABLE t2 (a int, b int) ;
+INSERT INTO t2 VALUES (6,3), (9,NULL);
+
+set @@optimizer_switch='materialization=on,partial_match_rowid_merge=on,partial_match_table_scan=off,in_to_exists=off';
+
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+
+set @@optimizer_switch='materialization=off,in_to_exists=on';
+
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+
+drop table t1, t2;
+
set @@optimizer_switch=@save_optimizer_switch;
=== modified file 'sql/item_subselect.cc'
--- a/sql/item_subselect.cc 2011-07-13 14:09:09 +0000
+++ b/sql/item_subselect.cc 2011-07-13 21:15:07 +0000
@@ -4572,29 +4572,36 @@ int subselect_hash_sj_engine::exec()
if (strategy == PARTIAL_MATCH)
{
uint count_pm_keys; /* Total number of keys needed for partial matching. */
- MY_BITMAP *nn_key_parts; /* The key parts of the only non-NULL index. */
- uint covering_null_row_width;
+ MY_BITMAP *nn_key_parts= NULL; /* Key parts of the only non-NULL index. */
+ uint count_non_null_columns= 0; /* Number of columns in nn_key_parts. */
+ bool has_covering_null_row;
+ bool has_covering_null_columns;
select_materialize_with_stats *result_sink=
(select_materialize_with_stats *) result;
+ uint field_count= tmp_table->s->fields;
- nn_key_parts= (count_partial_match_columns < tmp_table->s->fields) ?
- &non_null_key_parts : NULL;
-
- if (result_sink->get_max_nulls_in_row() ==
- tmp_table->s->fields -
- (nn_key_parts ? bitmap_bits_set(nn_key_parts) : 0))
- covering_null_row_width= result_sink->get_max_nulls_in_row();
- else
- covering_null_row_width= 0;
+ if (count_partial_match_columns < field_count)
+ {
+ nn_key_parts= &non_null_key_parts;
+ count_non_null_columns= bitmap_bits_set(nn_key_parts);
+ }
+ has_covering_null_row= (result_sink->get_max_nulls_in_row() == field_count);
+ has_covering_null_columns= (count_non_null_columns +
+ count_null_only_columns == field_count);
- if (covering_null_row_width)
- count_pm_keys= nn_key_parts ? 1 : 0;
+ if (has_covering_null_row)
+ {
+ DBUG_ASSERT(count_partial_match_columns = field_count);
+ count_pm_keys= 0;
+ }
+ else if (has_covering_null_columns)
+ count_pm_keys= 1;
else
count_pm_keys= count_partial_match_columns - count_null_only_columns +
- (nn_key_parts ? 1 : 0);
+ (nn_key_parts ? 1 : 0);
choose_partial_match_strategy(test(nn_key_parts),
- test(covering_null_row_width),
+ has_covering_null_row,
&partial_match_key_parts);
DBUG_ASSERT(strategy == PARTIAL_MATCH_MERGE ||
strategy == PARTIAL_MATCH_SCAN);
@@ -4604,7 +4611,8 @@ int subselect_hash_sj_engine::exec()
new subselect_rowid_merge_engine(thd, (subselect_uniquesubquery_engine*)
lookup_engine, tmp_table,
count_pm_keys,
- covering_null_row_width,
+ has_covering_null_row,
+ has_covering_null_columns,
item, result,
semi_join_conds->argument_list());
if (!pm_engine ||
@@ -4629,7 +4637,8 @@ int subselect_hash_sj_engine::exec()
lookup_engine, tmp_table,
item, result,
semi_join_conds->argument_list(),
- covering_null_row_width)))
+ has_covering_null_row,
+ has_covering_null_columns)))
{
/* This is an irrecoverable error. */
res= 1;
@@ -5065,11 +5074,13 @@ subselect_partial_match_engine::subselec
TABLE *tmp_table_arg, Item_subselect *item_arg,
select_result_interceptor *result_arg,
List<Item> *equi_join_conds_arg,
- uint covering_null_row_width_arg)
+ bool has_covering_null_row_arg,
+ bool has_covering_null_columns_arg)
:subselect_engine(thd_arg, item_arg, result_arg),
tmp_table(tmp_table_arg), lookup_engine(engine_arg),
equi_join_conds(equi_join_conds_arg),
- covering_null_row_width(covering_null_row_width_arg)
+ has_covering_null_row(has_covering_null_row_arg),
+ has_covering_null_columns(has_covering_null_columns_arg)
{}
@@ -5107,7 +5118,7 @@ int subselect_partial_match_engine::exec
}
}
- if (covering_null_row_width == tmp_table->s->fields)
+ if (has_covering_null_row)
{
/*
If there is a NULL-only row that coveres all columns the result of IN
@@ -5187,19 +5198,25 @@ subselect_rowid_merge_engine::init(MY_BI
rownum_t cur_rownum= 0;
select_materialize_with_stats *result_sink=
(select_materialize_with_stats *) result;
- uint cur_keyid;
+ uint cur_keyid= 0;
Item_in_subselect *item_in= (Item_in_subselect*) item;
int error;
if (merge_keys_count == 0)
{
+ DBUG_ASSERT(bitmap_bits_set(partial_match_key_parts) == 0 ||
+ has_covering_null_row);
/* There is nothing to initialize, we will only do regular lookups. */
return FALSE;
}
- DBUG_ASSERT(!covering_null_row_width || (covering_null_row_width &&
- merge_keys_count == 1 &&
- non_null_key_parts));
+ /*
+ If all nullable columns contain only NULLs, there must be one index
+ over all non-null columns.
+ */
+ DBUG_ASSERT(!has_covering_null_columns ||
+ (has_covering_null_columns &&
+ merge_keys_count == 1 && non_null_key_parts));
/*
Allocate buffers to hold the merged keys and the mapping between rowids and
row numbers.
@@ -5213,23 +5230,20 @@ subselect_rowid_merge_engine::init(MY_BI
/* Create the only non-NULL key if there is any. */
if (non_null_key_parts)
{
- non_null_key= new Ordered_key(0, tmp_table, item_in->left_expr,
+ non_null_key= new Ordered_key(cur_keyid, tmp_table, item_in->left_expr,
0, 0, 0, row_num_to_rowid);
if (non_null_key->init(non_null_key_parts))
return TRUE;
- merge_keys[0]= non_null_key;
- merge_keys[0]->first();
- cur_keyid= 1;
+ merge_keys[cur_keyid]= non_null_key;
+ merge_keys[cur_keyid]->first();
+ ++cur_keyid;
}
- else
- cur_keyid= 0;
/*
- If there is a covering NULL row, the only key that is needed is the
- only non-NULL key that is already created above. We create keys on
- NULL-able columns only if there is no covering NULL row.
+ If all nullable columns contain NULLs, the only key that is needed is the
+ only non-NULL key that is already created above.
*/
- if (!covering_null_row_width)
+ if (!has_covering_null_columns)
{
if (bitmap_init_memroot(&matching_keys, merge_keys_count, thd->mem_root) ||
bitmap_init_memroot(&matching_outer_cols, merge_keys_count, thd->mem_root))
@@ -5460,11 +5474,10 @@ bool subselect_rowid_merge_engine::parti
}
/*
- If there is a NULL (sub)row that covers all NULL-able columns,
- then there is a guranteed partial match, and we don't need to search
- for the matching row.
- */
- if (covering_null_row_width)
+ If all nullable columns contain only NULLs, then there is a guranteed
+ partial match, and we don't need to search for a matching row.
+ */
+ if (has_covering_null_columns)
{
res= TRUE;
goto end;
@@ -5581,10 +5594,12 @@ subselect_table_scan_engine::subselect_t
Item_subselect *item_arg,
select_result_interceptor *result_arg,
List<Item> *equi_join_conds_arg,
- uint covering_null_row_width_arg)
+ bool has_covering_null_row_arg,
+ bool has_covering_null_columns_arg)
:subselect_partial_match_engine(thd_arg, engine_arg, tmp_table_arg, item_arg,
result_arg, equi_join_conds_arg,
- covering_null_row_width_arg)
+ has_covering_null_row_arg,
+ has_covering_null_columns_arg)
{}
@@ -5623,10 +5638,6 @@ bool subselect_table_scan_engine::partia
tmp_table->file->extra_opt(HA_EXTRA_CACHE,
current_thd->variables.read_buff_size);
- /*
- TIMOUR:
- scan_table() also calls "table->null_row= 0;", why, do we need it?
- */
for (;;)
{
error= tmp_table->file->ha_rnd_next(tmp_table->record[0]);
=== modified file 'sql/item_subselect.h'
--- a/sql/item_subselect.h 2011-07-13 14:09:09 +0000
+++ b/sql/item_subselect.h 2011-07-13 21:15:07 +0000
@@ -1131,11 +1131,18 @@ class subselect_partial_match_engine : p
/* A list of equalities between each pair of IN operands. */
List<Item> *equi_join_conds;
/*
- If there is a row, such that all its NULL-able components are NULL, this
- member is set to the number of covered columns. If there is no covering
- row, then this is 0.
+ True if there is an all NULL row in tmp_table. If so, then if there is
+ no complete match, there is a guaranteed partial match.
*/
- uint covering_null_row_width;
+ bool has_covering_null_row;
+
+ /*
+ True if all nullable columns of tmp_table consist of only NULL values.
+ If so, then if there is a match in the non-null columns, there is a
+ guaranteed partial match.
+ */
+ bool has_covering_null_columns;
+
protected:
virtual bool partial_match()= 0;
public:
@@ -1144,7 +1151,8 @@ class subselect_partial_match_engine : p
TABLE *tmp_table_arg, Item_subselect *item_arg,
select_result_interceptor *result_arg,
List<Item> *equi_join_conds_arg,
- uint covering_null_row_width_arg);
+ bool has_covering_null_row_arg,
+ bool has_covering_null_columns_arg);
int prepare() { return 0; }
int exec();
void fix_length_and_dec(Item_cache**) {}
@@ -1232,13 +1240,15 @@ class subselect_rowid_merge_engine: publ
subselect_rowid_merge_engine(THD *thd_arg,
subselect_uniquesubquery_engine *engine_arg,
TABLE *tmp_table_arg, uint merge_keys_count_arg,
- uint covering_null_row_width_arg,
+ bool has_covering_null_row_arg,
+ bool has_covering_null_columns_arg,
Item_subselect *item_arg,
select_result_interceptor *result_arg,
List<Item> *equi_join_conds_arg)
:subselect_partial_match_engine(thd_arg, engine_arg, tmp_table_arg,
item_arg, result_arg, equi_join_conds_arg,
- covering_null_row_width_arg),
+ has_covering_null_row_arg,
+ has_covering_null_columns_arg),
merge_keys_count(merge_keys_count_arg), non_null_key(NULL)
{}
~subselect_rowid_merge_engine();
@@ -1258,7 +1268,8 @@ class subselect_table_scan_engine: publi
TABLE *tmp_table_arg, Item_subselect *item_arg,
select_result_interceptor *result_arg,
List<Item> *equi_join_conds_arg,
- uint covering_null_row_width_arg);
+ bool has_covering_null_row_arg,
+ bool has_covering_null_columns_arg);
void cleanup();
virtual enum_engine_type engine_type() { return TABLE_SCAN_ENGINE; }
};
_______________________________________________
commits mailing list
commits@xxxxxxxxxxx
https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
Follow ups