Please review the below: ----- Forwarded message from Sergei Petrunia <psergey@askmonty.org> ----- Date: Wed, 2 Mar 2016 13:35:11 +0300 (FET) From: Sergei Petrunia <psergey@askmonty.org> To: commits@mariadb.org, sergey@mariadb.com Subject: b6a2441: Remove all comments starting with 'psergey' revision-id: b6a2441edf3bfce381f5dc24f5be23fce9ac36dd parent(s): b05158cc10a75196b5c0bf8dad9360608a2dd5b9 committer: Sergei Petrunia branch nick: 10.1-dbg5 timestamp: 2016-03-02 13:35:11 +0300 message: Remove all comments starting with 'psergey' --- sql/item.cc | 3 +-- sql/item_subselect.cc | 15 +-------------- sql/opt_range.cc | 4 ++-- sql/opt_subselect.cc | 1 - sql/opt_table_elimination.cc | 2 -- sql/sql_base.cc | 4 ++-- sql/sql_join_cache.cc | 7 +------ sql/sql_select.cc | 39 ++------------------------------------- sql/sql_statistics.cc | 9 --------- sql/sql_union.cc | 2 +- 10 files changed, 10 insertions(+), 76 deletions(-) diff --git a/sql/item.cc b/sql/item.cc index d6132cc..8fd45d5 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -4227,8 +4227,7 @@ static bool mark_as_dependent(THD *thd, SELECT_LEX *last, SELECT_LEX *current, DBUG_PRINT("info", ("mark_item: %p lex: %p", mark_item, last)); mark_item->depended_from= last; } - if (current->mark_as_dependent(thd, last, - /** resolved_item psergey-thu **/ mark_item)) + if (current->mark_as_dependent(thd, last, mark_item)) DBUG_RETURN(TRUE); if (thd->lex->describe & DESCRIBE_EXTENDED) { diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index a5ef81e..e11eafe 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -243,10 +243,6 @@ bool Item_subselect::fix_fields(THD *thd_param, Item **ref) done_first_fix_fields= TRUE; inside_first_fix_fields= TRUE; upper_refs.empty(); - /* - psergey-todo: remove _first_fix_fields calls, we need changes on every - execution - */ } eliminated= FALSE; @@ -1753,7 +1749,6 @@ my_decimal *Item_in_subselect::val_decimal(my_decimal *decimal_value) Check that the right part of the subselect contains no more than one column. E.g. in SELECT 1 IN (SELECT * ..) the right part is (SELECT * ...) */ - // psergey: duplicated_subselect_card_check if (select_lex->item_list.elements > 1) { my_error(ER_OPERAND_COLUMNS, MYF(0), 1); @@ -2172,7 +2167,6 @@ bool Item_allany_subselect::is_maxmin_applicable(JOIN *join) DBUG_ENTER("Item_in_subselect::row_value_transformer"); - // psergey: duplicated_subselect_card_check if (select_lex->item_list.elements != cols_num) { my_error(ER_OPERAND_COLUMNS, MYF(0), cols_num); @@ -3600,7 +3594,6 @@ void subselect_engine::set_row(List<Item> &item_list, Item_cache **row) if (!(row[i]= Item_cache::get_cache(thd, sel_item, sel_item->cmp_type()))) return; row[i]->setup(thd, sel_item); - //psergey-backport-timours: row[i]->store(sel_item); } if (item_list.elements > 1) cmp_type= res_type= ROW_RESULT; @@ -4017,9 +4010,6 @@ int subselect_uniquesubquery_engine::index_lookup() subselect_uniquesubquery_engine::~subselect_uniquesubquery_engine() { - /* Tell handler we don't need the index anymore */ - //psergey-merge-todo: the following was gone in 6.0: - //psergey-merge: don't need this after all: tab->table->file->ha_index_end(); } @@ -4172,9 +4162,6 @@ int subselect_indexsubquery_engine::exec() uint subselect_single_select_engine::cols() { - //psergey-sj-backport: the following assert was gone in 6.0: - //DBUG_ASSERT(select_lex->join != 0); // should be called after fix_fields() - //return select_lex->join->fields_list.elements; return select_lex->item_list.elements; } @@ -5136,7 +5123,7 @@ void check_out_index_stats(JOIN *join) table_map key_used=0; table_map non_key_used= 0; - bzero(&key_start_use, sizeof(key_start_use)); //psergey-todo: safe initialization! + bzero(&key_start_use, sizeof(key_start_use)); bzero(&key_infix_use, sizeof(key_infix_use)); for (order= join->group_list; order; order= order->next) diff --git a/sql/opt_range.cc b/sql/opt_range.cc index ed7e9a5..72a851d 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -8640,7 +8640,7 @@ static bool remove_nonrange_trees(RANGE_OPT_PARAM *param, SEL_TREE *tree) if (key1->elements != 1) { - key2->use_count+=key1->elements-1; //psergey: why we don't count that key1 has n-k-p? + key2->use_count+=key1->elements-1; key2->increment_use_count((int) key1->elements-1); } if (key1->type == SEL_ARG::MAYBE_KEY) @@ -10104,7 +10104,7 @@ ha_rows check_quick_select(PARAM *param, uint idx, bool index_only, } DBUG_PRINT("exit", ("Records: %lu", (ulong) rows)); - DBUG_RETURN(rows); //psergey-merge:todo: maintain first_null_comp. + DBUG_RETURN(rows); } diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc index cb356fb..1a13dd6 100644 --- a/sql/opt_subselect.cc +++ b/sql/opt_subselect.cc @@ -635,7 +635,6 @@ int check_and_do_in_subquery_rewrites(JOIN *join) (oe1, oe2) IN (SELECT ie1, ie2, ie3 ...) TODO why do we have this duplicated in IN->EXISTS transformers? - psergey-todo: fix these: grep for duplicated_subselect_card_check */ if (select_lex->item_list.elements != in_subs->left_expr->cols()) { diff --git a/sql/opt_table_elimination.cc b/sql/opt_table_elimination.cc index 912ef4a..8378e8e 100644 --- a/sql/opt_table_elimination.cc +++ b/sql/opt_table_elimination.cc @@ -757,8 +757,6 @@ void eliminate_tables(JOIN *join) else { DBUG_ASSERT(!tbl->nested_join || tbl->sj_on_expr); - //psergey-todo: is the following really correct or we'll need to descend - //down all ON clauses: ? if (tbl->sj_on_expr) tables_used_on_left |= tbl->sj_on_expr->used_tables(); } diff --git a/sql/sql_base.cc b/sql/sql_base.cc index ac2162b..3d02f9c 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -3965,7 +3965,7 @@ thr_lock_type read_lock_type_for_table(THD *thd, goto end; } DBUG_PRINT("tcache", ("opening table: '%s'.'%s' item: %p", - tables->db, tables->table_name, tables)); //psergey: invalid read of size 1 here + tables->db, tables->table_name, tables)); (*counter)++; /* @@ -8105,7 +8105,7 @@ bool setup_tables(THD *thd, Name_resolution_context *context, Item *item= table_list->jtbm_subselect->optimizer; if (table_list->jtbm_subselect->optimizer->fix_fields(thd, &item)) { - my_error(ER_TOO_MANY_TABLES,MYF(0), static_cast<int>(MAX_TABLES)); /* psergey-todo: WHY ER_TOO_MANY_TABLES ???*/ + my_error(ER_TOO_MANY_TABLES,MYF(0), static_cast<int>(MAX_TABLES)); DBUG_RETURN(1); } DBUG_ASSERT(item == table_list->jtbm_subselect->optimizer); diff --git a/sql/sql_join_cache.cc b/sql/sql_join_cache.cc index f84440a..9f69169 100644 --- a/sql/sql_join_cache.cc +++ b/sql/sql_join_cache.cc @@ -3919,11 +3919,6 @@ int JOIN_TAB_SCAN_MRR::next() If a record in in an incremental cache contains no fields then the association for the last record in cache will be equal to cache->end_pos */ - /* - psergey: this makes no sense where HA_MRR_NO_ASSOC is used. - DBUG_ASSERT(cache->buff <= (uchar *) (*ptr) && - (uchar *) (*ptr) <= cache->end_pos); - */ if (join_tab->table->vfield) update_virtual_fields(join->thd, join_tab->table); } @@ -4591,7 +4586,7 @@ bool JOIN_CACHE_BKAH::prepare_look_for_matches(bool skip_last) { last_matching_rec_ref_ptr= next_matching_rec_ref_ptr= 0; if (no_association && - !(curr_matching_chain= get_matching_chain_by_join_key())) //psergey: added '!' + !(curr_matching_chain= get_matching_chain_by_join_key())) return 1; last_matching_rec_ref_ptr= get_next_rec_ref(curr_matching_chain); return 0; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 4759af0..878cb8b 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -2861,9 +2861,6 @@ void JOIN::exec_inner() /* Free first data from old join */ - /* - psergey-todo: this is the place of pre-mature JOIN::free call. - */ curr_join->join_free(); if (curr_join->make_simple_join(this, curr_tmp_table)) DBUG_VOID_RETURN; @@ -3001,7 +2998,6 @@ void JOIN::exec_inner() curr_join->select_distinct=0; } curr_tmp_table->reginfo.lock_type= TL_UNLOCK; - // psergey-todo: here is one place where we switch to if (curr_join->make_simple_join(this, curr_tmp_table)) DBUG_VOID_RETURN; calc_group_buffer(curr_join, curr_join->group_list); @@ -6646,12 +6642,6 @@ static void choose_initial_table_order(JOIN *join) jtab_sort_func= straight_join ? join_tab_cmp_straight : join_tab_cmp; } - /* - psergey-todo: if we're not optimizing an SJM nest, - - sort that outer tables are first, and each sjm nest follows - - then, put each [sjm_table1, ... sjm_tableN] sub-array right where - WHERE clause pushdown would have put it. - */ my_qsort2(join->best_ref + join->const_tables, join->table_count - join->const_tables, sizeof(JOIN_TAB*), jtab_sort_func, (void*)join->emb_sjm_nest); @@ -9171,7 +9161,6 @@ static bool create_ref_for_key(JOIN *join, JOIN_TAB *j, !(parent->join_tab_reexec= (JOIN_TAB*) thd->alloc(sizeof(JOIN_TAB)))) DBUG_RETURN(TRUE); /* purecov: inspected */ - // psergey-todo: here, save the pointer for original join_tabs. join_tab= parent->join_tab_reexec; table= &parent->table_reexec[0]; parent->table_reexec[0]= temp_table; table_count= top_join_tab_count= 1; @@ -10060,9 +10049,6 @@ bool TABLE_LIST::is_active_sjm() DBUG_ASSERT(inner_tab->table); current_map= inner_tab->table->map; used_tables2|= current_map; - /* - psergey: have put the -1 below. It's bad, will need to fix it. - */ COND *tmp_cond= make_cond_for_table(thd, on_expr, used_tables2, current_map, /*(inner_tab - first_tab)*/ -1, @@ -11070,11 +11056,6 @@ void check_join_cache_usage_for_tables(JOIN *join, ulonglong options, idx, prev_tab); tab->use_join_cache= MY_TEST(tab->used_join_cache_level); - /* - psergey-merge: todo: raise the question that this is really stupid that - we can first allocate a join buffer, then decide not to use it and free - it. - */ if (join->return_tab) { tab= join->return_tab; @@ -15622,12 +15603,6 @@ bool cond_is_datetime_is_null(Item *cond) @retval false cannot be used */ -/* - psergey-todo: this returns false for int_column='1234' (here '1234' is a - constant. Need to discuss this with Bar). - - See also Field::test_if_equality_guaranees_uniqueness(const Item *item); -*/ static bool test_if_equality_guarantees_uniqueness(Item *l, Item *r) { @@ -18293,10 +18268,6 @@ enum_nested_loop_state rc= evaluate_join_record(join, join_tab, error); } - /* - Note: psergey has added the 2nd part of the following condition; the - change should probably be made in 5.1, too. - */ bool skip_over= FALSE; while (rc == NESTED_LOOP_OK && join->return_tab >= join_tab) { @@ -20956,7 +20927,7 @@ uint find_shortest_key(TABLE *table, const key_map *usable_keys) !(table->file->index_flags(best_key, 0, 1) & HA_CLUSTERED_INDEX))) goto use_filesort; - if (select && // psergey: why doesn't this use a quick? + if (select && table->quick_keys.is_set(best_key) && best_key != ref_key) { key_map tmp_map; @@ -24002,11 +23973,7 @@ void JOIN_TAB::save_explain_data(Explain_table_access *eta, eta->type= tab_type; /* Build "possible_keys" value */ - // psergey-todo: why does this use thd MEM_ROOT??? Doesn't this - // break ANALYZE ? thd->mem_root will be freed, and after that we will - // attempt to print the query plan? append_possible_keys(thd->mem_root, eta->possible_keys, table, keys); - // psergey-todo: ^ check for error return code /* Build "key", "key_len", and "ref" */ if (tab_type == JT_NEXT) @@ -24054,8 +24021,6 @@ void JOIN_TAB::save_explain_data(Explain_table_access *eta, eta->hash_next_key.set(thd->mem_root, & table->key_info[index], table->key_info[index].key_length); - // psergey-todo: ^ is the above correct? are we necessarily joining on all - // columns? } if (!key_info) @@ -24211,7 +24176,7 @@ void JOIN_TAB::save_explain_data(Explain_table_access *eta, eta->push_extra(ET_OPEN_FRM_ONLY); else eta->push_extra(ET_OPEN_FULL_TABLE); - /* psergey-note: the following has a bug.*/ + if (table_list->is_table_read_plan->trivial_show_command || (table_list->is_table_read_plan->has_db_lookup_value() && table_list->is_table_read_plan->has_table_lookup_value())) diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc index 28e76ae..b22a2ae 100644 --- a/sql/sql_statistics.cc +++ b/sql/sql_statistics.cc @@ -3650,15 +3650,6 @@ double get_column_range_cardinality(Field *field, { double avg_frequency= col_stats->get_avg_frequency(); res= avg_frequency; - /* - psergey-todo: what does check for min_value, max_value mean? - min/max_value are set to NULL in alloc_statistics_for_table() and - alloc_statistics_for_table_share(). Both functions will immediately - call create_min_max_statistical_fields_for_table and - create_min_max_statistical_fields_for_table_share() respectively, - which will set min/max_value to be valid pointers, unless OOM - occurs. - */ if (avg_frequency > 1.0 + 0.000001 && col_stats->min_value && col_stats->max_value) { diff --git a/sql/sql_union.cc b/sql/sql_union.cc index c835083..b369ef0 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -974,7 +974,7 @@ bool st_select_lex_unit::exec() 1st execution sets certain members (e.g. select_result) to perform subquery execution rather than EXPLAIN line production. In order to reset them back, we re-do all of the actions (yes it is ugly): - */ // psergey-todo: is the above really necessary anymore?? + */ join->init(thd, item_list, fake_select_lex->options, result); saved_error= mysql_select(thd, &fake_select_lex->ref_pointer_array, &result_table_list, ----- End forwarded message ----- -- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog