Below is the list of changes that have just been committed into a local
5.0 repository of psergey. When psergey does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
ChangeSet@stripped, 2007-01-11 12:15:08+03:00, sergefp@stripped +7 -0
BUG#24127: post-review fixes, batch 3 (more to come)
mysql-test/t/subselect3.test@stripped, 2007-01-11 12:15:05+03:00, sergefp@stripped +13 -10
BUG#24127: post-review fixes, batch 3 (more to come)
sql/item_cmpfunc.h@stripped, 2007-01-11 12:15:05+03:00, sergefp@stripped +1 -2
BUG#24127: post-review fixes, batch 3 (more to come)
sql/item_subselect.cc@stripped, 2007-01-11 12:15:05+03:00, sergefp@stripped +30 -41
BUG#24127: post-review fixes, batch 3 (more to come)
sql/item_subselect.h@stripped, 2007-01-11 12:15:05+03:00, sergefp@stripped +8 -10
BUG#24127: post-review fixes, batch 3 (more to come)
sql/sql_lex.h@stripped, 2007-01-11 12:15:05+03:00, sergefp@stripped +1 -1
BUG#24127: post-review fixes, batch 3 (more to come)
sql/sql_select.cc@stripped, 2007-01-11 12:15:05+03:00, sergefp@stripped +40 -16
BUG#24127: post-review fixes, batch 3 (more to come)
sql/sql_select.h@stripped, 2007-01-11 12:15:05+03:00, sergefp@stripped +4 -2
BUG#24127: post-review fixes, batch 3 (more to come)
# This is a BitKeeper patch. What follows are the unified diffs for the
# set of deltas contained in the patch. The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User: sergefp
# Host: pylon.mylan
# Root: /home/psergey/mysql-5.0-bug8804-r10
--- 1.135/sql/item_cmpfunc.h 2007-01-11 12:15:13 +03:00
+++ 1.136/sql/item_cmpfunc.h 2007-01-11 12:15:13 +03:00
@@ -305,9 +305,8 @@
class Item_func_trig_cond: public Item_bool_func
{
- bool *trig_var;
public:
- int marker; /* An opaque value used by module that created this trigger */
+ bool *trig_var;
Item_func_trig_cond(Item *a, bool *f) : Item_bool_func(a) { trig_var= f; }
longlong val_int() { return *trig_var ? args[0]->val_int() : 1; }
enum Functype functype() const { return TRIG_COND_FUNC; };
--- 1.232/sql/sql_lex.h 2007-01-11 12:15:13 +03:00
+++ 1.233/sql/sql_lex.h 2007-01-11 12:15:13 +03:00
@@ -470,7 +470,7 @@
void set_thd(THD *thd_arg) { thd= thd_arg; }
friend void lex_start(THD *thd, uchar *buf, uint length);
- friend int subselect_union_engine::exec(bool*);
+ friend int subselect_union_engine::exec();
List<Item> *get_unit_column_types();
};
--- 1.472/sql/sql_select.cc 2007-01-11 12:15:13 +03:00
+++ 1.473/sql/sql_select.cc 2007-01-11 12:15:13 +03:00
@@ -515,6 +515,7 @@
/*
Remove the predicates pushed down into the subquery
+
SYNOPSIS
JOIN::remove_subq_pushed_predicates()
where IN Must be NULL
@@ -527,13 +528,18 @@
We can remove the equalities that will be guaranteed to be true by the
fact that subquery engine will be using index lookup.
+
If the subquery compares scalar values, we can remove the condition that
- was wrapped into trig_cond. If the subquery compares row values, we need
- to keep the wrapped equalities in the WHERE clause: when some parts of
- the left tuple are NULLs and some aren't, we'll use full table scan and
- will rely on the equalities for non-NULL tuple parts to be guaranteed to
- be true.
+ was wrapped into trig_cond.
+
+ If the subquery compares row values, we need to keep the wrapped
+ equalities in the WHERE clause: when some parts of the left tuple are
+ NULLs and some aren't, we'll use full table scan and will rely on the
+ equalities for non-NULL tuple parts to be guaranteed to be true.
+
+ psergey-todo: ^ clarify ^.
*/
+
void JOIN::remove_subq_pushed_predicates(Item **where)
{
if (conds->type() == Item::FUNC_ITEM &&
@@ -553,6 +559,25 @@
}
+/*
+ Index lookup-based subquery: save some flags for EXPLAIN output
+
+ SYNOPSIS
+ save_index_subquery_explain_info()
+ join_tab Subquery's join tab (there is only one as index lookup is
+ only used for subqueries that are single-table SELECTs)
+ where Subquery's WHERE clause
+
+ DESCRIPTION
+ For index lookup-based subquery (i.e. one executed with
+ subselect_uniquesubquery_engine or subselect_indexsubquery_engine),
+ check its EXPLAIN output row should contain
+ "Using index" (TAB_INFO_FULL_SCAN_ON_NULL)
+ "Using Where" (TAB_INFO_USING_WHERE)
+ "Full scan on NULL key" (TAB_INFO_FULL_SCAN_ON_NULL)
+ and set appropriate flags in join_tab->packed_info.
+*/
+
static void save_index_subquery_explain_info(JOIN_TAB *join_tab, Item* where)
{
join_tab->packed_info= TAB_INFO_HAVE_VALUE;
@@ -562,7 +587,7 @@
join_tab->packed_info |= TAB_INFO_USING_WHERE;
for (uint i = 0; i < join_tab->ref.key_parts; i++)
{
- if (join_tab->ref.outer_ref_cols[i] != UINT_MAX)
+ if (join_tab->ref.cond_guards[i])
{
join_tab->packed_info |= TAB_INFO_FULL_SCAN_ON_NULL;
break;
@@ -2527,7 +2552,7 @@
when val IS NULL.
*/
bool null_rejecting;
- uint outer_ref_col; /* See KEYUSE::outer_ref_col */
+ bool *cond_guard; /* See KEYUSE::cond_guard */
} KEY_FIELD;
/* Values in optimize */
@@ -2826,7 +2851,7 @@
cond->functype() == Item_func::MULT_EQUAL_FUNC) &&
((*value)->type() == Item::FIELD_ITEM) &&
((Item_field*)*value)->field->maybe_null());
- (*key_fields)->outer_ref_col= UINT_MAX;
+ (*key_fields)->cond_guard= NULL;
(*key_fields)++;
}
@@ -2924,13 +2949,12 @@
/*
Subquery optimization: check if the encountered condition is one
- added by condition push down into subquery.
+ added by condition push down into subquery. (psergey-todo: better comment)
*/
{
if (cond->type() == Item::FUNC_ITEM &&
((Item_func*)cond)->functype() == Item_func::TRIG_COND_FUNC)
{
- int col= ((Item_func_trig_cond*)cond)->marker;
cond= ((Item_func*)cond)->arguments()[0];
if (!join->group_list && !join->order &&
join->unit->item &&
@@ -2942,7 +2966,7 @@
sargables);
// Indicate that this ref access candidate is for subquery lookup:
for (; save != *key_fields; save++)
- save->outer_ref_col= col;
+ save->cond_guard= (((Item_func_trig_cond*)cond)->trig_var);
}
return;
}
@@ -3122,7 +3146,7 @@
keyuse.used_tables=key_field->val->used_tables();
keyuse.optimize= key_field->optimize & KEY_OPTIMIZE_REF_OR_NULL;
keyuse.null_rejecting= key_field->null_rejecting;
- keyuse.outer_ref_col= key_field->outer_ref_col;
+ keyuse.cond_guard= key_field->cond_guard;
VOID(insert_dynamic(keyuse_array,(gptr) &keyuse));
}
}
@@ -4964,7 +4988,7 @@
!(j->ref.key_copy= (store_key**) thd->alloc((sizeof(store_key*) *
(keyparts+1)))) ||
!(j->ref.items= (Item**) thd->alloc(sizeof(Item*)*keyparts)) ||
- !(j->ref.outer_ref_cols= (uint*) thd->alloc(sizeof(uint*)*keyparts)))
+ !(j->ref.cond_guards= (bool**) thd->alloc(sizeof(uint*)*keyparts)))
{
DBUG_RETURN(TRUE);
}
@@ -4980,7 +5004,7 @@
{
j->ref.items[0]=((Item_func*)(keyuse->val))->key_item();
/* Predicates pushed down into subquery can't be used FT access */
- j->ref.outer_ref_cols[0]= UINT_MAX;
+ j->ref.cond_guards[0]= NULL;
if (keyuse->used_tables)
DBUG_RETURN(TRUE); // not supported yet. SerG
@@ -4997,7 +5021,7 @@
uint maybe_null= test(keyinfo->key_part[i].null_bit);
j->ref.items[i]=keyuse->val; // Save for cond removal
- j->ref.outer_ref_cols[i]= keyuse->outer_ref_col;
+ j->ref.cond_guards[i]= keyuse->cond_guard;
if (keyuse->null_rejecting)
j->ref.null_rejecting |= 1 << i;
keyuse_uses_no_tables= keyuse_uses_no_tables && !keyuse->used_tables;
@@ -14824,7 +14848,7 @@
for (uint part= 0; part < tab->ref.key_parts; part++)
{
- if (tab->ref.outer_ref_cols[part] != UINT_MAX)
+ if (tab->ref.cond_guards[part])
{
extra.append(STRING_WITH_LEN("; Full scan on NULL key"));
break;
--- 1.113/sql/sql_select.h 2007-01-11 12:15:13 +03:00
+++ 1.114/sql/sql_select.h 2007-01-11 12:15:13 +03:00
@@ -42,9 +42,10 @@
a subselect. The number is the number of column, e.g. for
(a, b) IN (SELECT x, keypart2 ...)
we'll get a KEYUSE created for "keypart2=b" with outer_ref_col == 1.
+ psergey-todo: fix comment
UINT_MAX - Otherwise
*/
- uint outer_ref_col;
+ bool *cond_guard;
} KEYUSE;
class store_key;
@@ -63,8 +64,9 @@
Array of numbers of pushed-down subq predicates that were used to
construct equalities on keypart #i. If equality on keypart #i was not
constructed from pushed-down predicate, outer_ref_col[i]==UINT_MAX.
+ psergey-todo: change-comment!
*/
- uint *outer_ref_cols;
+ bool **cond_guards;
/*
(null_rejecting & (1<<i)) means the condition is '=' and no matching
rows will be produced if items[i] IS NULL (see add_not_null_conds())
--- 1.144/sql/item_subselect.cc 2007-01-11 12:15:13 +03:00
+++ 1.145/sql/item_subselect.cc 2007-01-11 12:15:13 +03:00
@@ -192,16 +192,16 @@
return res;
}
-bool Item_subselect::exec(bool *enabled_conds)
+bool Item_subselect::exec()
{
int res;
- res= engine->exec(enabled_conds);
+ res= engine->exec();
if (engine_changed)
{
engine_changed= 0;
- return exec(enabled_conds);
+ return exec();
}
return (res);
}
@@ -450,13 +450,13 @@
void Item_singlerow_subselect::bring_value()
{
- exec(NULL);
+ exec();
}
double Item_singlerow_subselect::val_real()
{
DBUG_ASSERT(fixed == 1);
- if (!exec(NULL) && !value->null_value)
+ if (!exec() && !value->null_value)
{
null_value= 0;
return value->val_real();
@@ -471,7 +471,7 @@
longlong Item_singlerow_subselect::val_int()
{
DBUG_ASSERT(fixed == 1);
- if (!exec(NULL) && !value->null_value)
+ if (!exec() && !value->null_value)
{
null_value= 0;
return value->val_int();
@@ -485,7 +485,7 @@
String *Item_singlerow_subselect::val_str(String *str)
{
- if (!exec(NULL) && !value->null_value)
+ if (!exec() && !value->null_value)
{
null_value= 0;
return value->val_str(str);
@@ -500,7 +500,7 @@
my_decimal *Item_singlerow_subselect::val_decimal(my_decimal *decimal_value)
{
- if (!exec(NULL) && !value->null_value)
+ if (!exec() && !value->null_value)
{
null_value= 0;
return value->val_decimal(decimal_value);
@@ -515,7 +515,7 @@
bool Item_singlerow_subselect::val_bool()
{
- if (!exec(NULL) && !value->null_value)
+ if (!exec() && !value->null_value)
{
null_value= 0;
return value->val_bool();
@@ -613,7 +613,7 @@
double Item_exists_subselect::val_real()
{
DBUG_ASSERT(fixed == 1);
- if (exec(NULL))
+ if (exec())
{
reset();
return 0;
@@ -624,7 +624,7 @@
longlong Item_exists_subselect::val_int()
{
DBUG_ASSERT(fixed == 1);
- if (exec(NULL))
+ if (exec())
{
reset();
return 0;
@@ -635,7 +635,7 @@
String *Item_exists_subselect::val_str(String *str)
{
DBUG_ASSERT(fixed == 1);
- if (exec(NULL))
+ if (exec())
{
reset();
return 0;
@@ -648,7 +648,7 @@
my_decimal *Item_exists_subselect::val_decimal(my_decimal *decimal_value)
{
DBUG_ASSERT(fixed == 1);
- if (exec(NULL))
+ if (exec())
{
reset();
return 0;
@@ -661,7 +661,7 @@
bool Item_exists_subselect::val_bool()
{
DBUG_ASSERT(fixed == 1);
- if (exec(NULL))
+ if (exec())
{
reset();
return 0;
@@ -679,7 +679,7 @@
DBUG_ASSERT(0);
DBUG_ASSERT(fixed == 1);
null_value= 0;
- if (exec(enable_pushed_conds_vec))
+ if (exec())
{
reset();
null_value= 1;
@@ -700,7 +700,7 @@
DBUG_ASSERT(0);
DBUG_ASSERT(fixed == 1);
null_value= 0;
- if (exec(enable_pushed_conds_vec))
+ if (exec())
{
reset();
null_value= 1;
@@ -721,7 +721,7 @@
DBUG_ASSERT(0);
DBUG_ASSERT(fixed == 1);
null_value= 0;
- if (exec(enable_pushed_conds_vec))
+ if (exec())
{
reset();
null_value= 1;
@@ -741,7 +741,7 @@
{
DBUG_ASSERT(fixed == 1);
null_value= 0;
- if (exec(enable_pushed_conds_vec))
+ if (exec())
{
reset();
null_value= 1;
@@ -761,7 +761,7 @@
DBUG_ASSERT(0);
null_value= 0;
DBUG_ASSERT(fixed == 1);
- if (exec(enable_pushed_conds_vec))
+ if (exec())
{
reset();
null_value= 1;
@@ -984,7 +984,6 @@
within a trig_cond.
*/
item= new Item_func_trig_cond(item, &enable_pushed_conds);
- ((Item_func_trig_cond*)item)->marker= 0;
}
/*
@@ -1027,7 +1026,6 @@
if (!(having= new Item_func_trig_cond(having,
&enable_pushed_conds)))
DBUG_RETURN(RES_ERROR);
- ((Item_func_trig_cond*)having)->marker= 0;
}
/*
Item_is_not_null_test can't be changed during fix_fields()
@@ -1050,14 +1048,13 @@
new Item_func_isnull(orig_item));
}
/*
- If we may encounter NULL IN (SELECT ...) and care between NULL and
- FALSE, wrap it in a trig_cond
+ If we may encounter NULL IN (SELECT ...) and care whether subquery
+ result is NULL or FALSE, wrap condition in a trig_cond.
*/
if (!abort_on_null && left_expr->maybe_null)
{
if (!(item= new Item_func_trig_cond(item, &enable_pushed_conds)))
DBUG_RETURN(RES_ERROR);
- ((Item_func_trig_cond*)item)->marker= 0;
}
/*
TODO: figure out why the following is done here in
@@ -1101,7 +1098,6 @@
if (!(new_having= new Item_func_trig_cond(new_having,
&enable_pushed_conds)))
DBUG_RETURN(RES_ERROR);
- ((Item_func_trig_cond*)new_having)->marker= 0;
}
new_having->name= (char*)in_having_cond;
select_lex->having= join->having= new_having;
@@ -1236,7 +1232,6 @@
if (!(col_item= new Item_func_trig_cond(col_item,
enable_pushed_conds_vec + i)))
DBUG_RETURN(RES_ERROR);
- ((Item_func_trig_cond*)col_item)->marker= i;
}
having_item= and_items(having_item, col_item);
@@ -1253,7 +1248,6 @@
new Item_func_trig_cond(item_nnull_test,
enable_pushed_conds_vec + i)))
DBUG_RETURN(RES_ERROR);
- ((Item_func_trig_cond*)item_nnull_test)->marker= i;
}
item_having_part2= and_items(item_having_part2, item_nnull_test);
item_having_part2->top_level_item();
@@ -1331,13 +1325,10 @@
if (!(item= new Item_func_trig_cond(item,
enable_pushed_conds_vec + i)))
DBUG_RETURN(RES_ERROR);
- ((Item_func_trig_cond*)item)->marker= i;
-
if (!(having_col_item=
new Item_func_trig_cond(having_col_item,
enable_pushed_conds_vec + i)))
DBUG_RETURN(RES_ERROR);
- ((Item_func_trig_cond*)having_col_item)->marker= i;
}
having_item= and_items(having_item, having_col_item);
}
@@ -1744,7 +1735,7 @@
int join_read_always_key_or_null(JOIN_TAB *tab);
int join_read_next_same_or_null(READ_RECORD *info);
-int subselect_single_select_engine::exec(bool *enabled_conds)
+int subselect_single_select_engine::exec()
{
DBUG_ENTER("subselect_single_select_engine::exec");
char const *save_where= thd->where;
@@ -1785,7 +1776,7 @@
bool have_changed_access= FALSE;
JOIN_TAB *changed_tabs[MAX_TABLES];
JOIN_TAB **last_changed_tab= changed_tabs;
- if (enabled_conds)
+ if (1)//enabled_conds) //psergey-todo:!
{
/*
For at least one of the pushed predicates the following is true:
@@ -1800,14 +1791,16 @@
{
for (uint i= 0; i < tab->ref.key_parts; i++)
{
- uint ref_col= tab->ref.outer_ref_cols[i];
- if (ref_col != UINT_MAX && !enabled_conds[ref_col])
+ bool *cond_guard= tab->ref.cond_guards[i];
+ if (cond_guard && !*cond_guard)
{
+ /* ... */
tab->read_first_record= init_read_record_seq;
tab->read_record.record= tab->table->record[0];
tab->read_record.thd= join->thd;
tab->read_record.ref_length= tab->table->file->ref_length;
*(last_changed_tab++)= tab;
+ //psergey-todo: break; ?
}
}
}
@@ -1835,13 +1828,9 @@
DBUG_RETURN(0);
}
-int subselect_union_engine::exec(bool *enabled_conds)
+int subselect_union_engine::exec()
{
char const *save_where= thd->where;
- /*
- Ignore the enabled_conds parameter: the pushed down predicates are only
- used for filtering, and the caller has disabled them if necessary.
- */
int res= unit->exec();
thd->where= save_where;
return res;
@@ -1977,7 +1966,7 @@
TRUE - an error occured while scanning
*/
-int subselect_uniquesubquery_engine::exec(bool *enabled_conds)
+int subselect_uniquesubquery_engine::exec()
{
DBUG_ENTER("subselect_uniquesubquery_engine::exec");
int error;
@@ -2075,7 +2064,7 @@
1
*/
-int subselect_indexsubquery_engine::exec(bool *enabled_conds)
+int subselect_indexsubquery_engine::exec()
{
DBUG_ENTER("subselect_indexsubquery_engine::exec");
int error;
--- 1.83/sql/item_subselect.h 2007-01-11 12:15:13 +03:00
+++ 1.84/sql/item_subselect.h 2007-01-11 12:15:13 +03:00
@@ -95,7 +95,7 @@
return null_value;
}
bool fix_fields(THD *thd, Item **ref);
- virtual bool exec(bool *enabled_conds);
+ virtual bool exec();
virtual void fix_length_and_dec();
table_map used_tables() const;
table_map not_null_tables() const { return 0; }
@@ -343,10 +343,7 @@
SYNOPSIS
exec()
- enabled_conds Pointer to the array of bool flags which tell which
- of the pushed down predicates are enabled.
- NULL, if all predicates are enabled or there are no
- pushed-down predicates.
+
DESCRIPTION
Execute the engine. The result of execution is subquery value that is
either captured by previously set up select_result-based 'sink' or
@@ -356,12 +353,13 @@
disabled, call of subselect_engine->no_rows()must return correct
result after this call.
+ psergey-todo: check comment
RETURN
0 - OK
1 - Either an execution error, or the engine was "changed", and the
caller should call exec() again for the new engine.
*/
- virtual int exec(bool *enabled_conds)= 0;
+ virtual int exec()= 0;
virtual uint cols()= 0; /* return number of columns in select */
virtual uint8 uncacheable()= 0; /* query is uncacheable */
enum Item_result type() { return res_type; }
@@ -396,7 +394,7 @@
void cleanup();
int prepare();
void fix_length_and_dec(Item_cache** row);
- int exec(bool *enabled_conds);
+ int exec();
uint cols();
uint8 uncacheable();
void exclude();
@@ -419,7 +417,7 @@
void cleanup();
int prepare();
void fix_length_and_dec(Item_cache** row);
- int exec(bool *enabled_conds);
+ int exec();
uint cols();
uint8 uncacheable();
void exclude();
@@ -476,7 +474,7 @@
void cleanup();
int prepare();
void fix_length_and_dec(Item_cache** row);
- int exec(bool *enabled_conds);
+ int exec();
uint cols() { return 1; }
uint8 uncacheable() { return UNCACHEABLE_DEPENDENT; }
void exclude();
@@ -534,7 +532,7 @@
check_null(chk_null),
having(having_arg)
{}
- int exec(bool *enabled_conds);
+ int exec();
void print (String *str);
};
--- 1.5/mysql-test/t/subselect3.test 2007-01-11 12:15:13 +03:00
+++ 1.6/mysql-test/t/subselect3.test 2007-01-11 12:15:13 +03:00
@@ -16,13 +16,14 @@
(3, 1, 4),
(3, 2, NULL);
-# Ok, for
+# Ok, for
# select max(ie) from t1 where oref=PARAM group by grp
# we'll have:
-# 1 -> (1, NULL) matching + NULL
-# 2 -> (3) non-matching
-# 3 -> (3, NULL) non-matching + NULL
-# 4 -> () nothing.
+# PARAM subquery result
+# 1 -> {(1), (NULL)} matching + NULL
+# 2 -> {(3)} non-matching
+# 3 -> {(3), (NULL)} non-matching + NULL
+# 4 -> {} empty set
create table t2 (oref int, a int);
insert into t2 values
@@ -141,7 +142,7 @@
#
-# BUG#24085
+# BUG#24085: Wrong query result for "NULL IN (SELECT ... UNION SELECT ...)"
#
# case 1: NULL IN (SELECT not_null_val FROM ...) w/o HAVING/GROUP-BY/etc
@@ -172,11 +173,13 @@
insert into t1 (oref, grp) values
(1, 1),
(1, 1);
-# Ok, for
-# select count(*) from t1 group by grp having grp=$PARAM$
+
+# Ok, for
+# select count(*) from t1 group by grp having grp=PARAM
# we'll have:
-# 1 -> (2)
-# 2 -> () - nothing
+# PARAM subuqery result
+# 1 -> {(2)}
+# 2 -> {} - empty set
create table t2 (oref int, a int);
insert into t2 values
(1, NULL),
| Thread |
|---|
| • bk commit into 5.0 tree (sergefp:1.2307) BUG#24127 | Sergey Petrunia | 11 Jan |
