Hi Timour,
Congratulations on having pushed MWL#89 into 5.3-main! This is quite an
achievement which really moves us forward.
I've been studying the new code and has got some feedback which I thought
I need to share. Please find the notes below, marked with 'psergey'.
diff -urN --exclude='.*' 5.3-timours-wl-clean/sql/filesort.cc
5.3-timours-wl/sql/filesort.cc
--- 5.3-timours-wl-clean/sql/filesort.cc 2011-05-11 20:12:50.0
+0100
+++ 5.3-timours-wl/sql/filesort.cc2011-05-11 20:08:50.0 +0100
@@ -612,10 +612,34 @@
}
DBUG_RETURN(HA_POS_ERROR); /* purecov: inspected */
}
+
+bool write_record= false;
if (error == 0)
+{
param-examined_rows++;
-
-if (error == 0 (!select || select-skip_record(thd) 0))
+ if (select select-cond)
+ {
+/*
+ If the condition 'select-cond' contains a subquery, restore the
+ original read/write sets of the table 'sort_form' because when
+ SQL_SELECT::skip_record evaluates this condition. it may include a
psergey:
- The last sentence doesn't parse, please fix.
- Why is the check done *for each record we enumerate*? Please move it outside
of the loop.
- I suspect that now, with Sanja's subquery code, each subquery has a list of
references it makes to outside, so this shortcut is not necessary (let's
discuss that on the next optimizer call)
+ correlated subquery predicate, such that some field in the subquery
+ refers to 'sort_form'.
+*/
+if (select-cond-with_subselect)
+ sort_form-column_bitmaps_set(save_read_set, save_write_set,
+save_vcol_set);
+write_record= (select-skip_record(thd) 0);
+if (select-cond-with_subselect)
+ sort_form-column_bitmaps_set(sort_form-tmp_set,
+sort_form-tmp_set,
+sort_form-tmp_set);
+ }
+ else
+write_record= true;
+}
+
+if (write_record)
{
if (idx == param-keys)
{
diff -urN --exclude='.*' 5.3-timours-wl-clean/sql/item_cmpfunc.cc
5.3-timours-wl/sql/item_cmpfunc.cc
--- 5.3-timours-wl-clean/sql/item_cmpfunc.cc 2011-05-11 20:12:50.0
+0100
+++ 5.3-timours-wl/sql/item_cmpfunc.cc2011-05-11 20:08:50.0
+0100
@@ -2072,6 +2072,18 @@
}
+bool Item_in_optimizer::is_expensive_processor(uchar *arg)
+{
+ return args[1]-is_expensive_processor(arg);
+}
+
+
+bool Item_in_optimizer::is_expensive()
+{
+ return args[1]-is_expensive();
+}
+
psergey: what about the cases when args[0] is expensive? It should either be
checked, or a comment here is needed with explanation why we don't need to
check it.
longlong Item_func_eq::val_int()
{
DBUG_ASSERT(fixed == 1);
diff -urN --exclude='.*' 5.3-timours-wl-clean/sql/item.h
5.3-timours-wl/sql/item.h
--- 5.3-timours-wl-clean/sql/item.h 2011-05-11 20:12:50.0 +0100
+++ 5.3-timours-wl/sql/item.h 2011-05-11 20:08:50.0 +0100
@@ -510,7 +521,7 @@
SUBSELECT_ITEM, ROW_ITEM, CACHE_ITEM, TYPE_HOLDER,
PARAM_ITEM, TRIGGER_FIELD_ITEM, DECIMAL_ITEM,
XPATH_NODESET, XPATH_NODESET_CMP,
- VIEW_FIXER_ITEM, EXPR_CACHE_ITEM};
+ VIEW_FIXER_ITEM, EXPR_CACHE_ITEM, UNKNOWN_ITEM};
psergey: When I grep for UNKNOWN_ITEM, I find only this occurence. Why add it?
enum cond_result { COND_UNDEF,COND_OK,COND_TRUE,COND_FALSE };
...
diff -urN --exclude='.*' 5.3-timours-wl-clean/sql/item_subselect.cc
5.3-timours-wl/sql/item_subselect.cc
--- 5.3-timours-wl-clean/sql/item_subselect.cc2011-05-11
20:12:50.0 +0100
+++ 5.3-timours-wl/sql/item_subselect.cc 2011-05-11 20:08:50.0
+0100
...
@@ -1659,49 +1696,40 @@
{
if (!(new_having= new Item_func_trig_cond(new_having,
get_cond_guard(0
-DBUG_RETURN(RES_ERROR);
+DBUG_RETURN(true);
}
-new_having-name= (char*)in_having_cond;
- select_lex-having= join-having= new_having;
- select_lex-having_fix_field= 1;
-
-/*
- we do not check join-having-fixed, because comparison function
- (from func-create) can't be fixed after creation
-*/
- tmp= join-having-fix_fields(thd, 0);
-select_lex-having_fix_field= 0;
-if (tmp)
- DBUG_RETURN(RES_ERROR);
+
+new_having-name= (char*) in_having_cond;
+if (fix_having(new_having, select_lex))
+ DBUG_RETURN(true);
+*having_item= new_having;
}
else
- {
- // it is single select without tables = possible optimization
-// remove the dependence mark since the item is moved to upper
-// select and is not outer
