diff options
author | Igor Babaev <igor@askmonty.org> | 2010-05-25 23:14:18 -0700 |
---|---|---|
committer | Igor Babaev <igor@askmonty.org> | 2010-05-25 23:14:18 -0700 |
commit | d120c5b562629dda782e3c9a66cfeaa48cbb01d1 (patch) | |
tree | 6da6a69adc00f657dc62d863a334d57f7a0f1907 /sql | |
parent | d6c97c913e4e0084f01b7551c5c2f06e10c2fdc7 (diff) | |
download | mariadb-git-d120c5b562629dda782e3c9a66cfeaa48cbb01d1.tar.gz |
Changed the fixes for the following bugs:
Bug #39022: completed
Bug #39653: reverted as invalid
Bug #45640: ameliorated, simplified, optimized
Bug #48483: completed
Bug #49324: improved
Bug #51242/52336: reverted, applied a real fix.
Diffstat (limited to 'sql')
-rw-r--r-- | sql/filesort.cc | 5 | ||||
-rw-r--r-- | sql/item.cc | 59 | ||||
-rw-r--r-- | sql/item.h | 9 | ||||
-rw-r--r-- | sql/mysql_priv.h | 5 | ||||
-rw-r--r-- | sql/opt_range.h | 14 | ||||
-rw-r--r-- | sql/sql_delete.cc | 3 | ||||
-rw-r--r-- | sql/sql_lex.cc | 1 | ||||
-rw-r--r-- | sql/sql_lex.h | 2 | ||||
-rw-r--r-- | sql/sql_select.cc | 227 | ||||
-rw-r--r-- | sql/sql_update.cc | 7 |
10 files changed, 166 insertions, 166 deletions
diff --git a/sql/filesort.cc b/sql/filesort.cc index 76777d78085..0325da0606d 100644 --- a/sql/filesort.cc +++ b/sql/filesort.cc @@ -608,7 +608,9 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, } if (error == 0) param->examined_rows++; - if (error == 0 && (!select || select->skip_record() == 0)) + + int rc= 0; + if (error == 0 && (!select || select->skip_record(thd) > 0)) { if (idx == param->keys) { @@ -621,6 +623,7 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, } else file->unlock_row(); + /* It does not make sense to read more keys in case of a fatal error */ if (thd->is_error()) break; diff --git a/sql/item.cc b/sql/item.cc index c2d1bb30d0a..5e8a941185a 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -4321,31 +4321,18 @@ bool Item_field::fix_fields(THD *thd, Item **reference) It's not an Item_field in the select list so we must make a new Item_ref to point to the Item in the select list and replace the Item_field created by the parser with the new Item_ref. - - NOTE: If we are fixing an alias reference inside ORDER/GROUP BY - item tree, then we use new Item_ref as an intermediate value - to resolve referenced item only. - In this case the new Item_ref item is unused. */ Item_ref *rf= new Item_ref(context, db_name,table_name,field_name); if (!rf) return 1; - - bool save_group_fix_field= thd->lex->current_select->group_fix_field; - /* - No need for recursive resolving of aliases. - */ - thd->lex->current_select->group_fix_field= 0; - bool ret= rf->fix_fields(thd, (Item **) &rf) || rf->check_cols(1); - thd->lex->current_select->group_fix_field= save_group_fix_field; if (ret) return TRUE; - - if (save_group_fix_field && alias_name_used) - thd->change_item_tree(reference, *rf->ref); - else - thd->change_item_tree(reference, rf); + + SELECT_LEX *select= thd->lex->current_select; + thd->change_item_tree(reference, + select->parsing_place == IN_GROUP_BY && + alias_name_used ? *rf->ref : rf); return FALSE; } @@ -6439,6 +6426,42 @@ bool Item_outer_ref::fix_fields(THD *thd, Item **reference) /** + Mark references from inner selects used in group by clause + + The method is used by the walk method when called for the expressions + from the group by clause. The callsare occurred in the function + fix_inner_refs invoked by JOIN::prepare. + The parameter passed to Item_outer_ref::check_inner_refs_processor + is the iterator over the list of inner references from the subselects + of the select to be prepared. The function marks those references + from this list whose occurrences are encountered in the group by + expressions passed to the walk method. + + @param arg pointer to the iterator over a list of inner references + + @return + FALSE always +*/ + +bool Item_outer_ref::check_inner_refs_processor(uchar *arg) +{ + List_iterator_fast<Item_outer_ref> *it= + ((List_iterator_fast<Item_outer_ref> *) arg); + Item_outer_ref *ref; + while ((ref= (*it)++)) + { + if (ref == this) + { + ref->found_in_group_by= 1; + break; + } + } + (*it).rewind(); + return FALSE; +} + + +/** Compare two view column references for equality. A view column reference is considered equal to another column diff --git a/sql/item.h b/sql/item.h index e99aac3a804..a99c97a44ac 100644 --- a/sql/item.h +++ b/sql/item.h @@ -905,7 +905,6 @@ public: virtual bool change_context_processor(uchar *context) { return 0; } virtual bool reset_query_id_processor(uchar *query_id_arg) { return 0; } virtual bool is_expensive_processor(uchar *arg) { return 0; } - virtual bool find_item_processor(uchar *arg) { return this == (void *) arg; } virtual bool register_field_in_read_map(uchar *arg) { return 0; } virtual bool enumerate_field_refs_processor(uchar *arg) { return 0; } virtual bool mark_as_eliminated_processor(uchar *arg) { return 0; } @@ -998,6 +997,8 @@ public: return FALSE; } + virtual bool check_inner_refs_processor(uchar *arg) { return FALSE; } + /* For SP local variable returns pointer to Item representing its current value and pointer to current Item otherwise. @@ -2450,12 +2451,13 @@ public: of the outer select. */ bool found_in_select_list; + bool found_in_group_by; Item_outer_ref(Name_resolution_context *context_arg, Item_field *outer_field_arg) :Item_direct_ref(context_arg, 0, outer_field_arg->table_name, outer_field_arg->field_name), outer_ref(outer_field_arg), in_sum_func(0), - found_in_select_list(0) + found_in_select_list(0), found_in_group_by(0) { ref= &outer_ref; set_properties(); @@ -2466,7 +2468,7 @@ public: bool alias_name_used_arg) :Item_direct_ref(context_arg, item, table_name_arg, field_name_arg, alias_name_used_arg), - outer_ref(0), in_sum_func(0), found_in_select_list(1) + outer_ref(0), in_sum_func(0), found_in_select_list(1), found_in_group_by(0) {} void save_in_result_field(bool no_conversions) { @@ -2478,6 +2480,7 @@ public: return (*ref)->const_item() ? 0 : OUTER_REF_TABLE_BIT; } virtual Ref_Type ref_type() { return OUTER_REF; } + bool check_inner_refs_processor(uchar * arg); }; diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 5ceaf00d1f2..6629374187a 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -640,7 +640,8 @@ enum enum_parsing_place IN_HAVING, SELECT_LIST, IN_WHERE, - IN_ON + IN_ON, + IN_GROUP_BY }; struct st_table; @@ -1196,7 +1197,7 @@ int setup_group(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, List<Item> &fields, List<Item> &all_fields, ORDER *order, bool *hidden_group_fields); bool fix_inner_refs(THD *thd, List<Item> &all_fields, SELECT_LEX *select, - Item **ref_pointer_array, ORDER *group_list= NULL); + Item **ref_pointer_array); bool handle_select(THD *thd, LEX *lex, select_result *result, ulong setup_tables_done_option); diff --git a/sql/opt_range.h b/sql/opt_range.h index ad2eddba4f0..7b91074dbe1 100644 --- a/sql/opt_range.h +++ b/sql/opt_range.h @@ -718,7 +718,19 @@ class SQL_SELECT :public Sql_alloc { tmp.set_all(); return test_quick_select(thd, tmp, 0, limit, force_quick_range) < 0; } - inline bool skip_record() { return cond ? cond->val_int() == 0 : 0; } + /* + RETURN + 0 if record must be skipped <-> (cond && cond->val_int() == 0) + -1 if error + 1 otherwise + */ + inline int skip_record(THD *thd) + { + int rc= test(!cond || cond->val_int()); + if (thd->is_error()) + rc= -1; + return rc; + } int test_quick_select(THD *thd, key_map keys, table_map prev_tables, ha_rows limit, bool force_quick_range); }; diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc index 6b395fb2636..ddb6af97865 100644 --- a/sql/sql_delete.cc +++ b/sql/sql_delete.cc @@ -305,9 +305,8 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, ! thd->is_error()) { // thd->is_error() is tested to disallow delete row on error - if (!(select && select->skip_record())&& ! thd->is_error() ) + if (!select || select->skip_record(thd) > 0) { - if (triggers_applicable && table->triggers->process_triggers(thd, TRG_EVENT_DELETE, TRG_ACTION_BEFORE, FALSE)) diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 411a7f5ec49..5a3907d0f7f 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -1603,7 +1603,6 @@ void st_select_lex::init_query() having= prep_having= where= prep_where= 0; olap= UNSPECIFIED_OLAP_TYPE; having_fix_field= 0; - group_fix_field= 0; context.select_lex= this; context.init(); /* diff --git a/sql/sql_lex.h b/sql/sql_lex.h index f9838c26cb5..6b33793f830 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -647,8 +647,6 @@ public: bool braces; /* SELECT ... UNION (SELECT ... ) <- this braces */ /* TRUE when having fix field called in processing of this SELECT */ bool having_fix_field; - /* TRUE when GROUP BY fix field called in processing of this SELECT */ - bool group_fix_field; /* List of references to fields referenced from inner selects */ List<Item_outer_ref> inner_refs_list; /* Number of Item_sum-derived objects in this SELECT */ diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 4e05b959829..5245063f513 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -317,8 +317,8 @@ bool handle_select(THD *thd, LEX *lex, select_result *result, function is aggregated in the select where the outer field was resolved or in some more inner select then the Item_direct_ref class should be used. - Also it should be used if we are grouping by a subquery containing - the outer field. + It used used also if we are grouping by a subquery that refers + this outer field. The resolution is done here and not at the fix_fields() stage as it can be done only after sum functions are fixed and pulled up to selects where they are have to be aggregated. @@ -335,13 +335,25 @@ bool handle_select(THD *thd, LEX *lex, select_result *result, bool fix_inner_refs(THD *thd, List<Item> &all_fields, SELECT_LEX *select, - Item **ref_pointer_array, ORDER *group_list) + Item **ref_pointer_array) { Item_outer_ref *ref; bool res= FALSE; bool direct_ref= FALSE; - List_iterator<Item_outer_ref> ref_it(select->inner_refs_list); + /* + Mark the references from the inner_refs_list that are occurred in + the group by expressions. Those references will contain direct + references to the referred fields. The markers are set in + the found_in_group_by field of the references from the list. + */ + List_iterator_fast <Item_outer_ref> ref_it(select->inner_refs_list); + for (ORDER *group= select->join->group_list; group; group= group->next) + { + (*group->item)->walk(&Item::check_inner_refs_processor, + TRUE, (uchar *) &ref_it); + } + while ((ref= ref_it++)) { Item *item= ref->outer_ref; @@ -385,22 +397,9 @@ fix_inner_refs(THD *thd, List<Item> &all_fields, SELECT_LEX *select, } } } - else - { - /* - Check if GROUP BY item trees contain the outer ref: - in this case we have to use Item_direct_ref instead of Item_ref. - */ - for (ORDER *group= group_list; group; group= group->next) - { - if ((*group->item)->walk(&Item::find_item_processor, TRUE, - (uchar *) ref)) - { - direct_ref= TRUE; - break; - } - } - } + else if (ref->found_in_group_by) + direct_ref= TRUE; + new_ref= direct_ref ? new Item_direct_ref(ref->context, item_ref, ref->table_name, ref->field_name, ref->alias_name_used) : @@ -607,8 +606,7 @@ JOIN::prepare(Item ***rref_pointer_array, } if (select_lex->inner_refs_list.elements && - fix_inner_refs(thd, all_fields, select_lex, ref_pointer_array, - group_list)) + fix_inner_refs(thd, all_fields, select_lex, ref_pointer_array)) DBUG_RETURN(-1); if (group_list) @@ -1123,31 +1121,6 @@ JOIN::optimize() { conds=new Item_int((longlong) 0,1); // Always false } - - /* - It's necessary to check const part of HAVING cond as - there is a chance that some cond parts may become - const items after make_join_statisctics(for example - when Item is a reference to cost table field from - outer join). - This check is performed only for those conditions - which do not use aggregate functions. In such case - temporary table may not be used and const condition - elements may be lost during further having - condition transformation in JOIN::exec. - */ - if (having && const_table_map) - { - having->update_used_tables(); - having= remove_eq_conds(thd, having, &having_value); - if (having_value == Item::COND_FALSE) - { - having= new Item_int((longlong) 0,1); - zero_result_cause= "Impossible HAVING noticed after reading const tables"; - DBUG_RETURN(0); - } - } - if (make_join_select(this, select, conds)) { zero_result_cause= @@ -2210,7 +2183,7 @@ JOIN::exec() Item* sort_table_cond= make_cond_for_table(curr_join->tmp_having, used_tables, - used_tables); + (table_map) 0); if (sort_table_cond) { if (!curr_table->select) @@ -8943,7 +8916,8 @@ simplify_joins(JOIN *join, List<TABLE_LIST> *join_list, COND *conds, bool top) For example it might happen if RAND() function is used in JOIN ON clause. */ - if (!((prev_table->on_expr->used_tables() & ~RAND_TABLE_BIT) & + if (!((prev_table->on_expr->used_tables() & + ~(OUTER_REF_TABLE_BIT | RAND_TABLE_BIT)) & ~prev_used_tables)) prev_table->dep_tables|= used_tables; } @@ -11867,50 +11841,36 @@ flush_cached_records(JOIN *join,JOIN_TAB *join_tab,bool skip_last) join->thd->send_kill_message(); return NESTED_LOOP_KILLED; // Aborted by user /* purecov: inspected */ } + int err= 0; SQL_SELECT *select=join_tab->select; - if (rc == NESTED_LOOP_OK) + if (rc == NESTED_LOOP_OK && + (!join_tab->cache.select || + (err= join_tab->cache.select->skip_record(join->thd)) != 0 )) { - bool consider_record= !join_tab->cache.select || - !join_tab->cache.select->skip_record(); - - /* - Check for error: skip_record() can execute code by calling - Item_subselect::val_*. We need to check for errors (if any) - after such call. - */ - if (join->thd->is_error()) - { - reset_cache_write(&join_tab->cache); + if (err < 0) return NESTED_LOOP_ERROR; - } - - if (consider_record) + rc= NESTED_LOOP_OK; + reset_cache_read(&join_tab->cache); + for (uint i= (join_tab->cache.records- (skip_last ? 1 : 0)) ; i-- > 0 ;) { - uint i; - reset_cache_read(&join_tab->cache); - for (i=(join_tab->cache.records- (skip_last ? 1 : 0)) ; i-- > 0 ;) + read_cached_record(join_tab); + err= 0; + if (!select || (err= select->skip_record(join->thd)) != 0) { - read_cached_record(join_tab); - if (!select || !select->skip_record()) + if (err < 0) + return NESTED_LOOP_ERROR; + rc= (join_tab->next_select)(join,join_tab+1,0); + if (rc != NESTED_LOOP_OK && rc != NESTED_LOOP_NO_MORE_ROWS) { - /* - Check for error: skip_record() can execute code by calling - Item_subselect::val_*. We need to check for errors (if any) - after such call. - */ - if (join->thd->is_error()) - rc= NESTED_LOOP_ERROR; - else - rc= (join_tab->next_select)(join,join_tab+1,0); - if (rc != NESTED_LOOP_OK && rc != NESTED_LOOP_NO_MORE_ROWS) - { - reset_cache_write(&join_tab->cache); - return rc; - } + reset_cache_write(&join_tab->cache); + return rc; } } } } + + rc= NESTED_LOOP_OK; + } while (!(error=info->read_record(info))); if (skip_last) @@ -13280,35 +13240,12 @@ static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx, uint find_shortest_key(TABLE *table, const key_map *usable_keys) { + uint min_length= (uint) ~0; uint best= MAX_KEY; - uint usable_clustered_pk= (table->file->primary_key_is_clustered() && - table->s->primary_key != MAX_KEY && - usable_keys->is_set(table->s->primary_key)) ? - table->s->primary_key : MAX_KEY; if (!usable_keys->is_clear_all()) { - uint min_length= (uint) ~0; for (uint nr=0; nr < table->s->keys ; nr++) { - /* - As far as - 1) clustered primary key entry data set is a set of all record - fields (key fields and not key fields) and - 2) secondary index entry data is a union of its key fields and - primary key fields (at least InnoDB and its derivatives don't - duplicate primary key fields there, even if the primary and - the secondary keys have a common subset of key fields), - then secondary index entry data is always a subset of primary key - entry, and the PK is always longer. - Unfortunately, key_info[nr].key_length doesn't show the length - of key/pointer pair but a sum of key field lengths only, thus - we can't estimate index IO volume comparing only this key_length - value of seconday keys and clustered PK. - So, try secondary keys first, and choose PK only if there are no - usable secondary covering keys: - */ - if (nr == usable_clustered_pk) - continue; if (usable_keys->is_set(nr)) { if (table->key_info[nr].key_length < min_length) @@ -13319,7 +13256,7 @@ uint find_shortest_key(TABLE *table, const key_map *usable_keys) } } } - return best != MAX_KEY ? best : usable_clustered_pk; + return best; } /** @@ -13768,10 +13705,48 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit, key (e.g. as in Innodb). See Bug #28591 for details. */ - rec_per_key= used_key_parts && - used_key_parts <= keyinfo->key_parts ? - keyinfo->rec_per_key[used_key_parts-1] : 1; - set_if_bigger(rec_per_key, 1); + uint used_index_parts= keyinfo->key_parts; + uint used_pk_parts= 0; + if (used_key_parts > used_index_parts) + used_pk_parts= used_key_parts-used_index_parts; + rec_per_key= keyinfo->rec_per_key[used_key_parts-1]; + /* Take into account the selectivity of the used pk prefix */ + if (used_pk_parts) + { + KEY *pkinfo=tab->table->key_info+table->s->primary_key; + /* + If the values of of records per key for the prefixes + of the primary key are considered unknown we assume + they are equal to 1. + */ + if (used_key_parts == pkinfo->key_parts || + pkinfo->rec_per_key[0] == 0) + rec_per_key= 1; + if (rec_per_key > 1) + { + rec_per_key*= pkinfo->rec_per_key[used_pk_parts-1]; + rec_per_key/= pkinfo->rec_per_key[0]; + /* + The value of rec_per_key for the extended key has + to be adjusted accordingly if some components of + the secondary key are included in the primary key. + */ + for(uint i= 0; i < used_pk_parts; i++) + { + if (pkinfo->key_part[i].field->key_start.is_set(nr)) + { + /* + We presume here that for any index rec_per_key[i] != 0 + if rec_per_key[0] != 0. + */ + DBUG_ASSERT(pkinfo->rec_per_key[i]); + rec_per_key*= pkinfo->rec_per_key[i-1]; + rec_per_key/= pkinfo->rec_per_key[i]; + } + } + } + } + set_if_bigger(rec_per_key, 1); /* With a grouping query each group containing on average rec_per_key records produces only one row that will @@ -14957,30 +14932,12 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, time. We check order_item->fixed because Item_func_group_concat can put - arguments for which fix_fields already was called. - - group_fix_field= TRUE is to resolve aliases from the SELECT list - without creating of Item_ref-s: JOIN::exec() wraps aliased items - in SELECT list with Item_copy items. To re-evaluate such a tree - that includes Item_copy items we have to refresh Item_copy caches, - but: - - filesort() never refresh Item_copy items, - - end_send_group() checks every record for group boundary by the - test_if_group_changed function that obtain data from these - Item_copy items, but the copy_fields function that - refreshes Item copy items is called after group boundaries only - - that is a vicious circle. - So we prevent inclusion of Item_copy items. + arguments for which fix_fields already was called. */ - bool save_group_fix_field= thd->lex->current_select->group_fix_field; - if (is_group_field) - thd->lex->current_select->group_fix_field= TRUE; - bool ret= (!order_item->fixed && + if (!order_item->fixed && (order_item->fix_fields(thd, order->item) || (order_item= *order->item)->check_cols(1) || - thd->is_fatal_error)); - thd->lex->current_select->group_fix_field= save_group_fix_field; - if (ret) + thd->is_fatal_error)) return TRUE; /* Wrong field. */ uint el= all_fields.elements; @@ -15052,6 +15009,8 @@ setup_group(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, uint org_fields=all_fields.elements; thd->where="group statement"; + enum_parsing_place save_place= thd->lex->current_select->parsing_place; + thd->lex->current_select->parsing_place= IN_GROUP_BY; for (ord= order; ord; ord= ord->next) { if (find_order_in_list(thd, ref_pointer_array, tables, ord, fields, @@ -15064,6 +15023,8 @@ setup_group(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, return 1; } } + thd->lex->current_select->parsing_place= save_place; + if (thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY) { /* diff --git a/sql/sql_update.cc b/sql/sql_update.cc index d90480732cd..c71060d4066 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -469,9 +469,10 @@ int mysql_update(THD *thd, thd_proc_info(thd, "Searching rows for update"); ha_rows tmp_limit= limit; - while (!(error=info.read_record(&info)) && !thd->killed) + while (!(error=info.read_record(&info)) && + !thd->killed && !thd->is_error()) { - if (!(select && select->skip_record())) + if (!select || select->skip_record(thd) > 0) { if (table->file->was_semi_consistent_read()) continue; /* repeat the read of the same row if it still exists */ @@ -577,7 +578,7 @@ int mysql_update(THD *thd, while (!(error=info.read_record(&info)) && !thd->killed) { - if (!(select && select->skip_record())) + if (!select || select->skip_record(thd) > 0) { if (table->file->was_semi_consistent_read()) continue; /* repeat the read of the same row if it still exists */ |