From 588e67956af5c21191189f669f11f083e8ae35f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vicen=C8=9Biu=20Ciorbaru?= Date: Tue, 13 Aug 2019 19:29:59 +0300 Subject: Make sure histograms do not write uninitialized bytes to record A histogram size that is odd in size with DOUBLE precision will leave the last byte unwritten. When collecting histograms, this causes the last byte to be uninitialized in the record. memset the buffer to 0 first to make sure this does not happen. --- sql/sql_statistics.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc index 37f73adccb3..52b3811e60d 100644 --- a/sql/sql_statistics.cc +++ b/sql/sql_statistics.cc @@ -2109,7 +2109,12 @@ int alloc_statistics_for_table(THD* thd, TABLE *table) Histogram_type hist_type= (Histogram_type) (thd->variables.histogram_type); uchar *histogram= NULL; if (hist_size > 0) - histogram= (uchar *) alloc_root(&table->mem_root, hist_size * columns); + { + if ((histogram= (uchar *) alloc_root(&table->mem_root, + hist_size * columns))) + bzero(histogram, hist_size * columns); + + } if (!table_stats || !column_stats || !index_stats || !idx_avg_frequency || (hist_size && !histogram)) -- cgit v1.2.1 From 1c75ad6eed744672fdce77b0752801b67edb69f8 Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Thu, 15 Aug 2019 12:57:21 +0300 Subject: MDEV-19834 Selectivity of an equality condition discounted twice When discounting selectivity of ref access, don't discount the selectivity we've already discounted for range access. The 10.1 version of the fix. Will need to adjust condition filtering test results in 10.4 --- sql/sql_select.cc | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'sql') diff --git a/sql/sql_select.cc b/sql/sql_select.cc index fb8e4755b1d..29fc3f80ed0 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -7618,6 +7618,7 @@ double table_cond_selectivity(JOIN *join, uint idx, JOIN_TAB *s, KEYUSE *keyuse= pos->key; KEYUSE *prev_ref_keyuse= keyuse; uint key= keyuse->key; + bool used_range_selectivity= false; /* Check if we have a prefix of key=const that matches a quick select. @@ -7643,6 +7644,7 @@ double table_cond_selectivity(JOIN *join, uint idx, JOIN_TAB *s, keyparts++; } sel /= (double)table->quick_rows[key] / (double) table->stat_records(); + used_range_selectivity= true; } } @@ -7678,13 +7680,14 @@ double table_cond_selectivity(JOIN *join, uint idx, JOIN_TAB *s, if (keyparts > keyuse->keypart) { /* Ok this is the keyuse that will be used for ref access */ - uint fldno; - if (is_hash_join_key_no(key)) - fldno= keyuse->keypart; - else - fldno= table->key_info[key].key_part[keyparts-1].fieldnr - 1; - if (keyuse->val->const_item()) + if (!used_range_selectivity && keyuse->val->const_item()) { + uint fldno; + if (is_hash_join_key_no(key)) + fldno= keyuse->keypart; + else + fldno= table->key_info[key].key_part[keyparts-1].fieldnr - 1; + if (table->field[fldno]->cond_selectivity > 0) { sel /= table->field[fldno]->cond_selectivity; -- cgit v1.2.1 From fa74088838c12210d782aa6c69faa5acebc1d3bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Thu, 15 Aug 2019 07:46:41 +0300 Subject: MDEV-18778: mysql_tzinfo_to_sql does not work correctly in MariaDB Galera There were two problems: (1) If user wanted same time zone information on all nodes in the Galera cluster all updates were not replicated as time zone information was stored on MyISAM tables. This is fixed on Galera by altering time zone tables to InnoDB while they are modified. (2) If user wanted different time zone information to nodes in the Galera cluster TRUNCATE TABLE for time zone tables was replicated by Galera destroying time zone information from other nodes. This is fixed on Galera by introducing new option for mysql_tzinfo_to_sql_symlink tool --skip-write-binlog to disable Galera replication while time zone tables are modified. Changes to be committed: modified: mysql-test/r/mysql_tzinfo_to_sql_symlink.result modified: mysql-test/suite/wsrep/r/mysql_tzinfo_to_sql_symlink.result new file: mysql-test/suite/wsrep/r/mysql_tzinfo_to_sql_symlink_skip.result new file: mysql-test/suite/wsrep/t/mysql_tzinfo_to_sql_symlink_skip.test modified: sql/tztime.cc --- sql/tztime.cc | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 9 deletions(-) (limited to 'sql') diff --git a/sql/tztime.cc b/sql/tztime.cc index 060f5611fdb..8f66dfa0c9e 100644 --- a/sql/tztime.cc +++ b/sql/tztime.cc @@ -148,6 +148,7 @@ typedef struct st_time_zone_info static my_bool prepare_tz_info(TIME_ZONE_INFO *sp, MEM_ROOT *storage); +my_bool opt_leap, opt_verbose, opt_skip_write_binlog; #if defined(TZINFO2SQL) || defined(TESTTIME) @@ -2439,6 +2440,14 @@ print_tz_leaps_as_sql(const TIME_ZONE_INFO *sp) We are assuming that there are only one list of leap seconds For all timezones. */ + if (!opt_skip_write_binlog) + printf("\\d |\n" + "IF (select count(*) from information_schema.global_variables where\n" + "variable_name='wsrep_on') = 1 THEN\n" + "ALTER TABLE time_zone_leap_second ENGINE=InnoDB;\n" + "END IF|\n" + "\\d ;\n"); + printf("TRUNCATE TABLE time_zone_leap_second;\n"); if (sp->leapcnt) @@ -2451,6 +2460,14 @@ print_tz_leaps_as_sql(const TIME_ZONE_INFO *sp) printf(";\n"); } + if (!opt_skip_write_binlog) + printf("\\d |\n" + "IF (select count(*) from information_schema.global_variables where\n" + "variable_name='wsrep_on') = 1 THEN\n" + "ALTER TABLE time_zone_leap_second ENGINE=MyISAM;\n" + "END IF|\n" + "\\d ;\n"); + printf("ALTER TABLE time_zone_leap_second ORDER BY Transition_time;\n"); } @@ -2607,8 +2624,6 @@ scan_tz_dir(char * name_end, uint symlink_recursion_level, uint verbose) } -my_bool opt_leap, opt_verbose; - static const char *load_default_groups[]= { "mysql_tzinfo_to_sql", 0}; @@ -2629,6 +2644,8 @@ static struct my_option my_long_options[] = &opt_verbose, &opt_verbose, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, {"version", 'V', "Output version information and exit.", 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}, + {"skip-write-binlog", 'S', "Do not replicate changes to time zone tables to other nodes in a Galera cluster", + &opt_skip_write_binlog,&opt_skip_write_binlog, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, { 0, 0, 0, 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0} }; @@ -2697,11 +2714,14 @@ main(int argc, char **argv) return 1; } - // Replicate MyISAM DDL for this session, cf. lp:1161432 - // timezone info unfixable in XtraDB Cluster - printf("set @prep=if((select count(*) from information_schema.global_variables where variable_name='wsrep_on'), 'SET GLOBAL wsrep_replicate_myisam=?', 'do ?');\n" - "prepare set_wsrep_myisam from @prep;\n" - "set @toggle=1; execute set_wsrep_myisam using @toggle;\n"); + if (opt_skip_write_binlog) + /* If skip_write_binlog is set and wsrep is compiled in we disable + sql_log_bin and wsrep_on to avoid Galera replicating below + truncate table clauses. This will allow user to set different + time zones to nodes in Galera cluster. */ + printf("set @prep1=if((select count(*) from information_schema.global_variables where variable_name='wsrep_on'), 'SET SESSION SQL_LOG_BIN=?, WSREP_ON=OFF;', 'do ?');\n" + "prepare set_wsrep_write_binlog from @prep1;\n" + "set @toggle=0; execute set_wsrep_write_binlog using @toggle;\n"); if (argc == 1 && !opt_leap) { @@ -2709,6 +2729,21 @@ main(int argc, char **argv) root_name_end= strmake_buf(fullname, argv[0]); + if(!opt_skip_write_binlog) + { + // Alter time zone tables to InnoDB if wsrep_on is enabled + // to allow changes to them to replicate with Galera + printf("\\d |\n" + "IF (select count(*) from information_schema.global_variables where\n" + "variable_name='wsrep_on') = 1 THEN\n" + "ALTER TABLE time_zone ENGINE=InnoDB;\n" + "ALTER TABLE time_zone_name ENGINE=InnoDB;\n" + "ALTER TABLE time_zone_transition ENGINE=InnoDB;\n" + "ALTER TABLE time_zone_transition_type ENGINE=InnoDB;\n" + "END IF|\n" + "\\d ;\n"); + } + printf("TRUNCATE TABLE time_zone;\n"); printf("TRUNCATE TABLE time_zone_name;\n"); printf("TRUNCATE TABLE time_zone_transition;\n"); @@ -2750,8 +2785,19 @@ main(int argc, char **argv) free_root(&tz_storage, MYF(0)); } - // Reset wsrep_replicate_myisam. lp:1161432 - printf("set @toggle=0; execute set_wsrep_myisam using @toggle;\n"); + if(!opt_skip_write_binlog) + { + // Fall back to MyISAM + printf("\\d |\n" + "IF (select count(*) from information_schema.global_variables where\n" + "variable_name='wsrep_on') = 1 THEN\n" + "ALTER TABLE time_zone ENGINE=MyISAM;\n" + "ALTER TABLE time_zone_name ENGINE=MyISAM;\n" + "ALTER TABLE time_zone_transition ENGINE=MyISAM;\n" + "ALTER TABLE time_zone_transition_type ENGINE=MyISAM;\n" + "END IF|\n" + "\\d ;\n"); + } free_defaults(default_argv); my_end(0); -- cgit v1.2.1 From ec1f195ecf797fc5f37b0cfdbe4ee76d71a96729 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Fri, 16 Aug 2019 14:32:44 +0400 Subject: MDEV-15955 Assertion `field_types == 0 || field_types[field_pos] == MYSQL_TYPE_LONGLONG' failed in Protocol_text::store_longlong --- sql/sql_select.cc | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'sql') diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 3d78000b3d4..529ecd78836 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -14762,6 +14762,28 @@ static Field *create_tmp_field_from_item(THD *thd, Item *item, TABLE *table, if (item->cmp_type() == TIME_RESULT || item->field_type() == MYSQL_TYPE_GEOMETRY) new_field= item->tmp_table_field_from_field_type(table, 1); + else if (item->type() == Item::FUNC_ITEM && + static_cast(item)->functype() == Item_func::SUSERVAR_FUNC) + { + /* + A temporary solution for versions 5.5 .. 10.3. + This change should be null-merged to 10.4. + + Item_func_set_user_var is special. It overrides make_field(). + by adding a special branch `if (result_field)...`. + So it's important to preserve the exact data type here, + to avoid type mismatch in Protocol_text::store_longlong() + See MDEV-15955. + + Other Item_func descendants are not affected by MDEV-15955. + They don't override make_field() so they don't use result_field + when initializing Send_field. + + This is properly fixed in 10.4 in the method + Item_func_user_var::create_tmp_field_ex(). + */ + new_field= item->tmp_table_field_from_field_type(table, false); + } else switch (item->result_type()) { case REAL_RESULT: -- cgit v1.2.1 From 1639873671e85748f0dcf89ce76ef4efce9a087c Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Thu, 15 Aug 2019 22:32:59 +0300 Subject: MDEV-18154 Deadlock and assertion upon no-op ALTER under LOCK TABLES 1. Revert incorrect treatment of m_needs_reopen; 2. Close single instance of TABLE instead of all instances since reopened only those that are marked for reopen. --- sql/sql_base.cc | 13 ++++++++++--- sql/sql_table.cc | 3 --- 2 files changed, 10 insertions(+), 6 deletions(-) (limited to 'sql') diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 99c8a227861..3483c9381e8 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -2488,9 +2488,16 @@ Locked_tables_list::reopen_tables(THD *thd, bool need_reopen) { if (!table_list->table || !table_list->table->needs_reopen()) continue; - /* no need to remove the table from the TDC here, thus (TABLE*)1 */ - close_all_tables_for_name(thd, table_list->table->s, - HA_EXTRA_NOT_USED, (TABLE*)1); + for (TABLE **prev= &thd->open_tables; *prev; prev= &(*prev)->next) + { + if (*prev == table_list->table) + { + thd->locked_tables_list.unlink_from_list(thd, table_list, false); + mysql_lock_remove(thd, thd->lock, *prev); + close_thread_table(thd, prev); + break; + } + } DBUG_ASSERT(table_list->table == NULL); } else diff --git a/sql/sql_table.cc b/sql/sql_table.cc index be3f9b46c0f..1543a445d7b 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -7831,7 +7831,6 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, Create_field *def; Field **f_ptr,*field; MY_BITMAP *dropped_fields= NULL; // if it's NULL - no dropped fields - bool save_reopen= table->m_needs_reopen; DBUG_ENTER("mysql_prepare_alter_table"); /* @@ -8511,9 +8510,7 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, alter_info->create_list.swap(new_create_list); alter_info->key_list.swap(new_key_list); alter_info->check_constraint_list.swap(new_constraint_list); - DBUG_RETURN(rc); err: - table->m_needs_reopen= save_reopen; DBUG_RETURN(rc); } -- cgit v1.2.1 From 4d5382504d61ec27ace2d0dd4dd6cc71442ce067 Mon Sep 17 00:00:00 2001 From: Varun Gupta Date: Fri, 16 Aug 2019 16:49:12 +0530 Subject: MDEV-20349: Assertion `to_len >= 8' failed in convert_to_printable Use convert_to_printable function to write only non-empty ranges to the optimizer trace --- sql/sql_string.cc | 2 ++ 1 file changed, 2 insertions(+) (limited to 'sql') diff --git a/sql/sql_string.cc b/sql/sql_string.cc index a4050c579d0..483eb4fcbec 100644 --- a/sql/sql_string.cc +++ b/sql/sql_string.cc @@ -1204,6 +1204,8 @@ size_t convert_to_printable_required_length(uint len) bool String::append_semi_hex(const char *s, uint len, CHARSET_INFO *cs) { + if (!len) + return false; size_t dst_len= convert_to_printable_required_length(len); if (reserve(dst_len)) return true; -- cgit v1.2.1 From 6dd3f24090ce2d237037eb09cf7db083ebbc92f9 Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Sun, 18 Aug 2019 23:18:44 +0300 Subject: MDEV-19740 Debug build of 10.3.15 FTBFS * Replace LINT_INIT for non-struct types with ctor initializers; * Check BUILD_DEPS list is not empty so REMOVE_DUPLICATES won't throw error. --- sql/opt_subselect.h | 15 +++++++-------- sql/sql_explain.cc | 1 + sql/sql_select.h | 2 +- sql/sql_statistics.cc | 7 ++----- sql/sql_table.cc | 3 +-- sql/wsrep_thd.cc | 1 + 6 files changed, 13 insertions(+), 16 deletions(-) (limited to 'sql') diff --git a/sql/opt_subselect.h b/sql/opt_subselect.h index 94ae9a35246..2b9031b1b81 100644 --- a/sql/opt_subselect.h +++ b/sql/opt_subselect.h @@ -93,15 +93,14 @@ public: Loose_scan_opt(): try_loosescan(FALSE), bound_sj_equalities(0), - quick_uses_applicable_index(FALSE) + quick_uses_applicable_index(0), + quick_max_loose_keypart(0), + best_loose_scan_key(0), + best_loose_scan_cost(0), + best_loose_scan_records(0), + best_loose_scan_start_key(NULL), + best_max_loose_keypart(0) { - /* Protected by quick_uses_applicable_index */ - LINT_INIT(quick_max_loose_keypart); - /* The following are protected by best_loose_scan_cost!= DBL_MAX */ - LINT_INIT(best_loose_scan_key); - LINT_INIT(best_loose_scan_records); - LINT_INIT(best_max_loose_keypart); - LINT_INIT(best_loose_scan_start_key); } void init(JOIN *join, JOIN_TAB *s, table_map remaining_tables) diff --git a/sql/sql_explain.cc b/sql/sql_explain.cc index ca538752627..8ded81788a2 100644 --- a/sql/sql_explain.cc +++ b/sql/sql_explain.cc @@ -444,6 +444,7 @@ uint Explain_union::make_union_table_name(char *buf) break; default: DBUG_ASSERT(0); + type= {NULL, 0}; } memcpy(buf, type.str, (len= (uint)type.length)); diff --git a/sql/sql_select.h b/sql/sql_select.h index dd823a7d7df..efb0c474c89 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -866,7 +866,7 @@ public: void set_empty() { sjm_scan_need_tables= 0; - LINT_INIT_STRUCT(sjm_scan_last_inner); + sjm_scan_last_inner= 0; is_used= FALSE; } void set_from_prev(struct st_position *prev); diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc index 5b8c1b63b6f..9242e3b423a 100644 --- a/sql/sql_statistics.cc +++ b/sql/sql_statistics.cc @@ -1808,16 +1808,13 @@ public: bool is_partial_fields_present; Index_prefix_calc(THD *thd, TABLE *table, KEY *key_info) - : index_table(table), index_info(key_info) + : index_table(table), index_info(key_info), prefixes(0), empty(true), + calc_state(NULL), is_single_comp_pk(false), is_partial_fields_present(false) { uint i; Prefix_calc_state *state; uint key_parts= table->actual_n_key_parts(key_info); - empty= TRUE; - prefixes= 0; - LINT_INIT_STRUCT(calc_state); - is_partial_fields_present= is_single_comp_pk= FALSE; uint pk= table->s->primary_key; if ((uint) (table->key_info - key_info) == pk && table->key_info[pk].user_defined_key_parts == 1) diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 1543a445d7b..55e84cbfeb5 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -10863,10 +10863,9 @@ bool Sql_cmd_create_table_like::execute(THD *thd) { DBUG_ENTER("Sql_cmd_create_table::execute"); LEX *lex= thd->lex; - TABLE_LIST *all_tables= lex->query_tables; SELECT_LEX *select_lex= &lex->select_lex; TABLE_LIST *first_table= select_lex->table_list.first; - DBUG_ASSERT(first_table == all_tables && first_table != 0); + DBUG_ASSERT(first_table == lex->query_tables && first_table != 0); bool link_to_local; TABLE_LIST *create_table= first_table; TABLE_LIST *select_tables= lex->create_last_non_select_table->next_global; diff --git a/sql/wsrep_thd.cc b/sql/wsrep_thd.cc index a45828beab3..32fecda1eaa 100644 --- a/sql/wsrep_thd.cc +++ b/sql/wsrep_thd.cc @@ -432,6 +432,7 @@ static bool create_wsrep_THD(wsrep_thread_args* args) break; default: assert(0); + key= 0; break; } #endif -- cgit v1.2.1 From 52e276247d4573de0a01fa1500485fbd980d423a Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Mon, 19 Aug 2019 15:11:14 +0400 Subject: MDEV-19961 MIN(timestamp_column) returns a wrong result in a GROUP BY query --- sql/item_cmpfunc.h | 3 +++ sql/item_sum.cc | 72 +++++++++++++++++++++++++++++++++++++++++++++--------- sql/item_sum.h | 1 + sql/sql_type.h | 14 +++++++++++ 4 files changed, 78 insertions(+), 12 deletions(-) (limited to 'sql') diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 63c131cd8f4..10222bf8c42 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -132,6 +132,9 @@ public: int compare_e_json_str(); int compare_e_str_json(); + void min_max_update_field_native(THD *thd, Field *field, Item *item, + int cmp_sign); + Item** cache_converted_constant(THD *thd, Item **value, Item **cache, const Type_handler *type); inline bool is_owner_equal_func() diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 287a17aad3a..13a823fb10d 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -3086,18 +3086,32 @@ void Item_sum_min_max::update_field() tmp_item= args[0]; args[0]= direct_item; } - switch (result_type()) { - case STRING_RESULT: - min_max_update_str_field(); - break; - case INT_RESULT: - min_max_update_int_field(); - break; - case DECIMAL_RESULT: - min_max_update_decimal_field(); - break; - default: - min_max_update_real_field(); + if (Item_sum_min_max::type_handler()->is_val_native_ready()) + { + /* + TODO-10.5: change Item_sum_min_max to use val_native() for all data types + - make all type handlers val_native() ready + - use min_max_update_native_field() for all data types + - remove Item_sum_min_max::min_max_update_{str|real|int|decimal}_field() + */ + min_max_update_native_field(); + } + else + { + switch (Item_sum_min_max::type_handler()->cmp_type()) { + case STRING_RESULT: + case TIME_RESULT: + min_max_update_str_field(); + break; + case INT_RESULT: + min_max_update_int_field(); + break; + case DECIMAL_RESULT: + min_max_update_decimal_field(); + break; + default: + min_max_update_real_field(); + } } if (unlikely(direct_added)) { @@ -3108,6 +3122,40 @@ void Item_sum_min_max::update_field() } +void Arg_comparator::min_max_update_field_native(THD *thd, + Field *field, + Item *item, + int cmp_sign) +{ + DBUG_ENTER("Arg_comparator::min_max_update_field_native"); + if (!item->val_native(current_thd, &m_native2)) + { + if (field->is_null()) + field->store_native(m_native2); // The first non-null value + else + { + field->val_native(&m_native1); + if ((cmp_sign * m_compare_handler->cmp_native(m_native2, m_native1)) < 0) + field->store_native(m_native2); + } + field->set_notnull(); + } + DBUG_VOID_RETURN; +} + + +void +Item_sum_min_max::min_max_update_native_field() +{ + DBUG_ENTER("Item_sum_min_max::min_max_update_native_field"); + DBUG_ASSERT(cmp); + DBUG_ASSERT(type_handler_for_comparison() == cmp->compare_type_handler()); + THD *thd= current_thd; + cmp->min_max_update_field_native(thd, result_field, args[0], cmp_sign); + DBUG_VOID_RETURN; +} + + void Item_sum_min_max::min_max_update_str_field() { diff --git a/sql/item_sum.h b/sql/item_sum.h index 99cdf885c7c..7ef1b7eb234 100644 --- a/sql/item_sum.h +++ b/sql/item_sum.h @@ -1117,6 +1117,7 @@ public: void min_max_update_real_field(); void min_max_update_int_field(); void min_max_update_decimal_field(); + void min_max_update_native_field(); void cleanup(); bool any_value() { return was_values; } void no_rows_in_result(); diff --git a/sql/sql_type.h b/sql/sql_type.h index 18796f8c967..0f270ce9d5d 100644 --- a/sql/sql_type.h +++ b/sql/sql_type.h @@ -3244,6 +3244,16 @@ public: { return MYSQL_TIMESTAMP_ERROR; } + /* + Return true if the native format is fully implemented for a data type: + - Field_xxx::val_native() + - Item_xxx::val_native() for all classes supporting this data type + - Type_handler_xxx::cmp_native() + */ + virtual bool is_val_native_ready() const + { + return false; + } virtual bool is_timestamp_type() const { return false; @@ -5582,6 +5592,10 @@ public: { return MYSQL_TIMESTAMP_DATETIME; } + bool is_val_native_ready() const + { + return true; + } bool is_timestamp_type() const { return true; -- cgit v1.2.1 From 938925211a6ea23b06859a875a3891810ed5e6e9 Mon Sep 17 00:00:00 2001 From: Monty Date: Mon, 19 Aug 2019 19:49:45 +0300 Subject: MDEV-19254 Server crashes in maria_status with partitioned table Bug was that storage_engine::info() was called with not opened table in ha_partition::info(). Fixed by ensuring that we are using an opened table. --- sql/ha_partition.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index aa630e83ba0..5a78249644d 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -8304,6 +8304,7 @@ int ha_partition::info(uint flag) ulonglong max_records= 0; uint32 i= 0; uint32 handler_instance= 0; + bool handler_instance_set= 0; file_array= m_file; do @@ -8316,8 +8317,9 @@ int ha_partition::info(uint flag) !bitmap_is_set(&(m_part_info->read_partitions), (uint) (file_array - m_file))) file->info(HA_STATUS_VARIABLE | no_lock_flag | extra_var_flag); - if (file->stats.records > max_records) + if (file->stats.records > max_records || !handler_instance_set) { + handler_instance_set= 1; max_records= file->stats.records; handler_instance= i; } -- cgit v1.2.1 From e746f451d57def4be679caafc29976741b3e89f7 Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Thu, 15 Aug 2019 17:27:49 -0700 Subject: MDEV-20265 Unknown column in field list This patch corrects the fix of the patch for mdev-19421 that resolved the problem of parsing some embedded join expressions such as t1 join t2 left join t3 on t2.a=t3.a on t1.a=t2.a. Yet the patch contained a bug that prevented proper context analysis of the queries where such expressions were used together with comma separated table references in from clauses. --- sql/sql_parse.cc | 103 ++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 75 insertions(+), 28 deletions(-) (limited to 'sql') diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 346f8ad5e8b..ae5a6b4cd35 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -6435,10 +6435,6 @@ TABLE_LIST *st_select_lex::nest_last_join(THD *thd) TABLE_LIST *head= join_list->head(); if (head->nested_join && head->nested_join->nest_type & REBALANCED_NEST) { - List_iterator li(*join_list); - li++; - while (li++) - li.remove(); DBUG_RETURN(head); } @@ -6522,13 +6518,13 @@ void st_select_lex::add_joined_table(TABLE_LIST *table) context and right-associative in another context. In this query - SELECT * FROM t1 JOIN t2 LEFT JOIN t3 ON t2.a=t3.a (Q1) + SELECT * FROM t1 JOIN t2 LEFT JOIN t3 ON t2.a=t3.a (Q1) JOIN is left-associative and the query Q1 is interpreted as - SELECT * FROM (t1 JOIN t2) LEFT JOIN t3 ON t2.a=t3.a. + SELECT * FROM (t1 JOIN t2) LEFT JOIN t3 ON t2.a=t3.a. While in this query - SELECT * FROM t1 JOIN t2 LEFT JOIN t3 ON t2.a=t3.a ON t1.b=t2.b (Q2) + SELECT * FROM t1 JOIN t2 LEFT JOIN t3 ON t2.a=t3.a ON t1.b=t2.b (Q2) JOIN is right-associative and the query Q2 is interpreted as - SELECT * FROM t1 JOIN (t2 LEFT JOIN t3 ON t2.a=t3.a) ON t1.b=t2.b + SELECT * FROM t1 JOIN (t2 LEFT JOIN t3 ON t2.a=t3.a) ON t1.b=t2.b JOIN is right-associative if it is used with ON clause or with USING clause. Otherwise it is left-associative. @@ -6574,9 +6570,9 @@ void st_select_lex::add_joined_table(TABLE_LIST *table) J LJ - ON / \ / \ - t1 LJ - ON (TQ3*) => J t2 - / \ / \ - t3 t2 t1 t3 + t1 LJ - ON (TQ3*) => t3 J + / \ / \ + t3 t2 t1 t2 With several left associative JOINs SELECT * FROM t1 JOIN t2 JOIN t3 LEFT JOIN t4 ON t3.a=t4.a (Q4) @@ -6584,15 +6580,15 @@ void st_select_lex::add_joined_table(TABLE_LIST *table) J1 LJ - ON / \ / \ - t1 LJ - ON J2 t4 + t1 J2 J2 t4 / \ => / \ - J2 t4 J1 t3 - / \ / \ - t2 t3 t1 t2 + t2 LJ - ON J1 t3 + / \ / \ + t3 t4 t1 t2 - Here's another example: - SELECT * - FROM t1 JOIN t2 LEFT JOIN t3 JOIN t4 ON t3.a=t4.a ON t2.b=t3.b (Q5) + Here's another example: + SELECT * + FROM t1 JOIN t2 LEFT JOIN t3 JOIN t4 ON t3.a=t4.a ON t2.b=t3.b (Q5) J LJ - ON / \ / \ @@ -6602,15 +6598,58 @@ void st_select_lex::add_joined_table(TABLE_LIST *table) / \ t3 t4 - If the transformed nested join node node is a natural join node like in - the following query - SELECT * FROM t1 JOIN t2 LEFT JOIN t3 USING(a) (Q6) - the transformation additionally has to take care about setting proper - references in the field natural_join for both operands of the natural - join operation. - The function also has to change the name resolution context for ON - expressions used in the transformed join expression to take into - account the tables of the left_op node. + If the transformed nested join node node is a natural join node like in + the following query + SELECT * FROM t1 JOIN t2 LEFT JOIN t3 USING(a) (Q6) + the transformation additionally has to take care about setting proper + references in the field natural_join for both operands of the natural + join operation. + + The queries that combine comma syntax for join operation with + JOIN expression require a special care. Consider the query + SELECT * FROM t1, t2 JOIN t3 LEFT JOIN t4 ON t3.a=t4.a (Q7) + This query is equivalent to the query + SELECT * FROM (t1, t2) JOIN t3 LEFT JOIN t4 ON t3.a=t4.a + The latter is transformed in the same way as query Q1 + + J LJ - ON + / \ / \ + (t1,t2) LJ - ON => J t4 + / \ / \ + t3 t4 (t1,t2) t3 + + A transformation similar to the transformation for Q3 is done for + the following query with RIGHT JOIN + SELECT * FROM t1, t2 JOIN t3 RIGHT JOIN t4 ON t3.a=t4.a (Q8) + + J LJ - ON + / \ / \ + t3 LJ - ON => t4 J + / \ / \ + t4 (t1,t2) (t1,t2) t3 + + The function also has to change the name resolution context for ON + expressions used in the transformed join expression to take into + account the tables of the left_op node. + + TODO: + A more elegant solution would be to implement the transformation that + eliminates nests for cross join operations. For Q7 it would work like this: + + J LJ - ON + / \ / \ + (t1,t2) LJ - ON => (t1,t2,t3) t4 + / \ + t3 t4 + + For Q8 with RIGHT JOIN the transformation would work similarly: + + J LJ - ON + / \ / \ + t3 LJ - ON => t4 (t1,t2,t3) + / \ + t4 (t1,t2) + */ bool st_select_lex::add_cross_joined_table(TABLE_LIST *left_op, @@ -6633,7 +6672,11 @@ bool st_select_lex::add_cross_joined_table(TABLE_LIST *left_op, } TABLE_LIST *tbl; - List *jl= &right_op->nested_join->join_list; + List *right_op_jl= right_op->join_list; + TABLE_LIST *r_tbl= right_op_jl->pop(); + DBUG_ASSERT(right_op == r_tbl); + TABLE_LIST *l_tbl= right_op_jl->pop(); + DBUG_ASSERT(left_op == l_tbl); TABLE_LIST *cj_nest; /* @@ -6650,6 +6693,8 @@ bool st_select_lex::add_cross_joined_table(TABLE_LIST *left_op, List *cjl= &cj_nest->nested_join->join_list; cjl->empty(); + List *jl= &right_op->nested_join->join_list; + DBUG_ASSERT(jl->elements == 2); /* Look for the left most node tbl of the right_op tree */ for ( ; ; ) { @@ -6721,6 +6766,8 @@ bool st_select_lex::add_cross_joined_table(TABLE_LIST *left_op, create a new top level nested join node. */ right_op->nested_join->nest_type|= REBALANCED_NEST; + if (unlikely(right_op_jl->push_front(right_op))) + DBUG_RETURN(true); DBUG_RETURN(false); } -- cgit v1.2.1 From 457dc9d64d9f044760adbeca6facc813b754d43b Mon Sep 17 00:00:00 2001 From: Julius Goryavsky Date: Tue, 30 Jul 2019 13:45:13 +0200 Subject: MDEV-18863: Galera SST scripts can't read [mysqldN] option groups Some users and some scripts (for example, mysqld_multi.sh) use special option groups with names like [mysqld1], [mysqld2], ..., [mysqldN]. But SST scripts can't currently fully support these option groups. The only option group-related value it gets from the server is --defaults-group-suffix, if that option was set for mysqld when the server was started. However, the SST scripts does not get told by the server to read these option groups, so this means that the SST script will fail to read options like innodb-data-home-dir when it is in a option group like [mysqld1]...[mysqldN]. Moreover, SST scripts ignore many parameters that can be passed to them explicitly and cannot transfer them further, for example, to the input of mariabackup utility. Ideally, we want to transfer all the parameters of the original mysqld call to utilities such as mariabackup, however the SST script does not receive these parameters from the server and therefore cannot transfer them to mariabackup. To correct these shortcomings, we need to transfer to the scripts all of the parameters of the original mysqld call, and in the SST scripts themselves provide for the transfer all of these parameters to utilities such as mariabackup. To prevent these parameters from mixing with the script's own parameters, they should be transferred to SST script after the special option "--mysqld-args", followed by the string argument with the original parameters, as it received by the mysqld call at the time of launch (further all these parameters will be passed to mariabackup, for example). In addition, the SST scripts themselves must be refined so that they can read the parameters from the user-selected group, not just from the global mysqld configuration group. And also so that they can receive the parameters (which important for their work) as command-line arguments. --- sql/wsrep_sst.cc | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++------ sql/wsrep_sst.h | 1 + 2 files changed, 79 insertions(+), 8 deletions(-) (limited to 'sql') diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index 033992b7bd8..c470133d1b9 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -592,17 +592,76 @@ static int sst_append_data_dir(wsp::env& env, const char* data_dir) return -env.error(); } +static size_t estimate_cmd_len (bool* extra_args) +{ + /* + The length of the area reserved for the control parameters + of the SST script (excluding the copying of the original + mysqld arguments): + */ + size_t cmd_len= 4096; + bool extra= false; + /* + If mysqld was started with arguments, add them all: + */ + if (orig_argc > 1) + { + for (int i = 1; i < orig_argc; i++) + { + cmd_len += strlen(orig_argv[i]); + } + extra = true; + cmd_len += strlen(WSREP_SST_OPT_MYSQLD); + /* + Add the separating spaces between arguments, + and one additional space before "--mysqld-args": + */ + cmd_len += orig_argc; + } + *extra_args= extra; + return cmd_len; +} + +static void copy_orig_argv (char* cmd_str) +{ + /* + If mysqld was started with arguments, copy them all: + */ + if (orig_argc > 1) + { + size_t n = strlen(WSREP_SST_OPT_MYSQLD); + *cmd_str++ = ' '; + memcpy(cmd_str, WSREP_SST_OPT_MYSQLD, n * sizeof(char)); + cmd_str += n; + for (int i = 1; i < orig_argc; i++) + { + char* arg= orig_argv[i]; + *cmd_str++ = ' '; + n = strlen(arg); + memcpy(cmd_str, arg, n * sizeof(char)); + cmd_str += n; + } + /* + Add a terminating null character (not counted in the length, + since we've overwritten the original null character which + was previously added by snprintf: + */ + *cmd_str = 0; + } +} + static ssize_t sst_prepare_other (const char* method, const char* sst_auth, const char* addr_in, const char** addr_out) { - int const cmd_len= 4096; + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); wsp::string cmd_str(cmd_len); if (!cmd_str()) { - WSREP_ERROR("sst_prepare_other(): could not allocate cmd buffer of %d bytes", + WSREP_ERROR("sst_prepare_other(): could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -653,6 +712,9 @@ static ssize_t sst_prepare_other (const char* method, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + wsp::env env(NULL); if (env.error()) { @@ -916,13 +978,14 @@ static int sst_donate_mysqldump (const char* addr, } memcpy(host, address.get_address(), address.get_address_len()); int port= address.get_port(); - int const cmd_len= 4096; - wsp::string cmd_str(cmd_len); + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); + wsp::string cmd_str(cmd_len); if (!cmd_str()) { WSREP_ERROR("sst_donate_mysqldump(): " - "could not allocate cmd buffer of %d bytes", cmd_len); + "could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -951,6 +1014,9 @@ static int sst_donate_mysqldump (const char* addr, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + WSREP_DEBUG("Running: '%s'", cmd_str()); ret= sst_run_shell (cmd_str(), env, 3); @@ -1270,13 +1336,14 @@ static int sst_donate_other (const char* method, bool bypass, char** env) // carries auth info { - int const cmd_len= 4096; - wsp::string cmd_str(cmd_len); + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); + wsp::string cmd_str(cmd_len); if (!cmd_str()) { WSREP_ERROR("sst_donate_other(): " - "could not allocate cmd buffer of %d bytes", cmd_len); + "could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -1317,6 +1384,9 @@ static int sst_donate_other (const char* method, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + if (!bypass && wsrep_sst_donor_rejects_queries) sst_reject_queries(FALSE); pthread_t tmp; diff --git a/sql/wsrep_sst.h b/sql/wsrep_sst.h index 829a567f2cf..18e159a7729 100644 --- a/sql/wsrep_sst.h +++ b/sql/wsrep_sst.h @@ -32,6 +32,7 @@ #define WSREP_SST_OPT_PARENT "--parent" #define WSREP_SST_OPT_BINLOG "--binlog" #define WSREP_SST_OPT_BINLOG_INDEX "--binlog-index" +#define WSREP_SST_OPT_MYSQLD "--mysqld-args" // mysqldump-specific options #define WSREP_SST_OPT_USER "--user" -- cgit v1.2.1 From ff6d3075d5e66b3248b37379b44c6c6abf1ef0c3 Mon Sep 17 00:00:00 2001 From: Julius Goryavsky Date: Tue, 30 Jul 2019 13:45:13 +0200 Subject: MDEV-18863: Galera SST scripts can't read [mysqldN] option groups Some users and some scripts (for example, mysqld_multi.sh) use special option groups with names like [mysqld1], [mysqld2], ..., [mysqldN]. But SST scripts can't currently fully support these option groups. The only option group-related value it gets from the server is --defaults-group-suffix, if that option was set for mysqld when the server was started. However, the SST scripts does not get told by the server to read these option groups, so this means that the SST script will fail to read options like innodb-data-home-dir when it is in a option group like [mysqld1]...[mysqldN]. Moreover, SST scripts ignore many parameters that can be passed to them explicitly and cannot transfer them further, for example, to the input of mariabackup utility. Ideally, we want to transfer all the parameters of the original mysqld call to utilities such as mariabackup, however the SST script does not receive these parameters from the server and therefore cannot transfer them to mariabackup. To correct these shortcomings, we need to transfer to the scripts all of the parameters of the original mysqld call, and in the SST scripts themselves provide for the transfer all of these parameters to utilities such as mariabackup. To prevent these parameters from mixing with the script's own parameters, they should be transferred to SST script after the special option "--mysqld-args", followed by the string argument with the original parameters, as it received by the mysqld call at the time of launch (further all these parameters will be passed to mariabackup, for example). In addition, the SST scripts themselves must be refined so that they can read the parameters from the user-selected group, not just from the global mysqld configuration group. And also so that they can receive the parameters (which important for their work) as command-line arguments. --- sql/wsrep_sst.cc | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++------ sql/wsrep_sst.h | 1 + 2 files changed, 79 insertions(+), 8 deletions(-) (limited to 'sql') diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index cb8b1cab225..fe9f5650b4f 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -648,17 +648,76 @@ static int sst_append_data_dir(wsp::env& env, const char* data_dir) return -env.error(); } +static size_t estimate_cmd_len (bool* extra_args) +{ + /* + The length of the area reserved for the control parameters + of the SST script (excluding the copying of the original + mysqld arguments): + */ + size_t cmd_len= 4096; + bool extra= false; + /* + If mysqld was started with arguments, add them all: + */ + if (orig_argc > 1) + { + for (int i = 1; i < orig_argc; i++) + { + cmd_len += strlen(orig_argv[i]); + } + extra = true; + cmd_len += strlen(WSREP_SST_OPT_MYSQLD); + /* + Add the separating spaces between arguments, + and one additional space before "--mysqld-args": + */ + cmd_len += orig_argc; + } + *extra_args= extra; + return cmd_len; +} + +static void copy_orig_argv (char* cmd_str) +{ + /* + If mysqld was started with arguments, copy them all: + */ + if (orig_argc > 1) + { + size_t n = strlen(WSREP_SST_OPT_MYSQLD); + *cmd_str++ = ' '; + memcpy(cmd_str, WSREP_SST_OPT_MYSQLD, n * sizeof(char)); + cmd_str += n; + for (int i = 1; i < orig_argc; i++) + { + char* arg= orig_argv[i]; + *cmd_str++ = ' '; + n = strlen(arg); + memcpy(cmd_str, arg, n * sizeof(char)); + cmd_str += n; + } + /* + Add a terminating null character (not counted in the length, + since we've overwritten the original null character which + was previously added by snprintf: + */ + *cmd_str = 0; + } +} + static ssize_t sst_prepare_other (const char* method, const char* sst_auth, const char* addr_in, const char** addr_out) { - int const cmd_len= 4096; + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); wsp::string cmd_str(cmd_len); if (!cmd_str()) { - WSREP_ERROR("sst_prepare_other(): could not allocate cmd buffer of %d bytes", + WSREP_ERROR("sst_prepare_other(): could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -709,6 +768,9 @@ static ssize_t sst_prepare_other (const char* method, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + wsp::env env(NULL); if (env.error()) { @@ -972,13 +1034,14 @@ static int sst_donate_mysqldump (const char* addr, } memcpy(host, address.get_address(), address.get_address_len()); int port= address.get_port(); - int const cmd_len= 4096; - wsp::string cmd_str(cmd_len); + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); + wsp::string cmd_str(cmd_len); if (!cmd_str()) { WSREP_ERROR("sst_donate_mysqldump(): " - "could not allocate cmd buffer of %d bytes", cmd_len); + "could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -1007,6 +1070,9 @@ static int sst_donate_mysqldump (const char* addr, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + WSREP_DEBUG("Running: '%s'", cmd_str()); ret= sst_run_shell (cmd_str(), env, 3); @@ -1326,13 +1392,14 @@ static int sst_donate_other (const char* method, bool bypass, char** env) // carries auth info { - int const cmd_len= 4096; - wsp::string cmd_str(cmd_len); + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); + wsp::string cmd_str(cmd_len); if (!cmd_str()) { WSREP_ERROR("sst_donate_other(): " - "could not allocate cmd buffer of %d bytes", cmd_len); + "could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -1373,6 +1440,9 @@ static int sst_donate_other (const char* method, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + if (!bypass && wsrep_sst_donor_rejects_queries) sst_reject_queries(FALSE); pthread_t tmp; diff --git a/sql/wsrep_sst.h b/sql/wsrep_sst.h index e54cc25a456..063cab5f0f1 100644 --- a/sql/wsrep_sst.h +++ b/sql/wsrep_sst.h @@ -32,6 +32,7 @@ #define WSREP_SST_OPT_PARENT "--parent" #define WSREP_SST_OPT_BINLOG "--binlog" #define WSREP_SST_OPT_BINLOG_INDEX "--binlog-index" +#define WSREP_SST_OPT_MYSQLD "--mysqld-args" // mysqldump-specific options #define WSREP_SST_OPT_USER "--user" -- cgit v1.2.1 From 137d8ed3fee14335bd707474bd49f4e43b3bf309 Mon Sep 17 00:00:00 2001 From: Julius Goryavsky Date: Tue, 30 Jul 2019 13:45:13 +0200 Subject: MDEV-18863: Galera SST scripts can't read [mysqldN] option groups Some users and some scripts (for example, mysqld_multi.sh) use special option groups with names like [mysqld1], [mysqld2], ..., [mysqldN]. But SST scripts can't currently fully support these option groups. The only option group-related value it gets from the server is --defaults-group-suffix, if that option was set for mysqld when the server was started. However, the SST scripts does not get told by the server to read these option groups, so this means that the SST script will fail to read options like innodb-data-home-dir when it is in a option group like [mysqld1]...[mysqldN]. Moreover, SST scripts ignore many parameters that can be passed to them explicitly and cannot transfer them further, for example, to the input of mariabackup utility. Ideally, we want to transfer all the parameters of the original mysqld call to utilities such as mariabackup, however the SST script does not receive these parameters from the server and therefore cannot transfer them to mariabackup. To correct these shortcomings, we need to transfer to the scripts all of the parameters of the original mysqld call, and in the SST scripts themselves provide for the transfer all of these parameters to utilities such as mariabackup. To prevent these parameters from mixing with the script's own parameters, they should be transferred to SST script after the special option "--mysqld-args", followed by the string argument with the original parameters, as it received by the mysqld call at the time of launch (further all these parameters will be passed to mariabackup, for example). In addition, the SST scripts themselves must be refined so that they can read the parameters from the user-selected group, not just from the global mysqld configuration group. And also so that they can receive the parameters (which important for their work) as command-line arguments. --- sql/wsrep_sst.cc | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++------ sql/wsrep_sst.h | 1 + 2 files changed, 79 insertions(+), 8 deletions(-) (limited to 'sql') diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index 5c3e5642cda..bda79ed7406 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -642,17 +642,76 @@ static int sst_append_data_dir(wsp::env& env, const char* data_dir) return -env.error(); } +static size_t estimate_cmd_len (bool* extra_args) +{ + /* + The length of the area reserved for the control parameters + of the SST script (excluding the copying of the original + mysqld arguments): + */ + size_t cmd_len= 4096; + bool extra= false; + /* + If mysqld was started with arguments, add them all: + */ + if (orig_argc > 1) + { + for (int i = 1; i < orig_argc; i++) + { + cmd_len += strlen(orig_argv[i]); + } + extra = true; + cmd_len += strlen(WSREP_SST_OPT_MYSQLD); + /* + Add the separating spaces between arguments, + and one additional space before "--mysqld-args": + */ + cmd_len += orig_argc; + } + *extra_args= extra; + return cmd_len; +} + +static void copy_orig_argv (char* cmd_str) +{ + /* + If mysqld was started with arguments, copy them all: + */ + if (orig_argc > 1) + { + size_t n = strlen(WSREP_SST_OPT_MYSQLD); + *cmd_str++ = ' '; + memcpy(cmd_str, WSREP_SST_OPT_MYSQLD, n * sizeof(char)); + cmd_str += n; + for (int i = 1; i < orig_argc; i++) + { + char* arg= orig_argv[i]; + *cmd_str++ = ' '; + n = strlen(arg); + memcpy(cmd_str, arg, n * sizeof(char)); + cmd_str += n; + } + /* + Add a terminating null character (not counted in the length, + since we've overwritten the original null character which + was previously added by snprintf: + */ + *cmd_str = 0; + } +} + static ssize_t sst_prepare_other (const char* method, const char* sst_auth, const char* addr_in, const char** addr_out) { - int const cmd_len= 4096; + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); wsp::string cmd_str(cmd_len); if (!cmd_str()) { - WSREP_ERROR("sst_prepare_other(): could not allocate cmd buffer of %d bytes", + WSREP_ERROR("sst_prepare_other(): could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -703,6 +762,9 @@ static ssize_t sst_prepare_other (const char* method, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + wsp::env env(NULL); if (env.error()) { @@ -975,13 +1037,14 @@ static int sst_donate_mysqldump (const char* addr, } memcpy(host, address.get_address(), address.get_address_len()); int port= address.get_port(); - int const cmd_len= 4096; - wsp::string cmd_str(cmd_len); + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); + wsp::string cmd_str(cmd_len); if (!cmd_str()) { WSREP_ERROR("sst_donate_mysqldump(): " - "could not allocate cmd buffer of %d bytes", cmd_len); + "could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -1010,6 +1073,9 @@ static int sst_donate_mysqldump (const char* addr, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + WSREP_DEBUG("Running: '%s'", cmd_str()); ret= sst_run_shell (cmd_str(), env, 3); @@ -1329,13 +1395,14 @@ static int sst_donate_other (const char* method, bool bypass, char** env) // carries auth info { - int const cmd_len= 4096; - wsp::string cmd_str(cmd_len); + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); + wsp::string cmd_str(cmd_len); if (!cmd_str()) { WSREP_ERROR("sst_donate_other(): " - "could not allocate cmd buffer of %d bytes", cmd_len); + "could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -1376,6 +1443,9 @@ static int sst_donate_other (const char* method, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + if (!bypass && wsrep_sst_donor_rejects_queries) sst_reject_queries(FALSE); pthread_t tmp; diff --git a/sql/wsrep_sst.h b/sql/wsrep_sst.h index 1ad8ff5b5de..38706fa4671 100644 --- a/sql/wsrep_sst.h +++ b/sql/wsrep_sst.h @@ -32,6 +32,7 @@ #define WSREP_SST_OPT_PARENT "--parent" #define WSREP_SST_OPT_BINLOG "--binlog" #define WSREP_SST_OPT_BINLOG_INDEX "--binlog-index" +#define WSREP_SST_OPT_MYSQLD "--mysqld-args" // mysqldump-specific options #define WSREP_SST_OPT_USER "--user" -- cgit v1.2.1 From 89fb295b1b8a0ca314f1b8b45abac5795f465b77 Mon Sep 17 00:00:00 2001 From: Julius Goryavsky Date: Tue, 30 Jul 2019 13:45:13 +0200 Subject: MDEV-18863: Galera SST scripts can't read [mysqldN] option groups Some users and some scripts (for example, mysqld_multi.sh) use special option groups with names like [mysqld1], [mysqld2], ..., [mysqldN]. But SST scripts can't currently fully support these option groups. The only option group-related value it gets from the server is --defaults-group-suffix, if that option was set for mysqld when the server was started. However, the SST scripts does not get told by the server to read these option groups, so this means that the SST script will fail to read options like innodb-data-home-dir when it is in a option group like [mysqld1]...[mysqldN]. Moreover, SST scripts ignore many parameters that can be passed to them explicitly and cannot transfer them further, for example, to the input of mariabackup utility. Ideally, we want to transfer all the parameters of the original mysqld call to utilities such as mariabackup, however the SST script does not receive these parameters from the server and therefore cannot transfer them to mariabackup. To correct these shortcomings, we need to transfer to the scripts all of the parameters of the original mysqld call, and in the SST scripts themselves provide for the transfer all of these parameters to utilities such as mariabackup. To prevent these parameters from mixing with the script's own parameters, they should be transferred to SST script after the special option "--mysqld-args", followed by the string argument with the original parameters, as it received by the mysqld call at the time of launch (further all these parameters will be passed to mariabackup, for example). In addition, the SST scripts themselves must be refined so that they can read the parameters from the user-selected group, not just from the global mysqld configuration group. And also so that they can receive the parameters (which important for their work) as command-line arguments. --- sql/wsrep_sst.cc | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++------ sql/wsrep_sst.h | 1 + 2 files changed, 79 insertions(+), 8 deletions(-) (limited to 'sql') diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index 85d5aca342d..89e637ae075 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -576,17 +576,76 @@ static int sst_append_data_dir(wsp::env& env, const char* data_dir) return -env.error(); } +static size_t estimate_cmd_len (bool* extra_args) +{ + /* + The length of the area reserved for the control parameters + of the SST script (excluding the copying of the original + mysqld arguments): + */ + size_t cmd_len= 4096; + bool extra= false; + /* + If mysqld was started with arguments, add them all: + */ + if (orig_argc > 1) + { + for (int i = 1; i < orig_argc; i++) + { + cmd_len += strlen(orig_argv[i]); + } + extra = true; + cmd_len += strlen(WSREP_SST_OPT_MYSQLD); + /* + Add the separating spaces between arguments, + and one additional space before "--mysqld-args": + */ + cmd_len += orig_argc; + } + *extra_args= extra; + return cmd_len; +} + +static void copy_orig_argv (char* cmd_str) +{ + /* + If mysqld was started with arguments, copy them all: + */ + if (orig_argc > 1) + { + size_t n = strlen(WSREP_SST_OPT_MYSQLD); + *cmd_str++ = ' '; + memcpy(cmd_str, WSREP_SST_OPT_MYSQLD, n * sizeof(char)); + cmd_str += n; + for (int i = 1; i < orig_argc; i++) + { + char* arg= orig_argv[i]; + *cmd_str++ = ' '; + n = strlen(arg); + memcpy(cmd_str, arg, n * sizeof(char)); + cmd_str += n; + } + /* + Add a terminating null character (not counted in the length, + since we've overwritten the original null character which + was previously added by snprintf: + */ + *cmd_str = 0; + } +} + static ssize_t sst_prepare_other (const char* method, const char* sst_auth, const char* addr_in, const char** addr_out) { - int const cmd_len= 4096; + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); wsp::string cmd_str(cmd_len); if (!cmd_str()) { - WSREP_ERROR("sst_prepare_other(): could not allocate cmd buffer of %d bytes", + WSREP_ERROR("sst_prepare_other(): could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -637,6 +696,9 @@ static ssize_t sst_prepare_other (const char* method, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + wsp::env env(NULL); if (env.error()) { @@ -890,13 +952,14 @@ static int sst_donate_mysqldump (const char* addr, } memcpy(host, address.get_address(), address.get_address_len()); int port= address.get_port(); - int const cmd_len= 4096; - wsp::string cmd_str(cmd_len); + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); + wsp::string cmd_str(cmd_len); if (!cmd_str()) { WSREP_ERROR("sst_donate_mysqldump(): " - "could not allocate cmd buffer of %d bytes", cmd_len); + "could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -933,6 +996,9 @@ static int sst_donate_mysqldump (const char* addr, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + WSREP_DEBUG("Running: '%s'", cmd_str()); ret= sst_run_shell (cmd_str(), env, 3); @@ -1283,13 +1349,14 @@ static int sst_donate_other (const char* method, bool bypass, char** env) // carries auth info { - int const cmd_len= 4096; - wsp::string cmd_str(cmd_len); + bool extra_args; + size_t const cmd_len= estimate_cmd_len(&extra_args); + wsp::string cmd_str(cmd_len); if (!cmd_str()) { WSREP_ERROR("sst_donate_other(): " - "could not allocate cmd buffer of %d bytes", cmd_len); + "could not allocate cmd buffer of %zd bytes", cmd_len); return -ENOMEM; } @@ -1345,6 +1412,9 @@ static int sst_donate_other (const char* method, return (ret < 0 ? ret : -EMSGSIZE); } + if (extra_args) + copy_orig_argv(cmd_str() + ret); + if (!bypass && wsrep_sst_donor_rejects_queries) sst_reject_queries(FALSE); pthread_t tmp; diff --git a/sql/wsrep_sst.h b/sql/wsrep_sst.h index eb218647bc0..2389db4abe7 100644 --- a/sql/wsrep_sst.h +++ b/sql/wsrep_sst.h @@ -32,6 +32,7 @@ #define WSREP_SST_OPT_PARENT "--parent" #define WSREP_SST_OPT_BINLOG "--binlog" #define WSREP_SST_OPT_BINLOG_INDEX "--binlog-index" +#define WSREP_SST_OPT_MYSQLD "--mysqld-args" // mysqldump-specific options #define WSREP_SST_OPT_USER "--user" -- cgit v1.2.1 From bc89b1c5582ac4044317a84e8a73d97fd1db9041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 20 Aug 2019 07:47:11 +0300 Subject: MDEV-18863: Fix -Wsign-compare --- sql/wsrep_sst.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'sql') diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index c470133d1b9..fb1efa1d451 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -706,7 +706,7 @@ static ssize_t sst_prepare_other (const char* method, my_free(binlog_opt_val); my_free(binlog_index_opt_val); - if (ret < 0 || ret >= cmd_len) + if (ret < 0 || size_t(ret) >= cmd_len) { WSREP_ERROR("sst_prepare_other(): snprintf() failed: %d", ret); return (ret < 0 ? ret : -EMSGSIZE); @@ -1008,7 +1008,7 @@ static int sst_donate_mysqldump (const char* addr, (long long)seqno, wsrep_gtid_domain_id, bypass ? " " WSREP_SST_OPT_BYPASS : ""); - if (ret < 0 || ret >= cmd_len) + if (ret < 0 || size_t(ret) >= cmd_len) { WSREP_ERROR("sst_donate_mysqldump(): snprintf() failed: %d", ret); return (ret < 0 ? ret : -EMSGSIZE); @@ -1378,7 +1378,7 @@ static int sst_donate_other (const char* method, bypass ? " " WSREP_SST_OPT_BYPASS : ""); my_free(binlog_opt_val); - if (ret < 0 || ret >= cmd_len) + if (ret < 0 || size_t(ret) >= cmd_len) { WSREP_ERROR("sst_donate_other(): snprintf() failed: %d", ret); return (ret < 0 ? ret : -EMSGSIZE); -- cgit v1.2.1 From 7b4de10477a7bdb51656d827ad2d914d29a4be4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Tue, 20 Aug 2019 10:32:04 +0300 Subject: MDEV-20378: Galera uses uninitialized memory Problem was that wsrep thread argument was deleted on wrong place. Furthermore, scan method incorrectly used unsafe c_ptr(). Finally, fixed wsrep thread initialization to correctly set up thread_id and pass correct argument to functions and fix signess problem causing compiler errors. --- sql/wsrep_mysqld.cc | 5 ++--- sql/wsrep_mysqld.h | 15 +++++++-------- sql/wsrep_schema.cc | 4 +++- sql/wsrep_sst.cc | 6 +++--- sql/wsrep_thd.cc | 26 +++++++++++++++----------- 5 files changed, 30 insertions(+), 26 deletions(-) (limited to 'sql') diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc index 74a28c4724f..0a9adc5fa2c 100644 --- a/sql/wsrep_mysqld.cc +++ b/sql/wsrep_mysqld.cc @@ -2696,7 +2696,7 @@ void* start_wsrep_THD(void *arg) WSREP_DEBUG("wsrep system thread %llu, %p starting", thd->thread_id, thd); - thd_args->fun()(thd, thd_args->args()); + thd_args->fun()(thd, static_cast(thd_args)); WSREP_DEBUG("wsrep system thread: %llu, %p closing", thd->thread_id, thd); @@ -2707,8 +2707,6 @@ void* start_wsrep_THD(void *arg) close_connection(thd, 0); - delete thd_args; - mysql_mutex_lock(&LOCK_wsrep_slave_threads); DBUG_ASSERT(wsrep_running_threads > 0); wsrep_running_threads--; @@ -2727,6 +2725,7 @@ void* start_wsrep_THD(void *arg) break; } + delete thd_args; WSREP_DEBUG("wsrep running threads now: %lu", wsrep_running_threads); mysql_cond_broadcast(&COND_wsrep_slave_threads); mysql_mutex_unlock(&LOCK_wsrep_slave_threads); diff --git a/sql/wsrep_mysqld.h b/sql/wsrep_mysqld.h index 8714753ba76..d71d4afea11 100644 --- a/sql/wsrep_mysqld.h +++ b/sql/wsrep_mysqld.h @@ -411,18 +411,17 @@ typedef void (*wsrep_thd_processor_fun)(THD*, void *); class Wsrep_thd_args { public: - Wsrep_thd_args(wsrep_thd_processor_fun fun, void* args, - wsrep_thread_type thread_type) + Wsrep_thd_args(wsrep_thd_processor_fun fun, + wsrep_thread_type thread_type, + pthread_t thread_id) : fun_ (fun), - args_ (args), - thread_type_ (thread_type) + thread_type_ (thread_type), + thread_id_ (thread_id) { } wsrep_thd_processor_fun fun() { return fun_; } - - void* args() { return args_; } - + pthread_t* thread_id() {return &thread_id_; } enum wsrep_thread_type thread_type() {return thread_type_;} private: @@ -431,8 +430,8 @@ class Wsrep_thd_args Wsrep_thd_args& operator=(const Wsrep_thd_args&); wsrep_thd_processor_fun fun_; - void* args_; enum wsrep_thread_type thread_type_; + pthread_t thread_id_; }; void* start_wsrep_THD(void*); diff --git a/sql/wsrep_schema.cc b/sql/wsrep_schema.cc index c1b955e4483..064b6cf9f46 100644 --- a/sql/wsrep_schema.cc +++ b/sql/wsrep_schema.cc @@ -474,7 +474,9 @@ static int scan(TABLE* table, uint field, char* strbuf, uint strbuf_len) { String str; (void)table->field[field]->val_str(&str); - strncpy(strbuf, str.c_ptr(), std::min(str.length(), strbuf_len)); + LEX_CSTRING tmp= str.lex_cstring(); + uint len = tmp.length; + strncpy(strbuf, tmp.str, std::min(len, strbuf_len)); strbuf[strbuf_len - 1]= '\0'; return 0; } diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index 89e637ae075..5602b13f4fd 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -640,7 +640,7 @@ static ssize_t sst_prepare_other (const char* method, const char** addr_out) { bool extra_args; - size_t const cmd_len= estimate_cmd_len(&extra_args); + ssize_t const cmd_len= estimate_cmd_len(&extra_args); wsp::string cmd_str(cmd_len); if (!cmd_str()) @@ -953,7 +953,7 @@ static int sst_donate_mysqldump (const char* addr, memcpy(host, address.get_address(), address.get_address_len()); int port= address.get_port(); bool extra_args; - size_t const cmd_len= estimate_cmd_len(&extra_args); + ssize_t const cmd_len= estimate_cmd_len(&extra_args); wsp::string cmd_str(cmd_len); if (!cmd_str()) @@ -1350,7 +1350,7 @@ static int sst_donate_other (const char* method, char** env) // carries auth info { bool extra_args; - size_t const cmd_len= estimate_cmd_len(&extra_args); + ssize_t const cmd_len= estimate_cmd_len(&extra_args); wsp::string cmd_str(cmd_len); if (!cmd_str()) diff --git a/sql/wsrep_thd.cc b/sql/wsrep_thd.cc index 5907d495ee9..659bb8545b2 100644 --- a/sql/wsrep_thd.cc +++ b/sql/wsrep_thd.cc @@ -86,7 +86,7 @@ static void wsrep_replication_process(THD *thd, static bool create_wsrep_THD(Wsrep_thd_args* args) { ulong old_wsrep_running_threads= wsrep_running_threads; - pthread_t unused; + #ifdef HAVE_PSI_THREAD_INTERFACE PSI_thread_key key; @@ -103,7 +103,7 @@ static bool create_wsrep_THD(Wsrep_thd_args* args) break; } #endif - bool res= mysql_thread_create(key, &unused, &connection_attrib, + bool res= mysql_thread_create(key, args->thread_id(), &connection_attrib, start_wsrep_THD, (void*)args); /* if starting a thread on server startup, wait until the this thread's THD @@ -123,9 +123,9 @@ void wsrep_create_appliers(long threads) /* Dont' start slave threads if wsrep-provider or wsrep-cluster-address is not set. */ - if (!WSREP_PROVIDER_EXISTS) + if (!WSREP_PROVIDER_EXISTS) { - return; + return; } if (!wsrep_cluster_address || wsrep_cluster_address[0]== 0) @@ -135,11 +135,12 @@ void wsrep_create_appliers(long threads) } long wsrep_threads=0; - + while (wsrep_threads++ < threads) { - Wsrep_thd_args* args(new Wsrep_thd_args(wsrep_replication_process, 0, - WSREP_APPLIER_THREAD)); + Wsrep_thd_args* args(new Wsrep_thd_args(wsrep_replication_process, + WSREP_APPLIER_THREAD, + pthread_self())); if (create_wsrep_THD(args)) { WSREP_WARN("Can't create thread to manage wsrep replication"); @@ -328,16 +329,19 @@ void wsrep_create_rollbacker() { if (wsrep_cluster_address && wsrep_cluster_address[0] != 0) { - Wsrep_thd_args* args= new Wsrep_thd_args(wsrep_rollback_process, 0, - WSREP_ROLLBACKER_THREAD); + Wsrep_thd_args* args(new Wsrep_thd_args(wsrep_rollback_process, + WSREP_ROLLBACKER_THREAD, + pthread_self())); /* create rollbacker */ if (create_wsrep_THD(args)) WSREP_WARN("Can't create thread to manage wsrep rollback"); /* create post_rollbacker */ - args= new Wsrep_thd_args(wsrep_post_rollback_process, 0, - WSREP_ROLLBACKER_THREAD); + args= new Wsrep_thd_args(wsrep_post_rollback_process, + WSREP_ROLLBACKER_THREAD, + pthread_self()); + if (create_wsrep_THD(args)) WSREP_WARN("Can't create thread to manage wsrep post rollback"); } -- cgit v1.2.1 From 888f6852261c67a57f21ae80ebb0f3c3f539db7b Mon Sep 17 00:00:00 2001 From: Anel Husakovic Date: Wed, 31 Jul 2019 03:28:38 -0700 Subject: MDEV-20210 If you have an INVISIBLE VIRTUAL column, SHOW CREATE TABLE doesn't list it as INVISIBLE --- sql/sql_show.cc | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'sql') diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 1170aead53c..77eb51f6368 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -2256,6 +2256,10 @@ int show_create_table(THD *thd, TABLE_LIST *table_list, String *packet, packet->append(STRING_WITH_LEN(" STORED")); else packet->append(STRING_WITH_LEN(" VIRTUAL")); + if (field->invisible == INVISIBLE_USER) + { + packet->append(STRING_WITH_LEN(" INVISIBLE")); + } } else { -- cgit v1.2.1 From e8de75db88acdf228237fbebad7f4c8f05e5cc1f Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Wed, 21 Aug 2019 11:37:40 +0300 Subject: MDEV-19740 Debug build of 10.3.15 FTBFS Fix debug build failing with error: extended initializer lists only available with -std=c++11 or -std=gnu++11 --- sql/sql_explain.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/sql_explain.cc b/sql/sql_explain.cc index 8ded81788a2..70b177a556d 100644 --- a/sql/sql_explain.cc +++ b/sql/sql_explain.cc @@ -444,7 +444,8 @@ uint Explain_union::make_union_table_name(char *buf) break; default: DBUG_ASSERT(0); - type= {NULL, 0}; + type.str= NULL; + type.length= 0; } memcpy(buf, type.str, (len= (uint)type.length)); -- cgit v1.2.1 From b96e4424fb4d35dd5de52f44ed6b726a3f0dd010 Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Wed, 21 Aug 2019 16:06:29 +0300 Subject: MDEV-17613 MIN/MAX Optimization (Select tables optimized away) does not work Current easy fix is not possible, because SELECT clones ha_partition and then closes the clone which leads to unclosed transaction in partitions we forcely prune out. We cound solve this by closing these partitions (and release from transaction) in change_partitions_to_open() at versioning conditions stage, but this is problematic because table lock is acquired for each partition at open stage and therefore must be released when we close partition handler in change_partitions_to_open(). More details in MDEV-20376. This should change after MDEV-20250 where mechanism of opening partitions will be improved. This reverts commit cdbac54df0bd857a053decd66b6067abf15a6801. --- sql/sql_select.cc | 38 +++++--------------------------------- 1 file changed, 5 insertions(+), 33 deletions(-) (limited to 'sql') diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 7dcbafd4a88..9d74505cb74 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -721,19 +721,6 @@ void vers_select_conds_t::print(String *str, enum_query_type query_type) const } } -/** - Setup System Versioning conditions - - Add WHERE condition according to FOR SYSTEM_TIME clause. - - If the table is partitioned by SYSTEM_TIME and there is no FOR SYSTEM_TIME - clause, then select now-partition instead of modifying WHERE condition. - - @retval - -1 on error - @retval - 0 on success -*/ int SELECT_LEX::vers_setup_conds(THD *thd, TABLE_LIST *tables) { DBUG_ENTER("SELECT_LEX::vers_setup_cond"); @@ -801,13 +788,12 @@ int SELECT_LEX::vers_setup_conds(THD *thd, TABLE_LIST *tables) vers_select_conds_t &vers_conditions= table->vers_conditions; #ifdef WITH_PARTITION_STORAGE_ENGINE - Vers_part_info *vers_info; - if (table->table->part_info && (vers_info= table->table->part_info->vers_info)) - { - if (table->partition_names) + /* + if the history is stored in partitions, then partitions + themselves are not versioned + */ + if (table->partition_names && table->table->part_info->vers_info) { - /* If the history is stored in partitions, then partitions - themselves are not versioned. */ if (vers_conditions.is_set()) { my_error(ER_VERS_QUERY_IN_PARTITION, MYF(0), table->alias.str); @@ -816,19 +802,6 @@ int SELECT_LEX::vers_setup_conds(THD *thd, TABLE_LIST *tables) else vers_conditions.init(SYSTEM_TIME_ALL); } - else if (!vers_conditions.is_set() && - /* We cannot optimize REPLACE .. SELECT because it may need - to call vers_set_hist_part() to update history. */ - thd->lex->sql_command != SQLCOM_REPLACE_SELECT) - { - table->partition_names= newx List; - String *s= newx String(vers_info->now_part->partition_name, - system_charset_info); - table->partition_names->push_back(s); - table->table->file->change_partitions_to_open(table->partition_names); - vers_conditions.init(SYSTEM_TIME_ALL); - } - } #endif if (outer_table && !vers_conditions.is_set()) @@ -978,7 +951,6 @@ int SELECT_LEX::vers_setup_conds(THD *thd, TABLE_LIST *tables) DBUG_RETURN(0); #undef newx } -#undef newx /***************************************************************************** Check fields, find best join, do the select and output fields. -- cgit v1.2.1 From 235cf969d21ba3406a9325d952fda47c589e58d6 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Thu, 22 Aug 2019 14:17:04 +0400 Subject: MDEV-20397 Support TIMESTAMP, DATETIME, TIME in ROUND() and TRUNCATE() --- sql/item_func.cc | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ sql/item_func.h | 19 ++++++++++++++-- sql/sql_time.cc | 3 ++- sql/sql_time.h | 2 +- sql/sql_type.cc | 41 ++++++++++++++++++++++++++++++++--- sql/sql_type.h | 3 +++ 6 files changed, 127 insertions(+), 7 deletions(-) (limited to 'sql') diff --git a/sql/item_func.cc b/sql/item_func.cc index ce01ef1a686..ced1d69caa9 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -2338,6 +2338,42 @@ void Item_func_round::fix_arg_double() } +void Item_func_round::fix_arg_temporal(const Type_handler *h, + uint int_part_length) +{ + set_handler(h); + if (args[1]->const_item() && !args[1]->is_expensive()) + { + Longlong_hybrid_null dec= args[1]->to_longlong_hybrid_null(); + fix_attributes_temporal(int_part_length, + dec.is_null() ? args[0]->decimals : + dec.to_uint(TIME_SECOND_PART_DIGITS)); + } + else + fix_attributes_temporal(int_part_length, args[0]->decimals); +} + + +void Item_func_round::fix_arg_time() +{ + fix_arg_temporal(&type_handler_time2, MIN_TIME_WIDTH); +} + + +void Item_func_round::fix_arg_datetime() +{ + /* + Day increment operations are not supported for '0000-00-00', + see get_date_from_daynr() for details. Therefore, expressions like + ROUND('0000-00-00 23:59:59.999999') + return NULL. + */ + if (!truncate) + maybe_null= true; + fix_arg_temporal(&type_handler_datetime2, MAX_DATETIME_WIDTH); +} + + void Item_func_round::fix_arg_int() { if (args[1]->const_item()) @@ -2477,6 +2513,36 @@ my_decimal *Item_func_round::decimal_op(my_decimal *decimal_value) } +bool Item_func_round::time_op(THD *thd, MYSQL_TIME *to) +{ + DBUG_ASSERT(args[0]->type_handler()->mysql_timestamp_type() == + MYSQL_TIMESTAMP_TIME); + Time::Options opt(Time::default_flags_for_get_date(), + truncate ? TIME_FRAC_TRUNCATE : TIME_FRAC_ROUND, + Time::DATETIME_TO_TIME_DISALLOW); + Longlong_hybrid_null dec= args[1]->to_longlong_hybrid_null(); + Time *tm= new (to) Time(thd, args[0], opt, + dec.to_uint(TIME_SECOND_PART_DIGITS)); + null_value= !tm->is_valid_time() || dec.is_null(); + DBUG_ASSERT(maybe_null || !null_value); + return null_value; +} + + +bool Item_func_round::date_op(THD *thd, MYSQL_TIME *to, date_mode_t fuzzydate) +{ + DBUG_ASSERT(args[0]->type_handler()->mysql_timestamp_type() == + MYSQL_TIMESTAMP_DATETIME); + Datetime::Options opt(thd, truncate ? TIME_FRAC_TRUNCATE : TIME_FRAC_ROUND); + Longlong_hybrid_null dec= args[1]->to_longlong_hybrid_null(); + Datetime *tm= new (to) Datetime(thd, args[0], opt, + dec.to_uint(TIME_SECOND_PART_DIGITS)); + null_value= !tm->is_valid_datetime() || dec.is_null(); + DBUG_ASSERT(maybe_null || !null_value); + return null_value; +} + + void Item_func_rand::seed_random(Item *arg) { /* diff --git a/sql/item_func.h b/sql/item_func.h index 610adb4bb46..eaa5410dea2 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -1700,21 +1700,36 @@ public: /* This handles round and truncate */ -class Item_func_round :public Item_func_numhybrid +class Item_func_round :public Item_func_hybrid_field_type { bool truncate; void fix_length_and_dec_decimal(uint decimals_to_set); void fix_length_and_dec_double(uint decimals_to_set); public: Item_func_round(THD *thd, Item *a, Item *b, bool trunc_arg) - :Item_func_numhybrid(thd, a, b), truncate(trunc_arg) {} + :Item_func_hybrid_field_type(thd, a, b), truncate(trunc_arg) {} const char *func_name() const { return truncate ? "truncate" : "round"; } double real_op(); longlong int_op(); my_decimal *decimal_op(my_decimal *); + bool date_op(THD *thd, MYSQL_TIME *ltime, date_mode_t fuzzydate); + bool time_op(THD *thd, MYSQL_TIME *ltime); + bool native_op(THD *thd, Native *to) + { + DBUG_ASSERT(0); + return true; + } + String *str_op(String *str) + { + DBUG_ASSERT(0); + return NULL; + } void fix_arg_decimal(); void fix_arg_int(); void fix_arg_double(); + void fix_arg_time(); + void fix_arg_datetime(); + void fix_arg_temporal(const Type_handler *h, uint int_part_length); bool fix_length_and_dec() { return args[0]->type_handler()->Item_func_round_fix_length_and_dec(this); diff --git a/sql/sql_time.cc b/sql/sql_time.cc index c64995fa3d6..b128a7f7291 100644 --- a/sql/sql_time.cc +++ b/sql/sql_time.cc @@ -914,7 +914,7 @@ void make_truncated_value_warning(THD *thd, #define GET_PART(X, N) X % N ## LL; X/= N ## LL bool date_add_interval(THD *thd, MYSQL_TIME *ltime, interval_type int_type, - const INTERVAL &interval) + const INTERVAL &interval, bool push_warn) { long period, sign; @@ -1027,6 +1027,7 @@ bool date_add_interval(THD *thd, MYSQL_TIME *ltime, interval_type int_type, return 0; // Ok invalid_date: + if (push_warn) { push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, ER_DATETIME_FUNCTION_OVERFLOW, diff --git a/sql/sql_time.h b/sql/sql_time.h index 25980d6417c..fe9697adf67 100644 --- a/sql/sql_time.h +++ b/sql/sql_time.h @@ -92,7 +92,7 @@ bool my_TIME_to_str(const MYSQL_TIME *ltime, String *str, uint dec); /* MYSQL_TIME operations */ bool date_add_interval(THD *thd, MYSQL_TIME *ltime, interval_type int_type, - const INTERVAL &interval); + const INTERVAL &interval, bool push_warn= true); bool calc_time_diff(const MYSQL_TIME *l_time1, const MYSQL_TIME *l_time2, int l_sign, ulonglong *seconds_out, ulong *microseconds_out); int append_interval(String *str, interval_type int_type, diff --git a/sql/sql_type.cc b/sql/sql_type.cc index 6653b8b1a3f..8b382773f7a 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -953,10 +953,21 @@ bool Temporal::datetime_add_nanoseconds_or_invalidate(THD *thd, int *warn, ulong INTERVAL interval; memset(&interval, 0, sizeof(interval)); interval.hour= 1; - /* date_add_interval cannot handle bad dates */ - if (check_date(TIME_NO_ZERO_IN_DATE | TIME_NO_ZERO_DATE, warn) || - date_add_interval(thd, this, INTERVAL_HOUR, interval)) + /* + date_add_interval cannot handle bad dates with zero YYYY or MM. + Note, check_date(NO_ZERO_XX) does not check YYYY against zero, + so let's additionally check it. + */ + if (year == 0 || + check_date(TIME_NO_ZERO_IN_DATE | TIME_NO_ZERO_DATE, warn) || + date_add_interval(thd, this, INTERVAL_HOUR, interval, false/*no warn*/)) { + char buf[MAX_DATE_STRING_REP_LENGTH]; + my_date_to_str(this, buf); + push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, + ER_WRONG_VALUE_FOR_TYPE, + ER_THD(thd, ER_WRONG_VALUE_FOR_TYPE), + "date", buf, "round(datetime)"); make_from_out_of_range(warn); return true; } @@ -5620,6 +5631,30 @@ bool Type_handler_temporal_result:: } +bool Type_handler_time_common:: + Item_func_round_fix_length_and_dec(Item_func_round *item) const +{ + item->fix_arg_time(); + return false; +} + + +bool Type_handler_datetime_common:: + Item_func_round_fix_length_and_dec(Item_func_round *item) const +{ + item->fix_arg_datetime(); + return false; +} + + +bool Type_handler_timestamp_common:: + Item_func_round_fix_length_and_dec(Item_func_round *item) const +{ + item->fix_arg_datetime(); + return false; +} + + bool Type_handler_string_result:: Item_func_round_fix_length_and_dec(Item_func_round *item) const { diff --git a/sql/sql_type.h b/sql/sql_type.h index 0f270ce9d5d..4b87d6cc989 100644 --- a/sql/sql_type.h +++ b/sql/sql_type.h @@ -5281,6 +5281,7 @@ public: bool Item_func_min_max_get_date(THD *thd, Item_func_min_max*, MYSQL_TIME *, date_mode_t fuzzydate) const; longlong Item_func_between_val_int(Item_func_between *func) const; + bool Item_func_round_fix_length_and_dec(Item_func_round *) const; Item *make_const_item_for_comparison(THD *, Item *src, const Item *cmp) const; bool set_comparator_func(Arg_comparator *cmp) const; cmp_item *make_cmp_item(THD *thd, CHARSET_INFO *cs) const; @@ -5507,6 +5508,7 @@ public: longlong Item_func_min_max_val_int(Item_func_min_max *) const; my_decimal *Item_func_min_max_val_decimal(Item_func_min_max *, my_decimal *) const; + bool Item_func_round_fix_length_and_dec(Item_func_round *) const; bool Item_hybrid_func_fix_attributes(THD *thd, const char *name, Type_handler_hybrid_field_type *, @@ -5608,6 +5610,7 @@ public: bool Item_param_val_native(THD *thd, Item_param *item, Native *to) const; int cmp_native(const Native &a, const Native &b) const; longlong Item_func_between_val_int(Item_func_between *func) const; + bool Item_func_round_fix_length_and_dec(Item_func_round *) const; cmp_item *make_cmp_item(THD *thd, CHARSET_INFO *cs) const; in_vector *make_in_vector(THD *thd, const Item_func_in *f, uint nargs) const; void make_sort_key(uchar *to, Item *item, const SORT_FIELD_ATTR *sort_field, -- cgit v1.2.1 From 9fce94287827eee4c1bcddd70a4aacff96314871 Mon Sep 17 00:00:00 2001 From: Roman Nozdrin Date: Sat, 24 Aug 2019 20:47:24 +0300 Subject: MDEV-19831 find_select_handler() now tries its best to find a handlerton that is able to processes the whole query. For that it traverses tables from subqueries. Select_handler now cleans up temporary table structures on dctor call. --- sql/select_handler.cc | 2 ++ sql/sql_select.cc | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/select_handler.cc b/sql/select_handler.cc index f020d2f6b80..b364cb12341 100644 --- a/sql/select_handler.cc +++ b/sql/select_handler.cc @@ -45,6 +45,8 @@ Pushdown_select::Pushdown_select(SELECT_LEX *sel, select_handler *h) Pushdown_select::~Pushdown_select() { + if (handler->table) + free_tmp_table(handler->thd, handler->table); delete handler; select->select_h= NULL; } diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 555dfcf907d..150e8008096 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -28526,7 +28526,7 @@ select_handler *SELECT_LEX::find_select_handler(THD *thd) return 0; if (master_unit()->outer_select()) return 0; - for (TABLE_LIST *tbl= join->tables_list; tbl; tbl= tbl->next_local) + for (TABLE_LIST *tbl= join->tables_list; tbl; tbl= tbl->next_global) { if (!tbl->table) continue; -- cgit v1.2.1 From 4a490d1a993959222deb8bae822f40575586a148 Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Sun, 25 Aug 2019 11:03:19 +0300 Subject: MDEV-6111: Optimizer Trace: add tracing for semi-join optimizations Added: - "semijoin_strategy_choice" element (actions in advance_sj_state(), name matches the name in MySQL) - semijoin_table_pullout element. --- sql/opt_subselect.cc | 101 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 84 insertions(+), 17 deletions(-) (limited to 'sql') diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc index 599642b3a26..9205380bd19 100644 --- a/sql/opt_subselect.cc +++ b/sql/opt_subselect.cc @@ -2192,12 +2192,15 @@ int pull_out_semijoin_tables(JOIN *join) TABLE_LIST *sj_nest; DBUG_ENTER("pull_out_semijoin_tables"); List_iterator sj_list_it(join->select_lex->sj_nests); - + /* Try pulling out of the each of the semi-joins */ while ((sj_nest= sj_list_it++)) { List_iterator child_li(sj_nest->nested_join->join_list); TABLE_LIST *tbl; + Json_writer_object trace_wrapper(join->thd); + Json_writer_object trace(join->thd, "semijoin_table_pullout"); + Json_writer_array trace_arr(join->thd, "pulled_out_tables"); /* Don't do table pull-out for nested joins (if we get nested joins here, it @@ -2296,7 +2299,8 @@ int pull_out_semijoin_tables(JOIN *join) pulled_a_table= TRUE; pulled_tables |= tbl->table->map; DBUG_PRINT("info", ("Table %s pulled out (reason: func dep)", - tbl->table->alias.c_ptr())); + tbl->table->alias.c_ptr_safe())); + trace_arr.add(tbl->table->alias.c_ptr_safe()); /* Pulling a table out of uncorrelated subquery in general makes makes it correlated. See the NOTE to this funtion. @@ -2778,27 +2782,30 @@ void advance_sj_state(JOIN *join, table_map remaining_tables, uint idx, { POSITION *pos= join->positions + idx; const JOIN_TAB *new_join_tab= pos->table; - Semi_join_strategy_picker *pickers[]= - { - &pos->firstmatch_picker, - &pos->loosescan_picker, - &pos->sjmat_picker, - &pos->dups_weedout_picker, - NULL, - }; - - if (join->emb_sjm_nest) + if (join->emb_sjm_nest || //(1) + !join->select_lex->have_merged_subqueries) //(2) { /* - We're performing optimization inside SJ-Materialization nest: + (1): We're performing optimization inside SJ-Materialization nest: - there are no other semi-joins inside semi-join nests - attempts to build semi-join strategies here will confuse the optimizer, so bail out. + (2): Don't waste time on semi-join optimizations if we don't have any + semi-joins */ pos->sj_strategy= SJ_OPT_NONE; return; } + Semi_join_strategy_picker *pickers[]= + { + &pos->firstmatch_picker, + &pos->loosescan_picker, + &pos->sjmat_picker, + &pos->dups_weedout_picker, + NULL, + }; + Json_writer_array trace_steps(join->thd, "semijoin_strategy_choice"); /* Update join->cur_sj_inner_tables (Used by FirstMatch in this function and LooseScan detector in best_access_path) @@ -2897,6 +2904,7 @@ void advance_sj_state(JOIN *join, table_map remaining_tables, uint idx, *current_read_time= read_time; *current_record_count= rec_count; dups_producing_tables &= ~handled_fanout; + //TODO: update bitmap of semi-joins that were handled together with // others. if (is_multiple_semi_joins(join, join->positions, idx, @@ -2924,6 +2932,30 @@ void advance_sj_state(JOIN *join, table_map remaining_tables, uint idx, } } } + + if (unlikely(join->thd->trace_started() && pos->sj_strategy != SJ_OPT_NONE)) + { + Json_writer_object tr(join->thd); + const char *sname; + switch (pos->sj_strategy) { + case SJ_OPT_MATERIALIZE: + sname= "SJ-Materialize"; + break; + case SJ_OPT_MATERIALIZE_SCAN: + sname= "SJ-Materialize-Scan"; + break; + case SJ_OPT_FIRST_MATCH: + sname= "FirstMatch"; + break; + case SJ_OPT_DUPS_WEEDOUT: + sname= "DuplicateWeedout"; + break; + default: + DBUG_ASSERT(0); + sname="Invalid"; + } + tr.add("chosen_strategy", sname); + } } if ((emb_sj_nest= new_join_tab->emb_sj_nest)) @@ -3000,6 +3032,8 @@ bool Sj_materialization_picker::check_qep(JOIN *join, } else { + Json_writer_object trace(join->thd); + trace.add("strategy", "SJ-Materialization"); /* This is SJ-Materialization with lookups */ Cost_estimate prefix_cost; signed int first_tab= (int)idx - mat_info->tables; @@ -3032,6 +3066,11 @@ bool Sj_materialization_picker::check_qep(JOIN *join, *record_count= prefix_rec_count; *handled_fanout= new_join_tab->emb_sj_nest->sj_inner_tables; *strategy= SJ_OPT_MATERIALIZE; + if (unlikely(join->thd->trace_started())) + { + trace.add("records", *record_count); + trace.add("read_time", *read_time); + } return TRUE; } } @@ -3040,6 +3079,8 @@ bool Sj_materialization_picker::check_qep(JOIN *join, if (sjm_scan_need_tables && /* Have SJM-Scan prefix */ !(sjm_scan_need_tables & remaining_tables)) { + Json_writer_object trace(join->thd); + trace.add("strategy", "SJ-Materialization-Scan"); TABLE_LIST *mat_nest= join->positions[sjm_scan_last_inner].table->emb_sj_nest; SJ_MATERIALIZATION_INFO *mat_info= mat_nest->sj_mat_info; @@ -3088,6 +3129,11 @@ bool Sj_materialization_picker::check_qep(JOIN *join, *read_time= prefix_cost; *record_count= prefix_rec_count; *handled_fanout= mat_nest->sj_inner_tables; + if (unlikely(join->thd->trace_started())) + { + trace.add("records", *record_count); + trace.add("read_time", *read_time); + } return TRUE; } return FALSE; @@ -3151,6 +3197,8 @@ bool LooseScan_picker::check_qep(JOIN *join, !(remaining_tables & loosescan_need_tables) && (new_join_tab->table->map & loosescan_need_tables)) { + Json_writer_object trace(join->thd); + trace.add("strategy", "SJ-Materialization-Scan"); /* Ok we have LooseScan plan and also have all LooseScan sj-nest's inner tables and outer correlated tables into the prefix. @@ -3181,6 +3229,11 @@ bool LooseScan_picker::check_qep(JOIN *join, */ *strategy= SJ_OPT_LOOSE_SCAN; *handled_fanout= first->table->emb_sj_nest->sj_inner_tables; + if (unlikely(join->thd->trace_started())) + { + trace.add("records", *record_count); + trace.add("read_time", *read_time); + } return TRUE; } return FALSE; @@ -3260,6 +3313,8 @@ bool Firstmatch_picker::check_qep(JOIN *join, if (in_firstmatch_prefix() && !(firstmatch_need_tables & remaining_tables)) { + Json_writer_object trace(join->thd); + trace.add("strategy", "FirstMatch"); /* Got a complete FirstMatch range. Calculate correct costs and fanout */ @@ -3292,6 +3347,11 @@ bool Firstmatch_picker::check_qep(JOIN *join, *handled_fanout= firstmatch_need_tables; /* *record_count and *read_time were set by the above call */ *strategy= SJ_OPT_FIRST_MATCH; + if (unlikely(join->thd->trace_started())) + { + trace.add("records", *record_count); + trace.add("read_time", *read_time); + } return TRUE; } } @@ -3370,6 +3430,8 @@ bool Duplicate_weedout_picker::check_qep(JOIN *join, double sj_inner_fanout= 1.0; double sj_outer_fanout= 1.0; uint temptable_rec_size; + Json_writer_object trace(join->thd); + trace.add("strategy", "DuplicateWeedout"); if (first_tab == join->const_tables) { prefix_rec_count= 1.0; @@ -3430,6 +3492,11 @@ bool Duplicate_weedout_picker::check_qep(JOIN *join, *record_count= prefix_rec_count * sj_outer_fanout; *handled_fanout= dups_removed_fanout; *strategy= SJ_OPT_DUPS_WEEDOUT; + if (unlikely(join->thd->trace_started())) + { + trace.add("records", *record_count); + trace.add("read_time", *read_time); + } return TRUE; } return FALSE; @@ -3660,7 +3727,7 @@ void fix_semijoin_strategies_for_picked_join_order(JOIN *join) join->best_positions[first].n_sj_tables= sjm->tables; join->best_positions[first].sj_strategy= SJ_OPT_MATERIALIZE; Json_writer_object semijoin_strategy(thd); - semijoin_strategy.add("semi_join_strategy","sj_materialize"); + semijoin_strategy.add("semi_join_strategy","SJ-Materialization"); Json_writer_array semijoin_plan(thd, "join_order"); for (uint i= first; i < first+ sjm->tables; i++) { @@ -3709,7 +3776,7 @@ void fix_semijoin_strategies_for_picked_join_order(JOIN *join) POSITION dummy; join->cur_sj_inner_tables= 0; Json_writer_object semijoin_strategy(thd); - semijoin_strategy.add("semi_join_strategy","sj_materialize_scan"); + semijoin_strategy.add("semi_join_strategy","SJ-Materialization-Scan"); Json_writer_array semijoin_plan(thd, "join_order"); for (i= first + sjm->tables; i <= tablenr; i++) { @@ -3747,7 +3814,7 @@ void fix_semijoin_strategies_for_picked_join_order(JOIN *join) */ join->cur_sj_inner_tables= 0; Json_writer_object semijoin_strategy(thd); - semijoin_strategy.add("semi_join_strategy","firstmatch"); + semijoin_strategy.add("semi_join_strategy","FirstMatch"); Json_writer_array semijoin_plan(thd, "join_order"); for (idx= first; idx <= tablenr; idx++) { @@ -3785,7 +3852,7 @@ void fix_semijoin_strategies_for_picked_join_order(JOIN *join) */ join->cur_sj_inner_tables= 0; Json_writer_object semijoin_strategy(thd); - semijoin_strategy.add("semi_join_strategy","sj_materialize"); + semijoin_strategy.add("semi_join_strategy","LooseScan"); Json_writer_array semijoin_plan(thd, "join_order"); for (idx= first; idx <= tablenr; idx++) { -- cgit v1.2.1 From de0f93fb0d7b03aaf293cc89b611aeff3ce3244e Mon Sep 17 00:00:00 2001 From: Julius Goryavsky Date: Mon, 26 Aug 2019 13:37:09 +0200 Subject: MDEV-20420: SST failed after MDEV-18863 in some test configurations After applying MDEV-18863, in some test configurations, SST may fails due to duplication of some parameters (in particular "--port") in the main part of the command line and after "--mysqld-args", as well as due to incorrect interpretation of the parameter "--port" passed after "--mysqld-args" when the SST script is invoked without explicitly specifying a port for SST. In addition, it is necessary to correctly handle spaces, quotation marks and special characters when copying original arguments from the argv[] array to a new command line (after "--mysqld-args"). This patch resolves these shortcomings. --- sql/wsrep_sst.cc | 308 ++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 285 insertions(+), 23 deletions(-) (limited to 'sql') diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index fb1efa1d451..af4e02b9e55 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -16,6 +16,7 @@ #include "wsrep_sst.h" #include +#include #include #include #include @@ -379,7 +380,31 @@ static char* my_fgets (char* buf, size_t buf_len, FILE* stream) } /* - Generate opt_binlog_opt_val for sst_donate_other(), sst_prepare_other(). + Generate "name 'value'" string. +*/ +static char* generate_name_value(const char* name, const char* value) +{ + size_t name_len= strlen(name); + size_t value_len= strlen(value); + char* buf= + (char*) my_malloc((name_len + value_len + 5) * sizeof(char), MYF(0)); + if (buf) + { + char* ref= buf; + *ref++ = ' '; + memcpy(ref, name, name_len * sizeof(char)); + ref += name_len; + *ref++ = ' '; + *ref++ = '\''; + memcpy(ref, value, value_len * sizeof(char)); + ref += value_len; + *ref++ = '\''; + *ref = 0; + } + return buf; +} +/* + Generate binlog option string for sst_donate_other(), sst_prepare_other(). Returns zero on success, negative error code otherwise. @@ -395,7 +420,9 @@ static int generate_binlog_opt_val(char** ret) { assert(opt_bin_logname); *ret= strcmp(opt_bin_logname, "0") ? - my_strdup(opt_bin_logname, MYF(0)) : my_strdup("", MYF(0)); + generate_name_value(WSREP_SST_OPT_BINLOG, + opt_bin_logname) : + my_strdup("", MYF(0)); } else { @@ -411,7 +438,9 @@ static int generate_binlog_index_opt_val(char** ret) *ret= NULL; if (opt_binlog_index_name) { *ret= strcmp(opt_binlog_index_name, "0") ? - my_strdup(opt_binlog_index_name, MYF(0)) : my_strdup("", MYF(0)); + generate_name_value(WSREP_SST_OPT_BINLOG_INDEX, + opt_binlog_index_name) : + my_strdup("", MYF(0)); } else { @@ -608,7 +637,86 @@ static size_t estimate_cmd_len (bool* extra_args) { for (int i = 1; i < orig_argc; i++) { - cmd_len += strlen(orig_argv[i]); + const char* arg= orig_argv[i]; + size_t n= strlen(arg); + if (n == 0) continue; + cmd_len += n; + bool quotation= false; + char c; + while ((c = *arg++) != 0) + { + /* A whitespace or a single quote requires double quotation marks: */ + if (isspace(c) || c == '\'') + { + quotation= true; + } + /* + If the equals symbol is encountered, then we need to separately + process the right side: + */ + else if (c == '=') + { + /* Perhaps we need to quote the left part of the argument: */ + if (quotation) + { + cmd_len += 2; + /* + Reset the quotation flag, since now the status for + the right side of the expression will be saved here: + */ + quotation= false; + } + while ((c = *arg++) != 0) + { + /* + A whitespace or a single quote requires double + quotation marks: + */ + if (isspace(c) || c == '\'') + { + quotation= true; + } + /* + Double quotation mark or backslash symbol requires backslash + prefixing: + */ +#ifdef __WIN__ + else if (c == '"' || c == '\\') +#else + /* + The dollar symbol is used to substitute a variable, therefore + it also requires escaping: + */ + else if (c == '"' || c == '\\' || c == '$') +#endif + { + cmd_len++; + } + } + break; + } + /* + Double quotation mark or backslash symbol requires backslash + prefixing: + */ +#ifdef __WIN__ + else if (c == '"' || c == '\\') +#else + /* + The dollar symbol is used to substitute a variable, therefore + it also requires escaping: + */ + else if (c == '"' || c == '\\' || c == '$') +#endif + { + cmd_len++; + } + } + /* Perhaps we need to quote the entire argument or its right part: */ + if (quotation) + { + cmd_len += 2; + } } extra = true; cmd_len += strlen(WSREP_SST_OPT_MYSQLD); @@ -636,10 +744,171 @@ static void copy_orig_argv (char* cmd_str) for (int i = 1; i < orig_argc; i++) { char* arg= orig_argv[i]; - *cmd_str++ = ' '; n = strlen(arg); - memcpy(cmd_str, arg, n * sizeof(char)); - cmd_str += n; + if (n == 0) continue; + *cmd_str++ = ' '; + bool quotation= false; + bool plain= true; + char *arg_scan= arg; + char c; + while ((c = *arg_scan++) != 0) + { + /* A whitespace or a single quote requires double quotation marks: */ + if (isspace(c) || c == '\'') + { + quotation= true; + } + /* + If the equals symbol is encountered, then we need to separately + process the right side: + */ + else if (c == '=') + { + /* Calculate length of the Left part of the argument: */ + size_t m = (size_t) (arg_scan - arg) - 1; + if (m) + { + /* Perhaps we need to quote the left part of the argument: */ + if (quotation) + { + *cmd_str++ = '"'; + } + /* + If there were special characters inside, then we can use + the fast memcpy function: + */ + if (plain) + { + memcpy(cmd_str, arg, m * sizeof(char)); + cmd_str += m; + /* Left part of the argument has already been processed: */ + n -= m; + arg += m; + } + /* Otherwise we need to prefix individual characters: */ + else + { + n -= m; + while (m) + { + c = *arg++; +#ifdef __WIN__ + if (c == '"' || c == '\\') +#else + if (c == '"' || c == '\\' || c == '$') +#endif + { + *cmd_str++ = '\\'; + } + *cmd_str++ = c; + m--; + } + /* + Reset the plain string flag, since now the status for + the right side of the expression will be saved here: + */ + plain= true; + } + /* Perhaps we need to quote the left part of the argument: */ + if (quotation) + { + *cmd_str++ = '"'; + /* + Reset the quotation flag, since now the status for + the right side of the expression will be saved here: + */ + quotation= false; + } + } + /* Copy equals symbol: */ + *cmd_str++ = '='; + arg++; + n--; + /* Let's deal with the left side of the expression: */ + while ((c = *arg_scan++) != 0) + { + /* + A whitespace or a single quote requires double + quotation marks: + */ + if (isspace(c) || c == '\'') + { + quotation= true; + } + /* + Double quotation mark or backslash symbol requires backslash + prefixing: + */ +#ifdef __WIN__ + else if (c == '"' || c == '\\') +#else + /* + The dollar symbol is used to substitute a variable, therefore + it also requires escaping: + */ + else if (c == '"' || c == '\\' || c == '$') +#endif + { + plain= false; + } + } + break; + } + /* + Double quotation mark or backslash symbol requires backslash + prefixing: + */ +#ifdef __WIN__ + else if (c == '"' || c == '\\') +#else + /* + The dollar symbol is used to substitute a variable, therefore + it also requires escaping: + */ + else if (c == '"' || c == '\\' || c == '$') +#endif + { + plain= false; + } + } + if (n) + { + /* Perhaps we need to quote the entire argument or its right part: */ + if (quotation) + { + *cmd_str++ = '"'; + } + /* + If there were no special characters inside, then we can use + the fast memcpy function: + */ + if (plain) + { + memcpy(cmd_str, arg, n * sizeof(char)); + cmd_str += n; + } + /* Otherwise we need to prefix individual characters: */ + else + { + while ((c = *arg++) != 0) + { +#ifdef __WIN__ + if (c == '"' || c == '\\') +#else + if (c == '"' || c == '\\' || c == '$') +#endif + { + *cmd_str++ = '\\'; + } + *cmd_str++ = c; + } + } + /* Perhaps we need to quote the entire argument or its right part: */ + if (quotation) + { + *cmd_str++ = '"'; + } + } } /* Add a terminating null character (not counted in the length, @@ -666,8 +935,6 @@ static ssize_t sst_prepare_other (const char* method, return -ENOMEM; } - const char* binlog_opt= ""; - const char* binlog_index_opt= ""; char* binlog_opt_val= NULL; char* binlog_index_opt_val= NULL; @@ -685,9 +952,6 @@ static ssize_t sst_prepare_other (const char* method, ret); } - if (strlen(binlog_opt_val)) binlog_opt= WSREP_SST_OPT_BINLOG; - if (strlen(binlog_index_opt_val)) binlog_index_opt= WSREP_SST_OPT_BINLOG_INDEX; - make_wsrep_defaults_file(); ret= snprintf (cmd_str(), cmd_len, @@ -695,14 +959,14 @@ static ssize_t sst_prepare_other (const char* method, WSREP_SST_OPT_ROLE " 'joiner' " WSREP_SST_OPT_ADDR " '%s' " WSREP_SST_OPT_DATA " '%s' " - " %s " + "%s" WSREP_SST_OPT_PARENT " '%d'" - " %s '%s'" - " %s '%s'", + "%s" + "%s", method, addr_in, mysql_real_data_home, wsrep_defaults_file, - (int)getpid(), binlog_opt, binlog_opt_val, - binlog_index_opt, binlog_index_opt_val); + (int)getpid(), + binlog_opt_val, binlog_index_opt_val); my_free(binlog_opt_val); my_free(binlog_index_opt_val); @@ -999,7 +1263,7 @@ static int sst_donate_mysqldump (const char* addr, WSREP_SST_OPT_PORT " '%d' " WSREP_SST_OPT_LPORT " '%u' " WSREP_SST_OPT_SOCKET " '%s' " - " %s " + "%s" WSREP_SST_OPT_GTID " '%s:%lld' " WSREP_SST_OPT_GTID_DOMAIN_ID " '%d'" "%s", @@ -1347,7 +1611,6 @@ static int sst_donate_other (const char* method, return -ENOMEM; } - const char* binlog_opt= ""; char* binlog_opt_val= NULL; int ret; @@ -1356,7 +1619,6 @@ static int sst_donate_other (const char* method, WSREP_ERROR("sst_donate_other(): generate_binlog_opt_val() failed: %d",ret); return ret; } - if (strlen(binlog_opt_val)) binlog_opt= WSREP_SST_OPT_BINLOG; make_wsrep_defaults_file(); @@ -1366,15 +1628,15 @@ static int sst_donate_other (const char* method, WSREP_SST_OPT_ADDR " '%s' " WSREP_SST_OPT_SOCKET " '%s' " WSREP_SST_OPT_DATA " '%s' " - " %s " - " %s '%s' " + "%s" WSREP_SST_OPT_GTID " '%s:%lld' " WSREP_SST_OPT_GTID_DOMAIN_ID " '%d'" + "%s" "%s", method, addr, mysqld_unix_port, mysql_real_data_home, wsrep_defaults_file, - binlog_opt, binlog_opt_val, uuid, (long long) seqno, wsrep_gtid_domain_id, + binlog_opt_val, bypass ? " " WSREP_SST_OPT_BYPASS : ""); my_free(binlog_opt_val); -- cgit v1.2.1 From 72c5a8d39bff8920ccc868390e5329dfa5af33a5 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Mon, 26 Aug 2019 23:42:06 +0400 Subject: MDEV-20417 Assertion `(m_ptr == __null) == item->null_value' failed in VDec::VDec(Item*) --- sql/sql_type.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sql') diff --git a/sql/sql_type.h b/sql/sql_type.h index 4b87d6cc989..0d6f7901480 100644 --- a/sql/sql_type.h +++ b/sql/sql_type.h @@ -2439,7 +2439,7 @@ public: Datetime to_datetime(THD *thd) const { if (is_zero_datetime()) - return Datetime(); + return Datetime::zero(); return Timestamp::to_datetime(thd); } bool is_zero_datetime() const { return m_is_zero_datetime; } @@ -2487,7 +2487,7 @@ public: Datetime to_datetime(THD *thd) const { return is_zero_datetime() ? - Datetime() : + Datetime::zero() : Datetime(thd, Timestamp(*this).tv()); } bool is_zero_datetime() const -- cgit v1.2.1 From eb8f7005bdcf3ee3e2db2cc5bc8e9fde42122cef Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sun, 28 Jul 2019 00:04:57 +0200 Subject: cleanup: remove few #ifdef's --- sql/handler.cc | 48 ++++++++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 28 deletions(-) (limited to 'sql') diff --git a/sql/handler.cc b/sql/handler.cc index c4f5febbdcf..647848f7702 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -2048,9 +2048,9 @@ static char* xid_to_str(char *buf, XID *xid) } #endif -#ifdef WITH_WSREP static my_xid wsrep_order_and_check_continuity(XID *list, int len) { +#ifdef WITH_WSREP wsrep_sort_xid_array(list, len); wsrep::gtid cur_position= wsrep_get_SE_checkpoint(); long long cur_seqno= cur_position.seqno().get(); @@ -2068,8 +2068,10 @@ static my_xid wsrep_order_and_check_continuity(XID *list, int len) } WSREP_INFO("Last wsrep seqno to be recovered %lld", cur_seqno); return (cur_seqno < 0 ? 0 : cur_seqno); -} +#else + return 0; #endif /* WITH_WSREP */ +} /** recover() step of xa. @@ -2107,7 +2109,6 @@ static my_bool xarecover_handlerton(THD *unused, plugin_ref plugin, { sql_print_information("Found %d prepared transaction(s) in %s", got, hton_name(hton)->str); -#ifdef WITH_WSREP /* If wsrep_on=ON, XIDs are first ordered and then the range of recovered XIDs is checked for continuity. All the XIDs which are in continuous range can be safely committed if binlog @@ -2123,12 +2124,10 @@ static my_bool xarecover_handlerton(THD *unused, plugin_ref plugin, crashes after T2 finishes prepare step but before T1 starts the prepare. */ - my_xid wsrep_limit= 0; + my_xid wsrep_limit __attribute__((unused))= 0; if (WSREP_ON) - { wsrep_limit= wsrep_order_and_check_continuity(info->list, got); - } -#endif /* WITH_WSREP */ + for (int i=0; i < got; i ++) { my_xid x= IF_WSREP(WSREP_ON && wsrep_is_wsrep_xid(&info->list[i]) ? @@ -2137,10 +2136,10 @@ static my_bool xarecover_handlerton(THD *unused, plugin_ref plugin, info->list[i].get_my_xid()); if (!x) // not "mine" - that is generated by external TM { -#ifndef DBUG_OFF - char buf[XIDDATASIZE*4+6]; // see xid_to_str - DBUG_PRINT("info", ("ignore xid %s", xid_to_str(buf, info->list+i))); -#endif + DBUG_EXECUTE("info",{ + char buf[XIDDATASIZE*4+6]; + _db_doprnt_("ignore xid %s", xid_to_str(buf, info->list+i)); + }); xid_cache_insert(info->list + i); info->found_foreign_xids++; continue; @@ -2161,32 +2160,25 @@ static my_bool xarecover_handlerton(THD *unused, plugin_ref plugin, my_hash_search(info->commit_list, (uchar *)&x, sizeof(x)) != 0 : tc_heuristic_recover == TC_HEURISTIC_RECOVER_COMMIT)) { -#ifndef DBUG_OFF - int rc= -#endif - hton->commit_by_xid(hton, info->list+i); -#ifndef DBUG_OFF + int rc= hton->commit_by_xid(hton, info->list+i); if (rc == 0) { - char buf[XIDDATASIZE*4+6]; // see xid_to_str - DBUG_PRINT("info", ("commit xid %s", xid_to_str(buf, info->list+i))); + DBUG_EXECUTE("info",{ + char buf[XIDDATASIZE*4+6]; + _db_doprnt_("commit xid %s", xid_to_str(buf, info->list+i)); + }); } -#endif } else { -#ifndef DBUG_OFF - int rc= -#endif - hton->rollback_by_xid(hton, info->list+i); -#ifndef DBUG_OFF + int rc= hton->rollback_by_xid(hton, info->list+i); if (rc == 0) { - char buf[XIDDATASIZE*4+6]; // see xid_to_str - DBUG_PRINT("info", ("rollback xid %s", - xid_to_str(buf, info->list+i))); + DBUG_EXECUTE("info",{ + char buf[XIDDATASIZE*4+6]; + _db_doprnt_("rollback xid %s", xid_to_str(buf, info->list+i)); + }); } -#endif } } if (got < info->len) -- cgit v1.2.1 From 77e44282ff8628975298f74796d19f0307f34dc8 Mon Sep 17 00:00:00 2001 From: Varun Gupta Date: Mon, 26 Aug 2019 20:59:51 +0530 Subject: MDEV-19705: Assertion `tmp >= 0' failed in best_access_path The reason for hitting the assert is that rec_per_key estimates have some garbage value. So the solution to fix this would be for long unique keys to use use rec_per_key for only 1 keypart, that means rec_per_key[0] would have the estimate. --- sql/table.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/table.cc b/sql/table.cc index 1ab4df0f7cf..da0c4be41b0 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -800,7 +800,8 @@ static bool create_key_infos(const uchar *strpos, const uchar *frm_image_end, { if (strpos + (new_frm_ver >= 1 ? 9 : 7) >= frm_image_end) return 1; - *rec_per_key++=0; + if (!(keyinfo->algorithm == HA_KEY_ALG_LONG_HASH)) + *rec_per_key++=0; key_part->fieldnr= (uint16) (uint2korr(strpos) & FIELD_NR_MASK); key_part->offset= (uint) uint2korr(strpos+2)-1; key_part->key_type= (uint) uint2korr(strpos+5); @@ -828,6 +829,7 @@ static bool create_key_infos(const uchar *strpos, const uchar *frm_image_end, { keyinfo->key_length= HA_HASH_KEY_LENGTH_WITHOUT_NULL; key_part++; // reserved for the hash value + *rec_per_key++=0; } /* -- cgit v1.2.1 From 29bbf4749ebcfe328d73ca93c7ee5908a339e850 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Tue, 27 Aug 2019 09:13:20 +0400 Subject: MDEV-19699 Server crashes in Item_null_result::field_type upon SELECT with ROLLUP on constant table Also fixes: MDEV-20431 GREATEST(int_col,date_col) returns wrong results in a view --- sql/item.cc | 32 ++++++++++++++++++++++++++++++++ sql/item.h | 4 ++++ 2 files changed, 36 insertions(+) (limited to 'sql') diff --git a/sql/item.cc b/sql/item.cc index de7e6ae65ff..86bd4670714 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -3131,6 +3131,20 @@ my_decimal *Item_null::val_decimal(my_decimal *decimal_value) } +longlong Item_null::val_datetime_packed() +{ + null_value= true; + return 0; +} + + +longlong Item_null::val_time_packed() +{ + null_value= true; + return 0; +} + + bool Item_null::get_date(MYSQL_TIME *ltime, ulonglong fuzzydate) { // following assert is redundant, because fixed=1 assigned in constructor @@ -7296,6 +7310,24 @@ bool Item_ref::get_date(MYSQL_TIME *ltime,ulonglong fuzzydate) } +longlong Item_ref::val_datetime_packed() +{ + DBUG_ASSERT(fixed); + longlong tmp= (*ref)->val_datetime_packed(); + null_value= (*ref)->null_value; + return tmp; +} + + +longlong Item_ref::val_time_packed() +{ + DBUG_ASSERT(fixed); + longlong tmp= (*ref)->val_time_packed(); + null_value= (*ref)->null_value; + return tmp; +} + + my_decimal *Item_ref::val_decimal(my_decimal *decimal_value) { my_decimal *val= (*ref)->val_decimal_result(decimal_value); diff --git a/sql/item.h b/sql/item.h index edf9f748d77..a182d99f701 100644 --- a/sql/item.h +++ b/sql/item.h @@ -2627,6 +2627,8 @@ public: String *val_str(String *str); my_decimal *val_decimal(my_decimal *); bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate); + longlong val_datetime_packed(); + longlong val_time_packed(); int save_in_field(Field *field, bool no_conversions); int save_safe_in_field(Field *field); bool send(Protocol *protocol, String *str); @@ -3971,6 +3973,8 @@ public: String *val_str(String* tmp); bool is_null(); bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate); + longlong val_datetime_packed(); + longlong val_time_packed(); double val_result(); longlong val_int_result(); String *str_result(String* tmp); -- cgit v1.2.1 From e7b71e0daa49ca9b5becbf2739ed13d7c8c4fcd6 Mon Sep 17 00:00:00 2001 From: Sujatha Date: Tue, 27 Aug 2019 13:05:04 +0530 Subject: MDEV-19925: Column ... cannot be converted from type 'varchar(20)' to type 'varchar(20)' Cherry picking: Bug#25135304: RBR: WRONG FIELD LENGTH IN ERROR MESSAGE commit 47bd3f7cf3c8518f62b1580ec65af2ba7ac13b95 Description: ============ In row based replication, when replicating from a table with a field with character set set to UTF8mb3 to the same table with the same field set to character set UTF8mb4 I get a confusing error message: For VARCHAR: VARCHAR(1) 'utf8mb3' to VARCHAR(1) 'utf8mb4' "Column 0 of table 'test.t1' cannot be converted from type 'varchar(3)' to type 'varchar(1)'" Similar issue with CHAR type as well. Issue with respect to BLOB types: For BLOB: LONGBLOB to TINYBLOB - Error message displays incorrect blob type. "Column 0 of table 'test.t1' cannot be converted from type 'tinyblob' to type 'tinyblob'" For BINARY to BINARY - Error message displays incorrect type for master side field. "Column 0 of table 'test.t' cannot be converted from type 'char(1)' to type 'binary(10)'" Similar issue exists for VARBINARY type. It is displayed as 'VARCHAR'. Analysis: ========= In Row based replication charset information is not sent as part of metadata from master to slave. For VARCHAR field its character length is converted into equivalent octets/bytes and stored internally. At the time of displaying the data to user it is converted back to original character length. For example: VARCHAR(2)- utf8mb3 is stored as:2*3 = VARCHAR(6) At the time of displaying it to user VARCHAR(6)- charset utf8mb3:6/3= VARCHAR(2). At present the internally converted octect length is sent from master to slave with out providing the charset information. On slave side if the type conversion fails 'show_sql_type' function is used to get the type specific information from metadata. Since there is no charset information is available the filed type is displayed as VARCHAR(6). This results in confused error message. For CHAR fields CHAR(1)- utf8mb3 - CHAR(3) CHAR(1)- utf8mb4 - CHAR(4) 'show_sql_type' function which retrieves type information from metadata uses (bytes/local charset length) to get actual character length. If slave's chaset is 'utf8mb4' then CHAR(3/4)-->CHAR(0) CHAR(4/4)-->CHAR(1). This results in confused error message. Analysis for BLOB type issue: BLOB's length is represented in two forms. 1. Actual length i.e (length < 256) type= MYSQL_TYPE_TINY_BLOB; (length < 65536) type= MYSQL_TYPE_BLOB; ... 2. packlength - The number of bytes used to represent the length of the blob 1- tinyblob 2- blob ... In row based replication only the packlength is written in the binary log. On the slave side this packlength is interpreted as actual length of the blob. Hence the length is always < 256 and the type is displayed as tiny blob. Analysis for BINARY to BINARY type issue: The character set information is needed to identify a filed's type as char or binary. Since master side character set information is not available on the slave side both binary and char fields are displayed as char. Fix: === For CHAR and VARCHAR fields display their length in octets for both source and target fields. For target field display the charset information if it is relevant. For blob type changed the code to use the packlength and display appropriate blob type in error message. For binary and varbinary fields use the slave side character set as reference to map them to binary or varbinary fields. --- sql/field.cc | 45 +++++++++++++++++++++++++++++++++++++++++++++ sql/field.h | 3 +++ sql/rpl_utility.cc | 47 ++++++++++++++++++++++++++++++----------------- sql/share/errmsg-utf8.txt | 2 +- 4 files changed, 79 insertions(+), 18 deletions(-) (limited to 'sql') diff --git a/sql/field.cc b/sql/field.cc index e9eaa440952..d7ac03328a0 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -7392,6 +7392,28 @@ void Field_string::sql_type(String &res) const res.append(STRING_WITH_LEN(" binary")); } +/** + For fields which are associated with character sets their length is provided + in octets and their character set information is also provided as part of + type information. + + @param res String which contains filed type and length. +*/ +void Field_string::sql_rpl_type(String *res) const +{ + CHARSET_INFO *cs=charset(); + if (Field_string::has_charset()) + { + size_t length= cs->cset->snprintf(cs, (char*) res->ptr(), + res->alloced_length(), + "char(%u octets) character set %s", + field_length, + charset()->csname); + res->length(length); + } + else + Field_string::sql_type(*res); + } uchar *Field_string::pack(uchar *to, const uchar *from, uint max_length) { @@ -7820,6 +7842,29 @@ void Field_varstring::sql_type(String &res) const res.append(STRING_WITH_LEN(" binary")); } +/** + For fields which are associated with character sets their length is provided + in octets and their character set information is also provided as part of + type information. + + @param res String which contains filed type and length. +*/ +void Field_varstring::sql_rpl_type(String *res) const +{ + CHARSET_INFO *cs=charset(); + if (Field_varstring::has_charset()) + { + size_t length= cs->cset->snprintf(cs, (char*) res->ptr(), + res->alloced_length(), + "varchar(%u octets) character set %s", + field_length, + charset()->csname); + res->length(length); + } + else + Field_varstring::sql_type(*res); +} + uint32 Field_varstring::data_length() { diff --git a/sql/field.h b/sql/field.h index 083f3d01bb8..b9eb2085f0b 100644 --- a/sql/field.h +++ b/sql/field.h @@ -1106,6 +1106,7 @@ public: in str and restore it with set() if needed */ virtual void sql_type(String &str) const =0; + virtual void sql_rpl_type(String *str) const { sql_type(*str); } virtual uint size_of() const =0; // For new field inline bool is_null(my_ptrdiff_t row_offset= 0) const { @@ -3162,6 +3163,7 @@ public: int cmp(const uchar *,const uchar *); void sort_string(uchar *buff,uint length); void sql_type(String &str) const; + void sql_rpl_type(String*) const; virtual uchar *pack(uchar *to, const uchar *from, uint max_length); virtual const uchar *unpack(uchar* to, const uchar *from, @@ -3264,6 +3266,7 @@ public: uint get_key_image(uchar *buff,uint length, imagetype type); void set_key_image(const uchar *buff,uint length); void sql_type(String &str) const; + void sql_rpl_type(String*) const; virtual uchar *pack(uchar *to, const uchar *from, uint max_length); virtual const uchar *unpack(uchar* to, const uchar *from, const uchar *from_end, uint param_data); diff --git a/sql/rpl_utility.cc b/sql/rpl_utility.cc index 4277e68a8b5..c28019e4dab 100644 --- a/sql/rpl_utility.cc +++ b/sql/rpl_utility.cc @@ -341,7 +341,8 @@ uint32 table_def::calc_field_size(uint col, uchar *master_data) const #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION) /** */ -void show_sql_type(enum_field_types type, uint16 metadata, String *str, CHARSET_INFO *field_cs) +void show_sql_type(enum_field_types type, uint16 metadata, String *str, + bool char_with_octets) { DBUG_ENTER("show_sql_type"); DBUG_PRINT("enter", ("type: %d, metadata: 0x%x", type, metadata)); @@ -408,9 +409,13 @@ void show_sql_type(enum_field_types type, uint16 metadata, String *str, CHARSET_ case MYSQL_TYPE_VARCHAR: { CHARSET_INFO *cs= str->charset(); - uint32 length= - cs->cset->snprintf(cs, (char*) str->ptr(), str->alloced_length(), - "varchar(%u)", metadata); + uint32 length=0; + if (char_with_octets) + length= cs->cset->snprintf(cs, (char*) str->ptr(), str->alloced_length(), + "varchar(%u octets)", metadata); + else + length= cs->cset->snprintf(cs, (char*) str->ptr(), str->alloced_length(), + "varbinary(%u)", metadata); str->length(length); } break; @@ -460,22 +465,22 @@ void show_sql_type(enum_field_types type, uint16 metadata, String *str, CHARSET_ it is necessary to check the pack length to figure out what kind of blob it really is. */ - switch (get_blob_type_from_length(metadata)) + switch (metadata) { - case MYSQL_TYPE_TINY_BLOB: + case 1: str->set_ascii(STRING_WITH_LEN("tinyblob")); break; - case MYSQL_TYPE_MEDIUM_BLOB: - str->set_ascii(STRING_WITH_LEN("mediumblob")); + case 2: + str->set_ascii(STRING_WITH_LEN("blob")); break; - case MYSQL_TYPE_LONG_BLOB: - str->set_ascii(STRING_WITH_LEN("longblob")); + case 3: + str->set_ascii(STRING_WITH_LEN("mediumblob")); break; - case MYSQL_TYPE_BLOB: - str->set_ascii(STRING_WITH_LEN("blob")); + case 4: + str->set_ascii(STRING_WITH_LEN("longblob")); break; default: @@ -491,9 +496,13 @@ void show_sql_type(enum_field_types type, uint16 metadata, String *str, CHARSET_ */ CHARSET_INFO *cs= str->charset(); uint bytes= (((metadata >> 4) & 0x300) ^ 0x300) + (metadata & 0x00ff); - uint32 length= - cs->cset->snprintf(cs, (char*) str->ptr(), str->alloced_length(), - "char(%d)", bytes / field_cs->mbmaxlen); + uint32 length=0; + if (char_with_octets) + length= cs->cset->snprintf(cs, (char*) str->ptr(), str->alloced_length(), + "char(%u octets)", bytes); + else + length= cs->cset->snprintf(cs, (char*) str->ptr(), str->alloced_length(), + "binary(%u)", bytes); str->length(length); } break; @@ -896,9 +905,13 @@ table_def::compatible_with(THD *thd, rpl_group_info *rgi, String source_type(source_buf, sizeof(source_buf), &my_charset_latin1); String target_type(target_buf, sizeof(target_buf), &my_charset_latin1); THD *thd= table->in_use; + bool char_with_octets= field->cmp_type() == STRING_RESULT ? + field->has_charset() : true; + + show_sql_type(type(col), field_metadata(col), &source_type, + char_with_octets); + field->sql_rpl_type(&target_type); - show_sql_type(type(col), field_metadata(col), &source_type, field->charset()); - field->sql_type(target_type); rli->report(ERROR_LEVEL, ER_SLAVE_CONVERSION_FAILED, rgi->gtid_info(), ER_THD(thd, ER_SLAVE_CONVERSION_FAILED), col, db_name, tbl_name, diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt index 1c828db65a8..325858ed05c 100644 --- a/sql/share/errmsg-utf8.txt +++ b/sql/share/errmsg-utf8.txt @@ -6461,7 +6461,7 @@ ER_MESSAGE_AND_STATEMENT eng "%s Statement: %s" ER_SLAVE_CONVERSION_FAILED - eng "Column %d of table '%-.192s.%-.192s' cannot be converted from type '%-.32s' to type '%-.32s'" + eng "Column %d of table '%-.192s.%-.192s' cannot be converted from type '%-.50s' to type '%-.50s'" ER_SLAVE_CANT_CREATE_CONVERSION eng "Can't create conversion table for table '%-.192s.%-.192s'" ER_INSIDE_TRANSACTION_PREVENTS_SWITCH_BINLOG_FORMAT -- cgit v1.2.1 From 9cd6e7ad73554be6d0186a42f983777a90a984f1 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Tue, 14 May 2019 14:01:15 +0200 Subject: MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER trying to add bad CHECK Make automatic name generation during execution (not prepare). Check result of memory allocation operation. --- sql/field.h | 3 ++- sql/sql_table.cc | 67 ++++++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 57 insertions(+), 13 deletions(-) (limited to 'sql') diff --git a/sql/field.h b/sql/field.h index b9eb2085f0b..ac7794081bc 100644 --- a/sql/field.h +++ b/sql/field.h @@ -628,6 +628,7 @@ public: /* Flag indicating that the field is physically stored in the database */ bool stored_in_db; bool utf8; /* Already in utf8 */ + bool automatic_name; Item *expr; LEX_STRING name; /* Name of constraint */ /* see VCOL_* (VCOL_FIELD_REF, ...) */ @@ -637,7 +638,7 @@ public: : vcol_type((enum_vcol_info_type)VCOL_TYPE_NONE), field_type((enum enum_field_types)MYSQL_TYPE_VIRTUAL), in_partitioning_expr(FALSE), stored_in_db(FALSE), - utf8(TRUE), expr(NULL), flags(0) + utf8(TRUE), automatic_name(FALSE), expr(NULL), flags(0) { name.str= NULL; name.length= 0; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index cc58dc4a7e8..33589b3044d 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -64,7 +64,7 @@ const char *primary_key_name="PRIMARY"; static bool check_if_keyname_exists(const char *name,KEY *start, KEY *end); static char *make_unique_key_name(THD *thd, const char *field_name, KEY *start, KEY *end); -static void make_unique_constraint_name(THD *thd, LEX_STRING *name, +static bool make_unique_constraint_name(THD *thd, LEX_STRING *name, List *vcol, uint *nr); static int copy_data_between_tables(THD *thd, TABLE *from,TABLE *to, @@ -78,6 +78,8 @@ static bool prepare_blob_field(THD *thd, Column_definition *sql_field); static int mysql_prepare_create_table(THD *, HA_CREATE_INFO *, Alter_info *, uint *, handler *, KEY **, uint *, int); static uint blob_length_by_type(enum_field_types type); +static bool fix_constraints_names(THD *thd, List + *check_constraint_list); /** @brief Helper function for explain_filename @@ -4178,15 +4180,13 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, /* Check table level constraints */ create_info->check_constraint_list= &alter_info->check_constraint_list; { - uint nr= 1; List_iterator_fast c_it(alter_info->check_constraint_list); Virtual_column_info *check; while ((check= c_it++)) { - if (!check->name.length) - make_unique_constraint_name(thd, &check->name, - &alter_info->check_constraint_list, - &nr); + if (!check->name.length || check->automatic_name) + continue; + { /* Check that there's no repeating constraint names. */ List_iterator_fast @@ -4722,6 +4722,9 @@ int create_table_impl(THD *thd, DBUG_PRINT("enter", ("db: '%s' table: '%s' tmp: %d path: %s", db, table_name, internal_tmp_table, path)); + if (fix_constraints_names(thd, &alter_info->check_constraint_list)) + DBUG_RETURN(1); + if (thd->variables.sql_mode & MODE_NO_DIR_IN_CREATE) { if (create_info->data_file_name) @@ -5185,7 +5188,7 @@ make_unique_key_name(THD *thd, const char *field_name,KEY *start,KEY *end) Make an unique name for constraints without a name */ -static void make_unique_constraint_name(THD *thd, LEX_STRING *name, +static bool make_unique_constraint_name(THD *thd, LEX_STRING *name, List *vcol, uint *nr) { @@ -5208,9 +5211,10 @@ static void make_unique_constraint_name(THD *thd, LEX_STRING *name, { name->length= (size_t) (real_end - buff); name->str= thd->strmake(buff, name->length); - return; + return (name->str == NULL); } } + return FALSE; } @@ -5793,10 +5797,11 @@ static bool is_candidate_key(KEY *key) from the list if existing found. RETURN VALUES - NONE + TRUE error + FALSE OK */ -static void +static bool handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info) { Field **f_ptr; @@ -6199,6 +6204,7 @@ remove_key: Virtual_column_info *check; TABLE_SHARE *share= table->s; uint c; + while ((check=it++)) { if (!(check->flags & Alter_info::CHECK_CONSTRAINT_IF_NOT_EXISTS) && @@ -6226,10 +6232,44 @@ remove_key: } } - DBUG_VOID_RETURN; + DBUG_RETURN(FALSE); } +static bool fix_constraints_names(THD *thd, List + *check_constraint_list) +{ + List_iterator it((*check_constraint_list)); + Virtual_column_info *check; + uint nr= 1; + DBUG_ENTER("fix_constraints_names"); + if (!check_constraint_list) + DBUG_RETURN(FALSE); + // Prevent accessing freed memory during generating unique names + while ((check=it++)) + { + if (check->automatic_name) + { + check->name.str= NULL; + check->name.length= 0; + } + } + it.rewind(); + // Generate unique names if needed + while ((check=it++)) + { + if (!check->name.length) + { + check->automatic_name= TRUE; + if (make_unique_constraint_name(thd, &check->name, + check_constraint_list, + &nr)) + DBUG_RETURN(TRUE); + } + } + DBUG_RETURN(FALSE); +} + /** Get Create_field object for newly created table by field index. @@ -9076,7 +9116,10 @@ do_continue:; } } - handle_if_exists_options(thd, table, alter_info); + if (handle_if_exists_options(thd, table, alter_info) || + fix_constraints_names(thd, &alter_info->check_constraint_list)) + DBUG_RETURN(true); + /* Look if we have to do anything at all. -- cgit v1.2.1 From e50b2bdbcf045e46d1a84ffeb9872d81e3aa5eff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 29 Aug 2019 13:10:40 +0300 Subject: MDEV-20425 Implement Boolean debug build option debug_assert Commit 536215e32fc43aa423684e9807640dcf3453924b in MariaDB Server 10.3.1 introduced the compiler flag (not cmake option) DBUG_ASSERT_AS_PRINTF that converts DBUG_ASSERT in non-debug builds into printouts. For debug builds, it could be useful to be able to convert DBUG_ASSERT into a warning or error printout, to allow execution to continue. This would allow debug builds to be used for reproducing hard failures that occur with release builds. my_assert: A Boolean flag (set by default), tied to the new option debug_assert that is available on debug builds only. When set, DBUG_ASSERT() will invoke assert(), like it did until now. When unset, DBUG_ASSERT() will invoke fprintf(stderr, ...) with the file name, line number and assertion expression. --- sql/mysqld.cc | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'sql') diff --git a/sql/mysqld.cc b/sql/mysqld.cc index c2475bbb46f..443fb54d466 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -7454,6 +7454,10 @@ struct my_option my_long_options[]= 0, GET_INT, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, #endif /* HAVE_REPLICATION */ #ifndef DBUG_OFF + {"debug-assert", 0, + "Allow DBUG_ASSERT() to invoke assert()", + &my_assert, &my_assert, + 0, GET_BOOL, OPT_ARG, 1, 0, 0, 0, 0, 0}, {"debug-assert-on-error", 0, "Do an assert in various functions if we get a fatal error", &my_assert_on_error, &my_assert_on_error, -- cgit v1.2.1 From 9487e0b259e7f410f5f93ae59851be60d6a5112c Mon Sep 17 00:00:00 2001 From: Teemu Ollakka Date: Fri, 30 Aug 2019 08:42:24 +0300 Subject: MDEV-19826 10.4 seems to crash with "pool-of-threads" (#1370) MariaDB 10.4 was crashing when thread-handling was set to pool-of-threads and wsrep was enabled. There were two apparent reasons for the crash: - Connection handling in threadpool_common.cc was missing calls to control wsrep client state. - Thread specific storage which contains thread variables (THR_KEY_mysys) was not handled appropriately by wsrep patch when pool-of-threads was configured. This patch addresses the above issues in the following way: - Wsrep client state open/close was moved in thd_prepare_connection() and end_connection() to have common handling for one-thread-per-connection and pool-of-threads. - Thread local storage handling in wsrep patch was reworked by introducing set of wsrep_xxx_threadvars() calls which replace calls to THD store_globals()/reset_globals() and deal with thread handling specifics internally. Wsrep-lib was updated to version which relaxes internal concurrency related sanity checks. Rollback code from wsrep_rollback_process() was extracted to separate calls for better readability. Post rollback thread was removed as it was completely unused. --- sql/service_wsrep.cc | 16 +- sql/sql_connect.cc | 18 +- sql/threadpool_common.cc | 9 +- sql/wsrep_client_service.cc | 10 +- sql/wsrep_high_priority_service.cc | 26 ++- sql/wsrep_mysqld.cc | 7 +- sql/wsrep_schema.cc | 22 +-- sql/wsrep_server_service.cc | 51 ++++-- sql/wsrep_server_service.h | 9 + sql/wsrep_sst.cc | 7 +- sql/wsrep_storage_service.cc | 12 +- sql/wsrep_thd.cc | 346 ++++++++++++++++++++++--------------- sql/wsrep_thd.h | 84 ++++++++- sql/wsrep_trans_observer.h | 11 ++ sql/wsrep_utils.cc | 4 +- 15 files changed, 416 insertions(+), 216 deletions(-) (limited to 'sql') diff --git a/sql/service_wsrep.cc b/sql/service_wsrep.cc index 8583897e064..43944366665 100644 --- a/sql/service_wsrep.cc +++ b/sql/service_wsrep.cc @@ -146,11 +146,10 @@ extern "C" void wsrep_handle_SR_rollback(THD *bf_thd, victim_thd->wsrep_trx_id(), victim_thd->wsrep_sr().fragments_certified(), wsrep_thd_transaction_state_str(victim_thd)); - if (bf_thd && bf_thd != victim_thd) - { - victim_thd->store_globals(); - } - else + + /* Note: do not store/reset globals before wsrep_bf_abort() call + to avoid losing BF thd context. */ + if (!(bf_thd && bf_thd != victim_thd)) { DEBUG_SYNC(victim_thd, "wsrep_before_SR_rollback"); } @@ -162,21 +161,24 @@ extern "C" void wsrep_handle_SR_rollback(THD *bf_thd, { wsrep_thd_self_abort(victim_thd); } - if (bf_thd && bf_thd != victim_thd) + if (bf_thd) { - bf_thd->store_globals(); + wsrep_store_threadvars(bf_thd); } } extern "C" my_bool wsrep_thd_bf_abort(const THD *bf_thd, THD *victim_thd, my_bool signal) { + /* Note: do not store/reset globals before wsrep_bf_abort() call + to avoid losing BF thd context. */ if (WSREP(victim_thd) && !victim_thd->wsrep_trx().active()) { WSREP_DEBUG("BF abort for non active transaction"); wsrep_start_transaction(victim_thd, victim_thd->wsrep_next_trx_id()); } my_bool ret= wsrep_bf_abort(bf_thd, victim_thd); + wsrep_store_threadvars((THD*)bf_thd); /* Send awake signal if victim was BF aborted or does not have wsrep on. Note that this should never interrupt RSU diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc index b8fff8c864a..6bf43d3df5e 100644 --- a/sql/sql_connect.cc +++ b/sql/sql_connect.cc @@ -1188,6 +1188,16 @@ void end_connection(THD *thd) { NET *net= &thd->net; +#ifdef WITH_WSREP + if (thd->wsrep_cs().state() == wsrep::client_state::s_exec) + { + /* Error happened after the thread acquired ownership to wsrep + client state, but before command was processed. Clean up the + state before wsrep_close(). */ + wsrep_after_command_ignore_result(thd); + } + wsrep_close(thd); +#endif /* WITH_WSREP */ if (thd->user_connect) { /* @@ -1321,6 +1331,7 @@ bool thd_prepare_connection(THD *thd) prepare_new_connection_state(thd); #ifdef WITH_WSREP thd->wsrep_client_thread= true; + wsrep_open(thd); #endif /* WITH_WSREP */ return FALSE; } @@ -1393,9 +1404,6 @@ void do_handle_one_connection(CONNECT *connect) create_user= FALSE; goto end_thread; } -#ifdef WITH_WSREP - wsrep_open(thd); -#endif /* WITH_WSREP */ while (thd_is_connection_alive(thd)) { @@ -1406,10 +1414,6 @@ void do_handle_one_connection(CONNECT *connect) } end_connection(thd); -#ifdef WITH_WSREP - wsrep_close(thd); -#endif /* WITH_WSREP */ - end_thread: close_connection(thd); diff --git a/sql/threadpool_common.cc b/sql/threadpool_common.cc index cfb831e2f55..6577472e20b 100644 --- a/sql/threadpool_common.cc +++ b/sql/threadpool_common.cc @@ -23,7 +23,9 @@ #include #include #include - +#ifdef WITH_WSREP +#include "wsrep_trans_observer.h" +#endif /* WITH_WSREP */ /* Threadpool parameters */ @@ -137,6 +139,11 @@ static inline void set_thd_idle(THD *thd) */ static void thread_attach(THD* thd) { +#ifdef WITH_WSREP + /* Wait until possible background rollback has finished before + attaching the thd. */ + wsrep_wait_rollback_complete_and_acquire_ownership(thd); +#endif /* WITH_WSREP */ pthread_setspecific(THR_KEY_mysys,thd->mysys_var); thd->thread_stack=(char*)&thd; thd->store_globals(); diff --git a/sql/wsrep_client_service.cc b/sql/wsrep_client_service.cc index b182691c593..5f73f9f714f 100644 --- a/sql/wsrep_client_service.cc +++ b/sql/wsrep_client_service.cc @@ -30,9 +30,9 @@ #include "slave.h" /* opt_log_slave_updates */ #include "transaction.h" /* trans_commit()... */ #include "log.h" /* stmt_has_updated_trans_table() */ -//#include "debug_sync.h" #include "mysql/service_debug_sync.h" #include "mysql/psi/mysql_thread.h" /* mysql_mutex_assert_owner() */ + namespace { @@ -57,16 +57,12 @@ Wsrep_client_service::Wsrep_client_service(THD* thd, void Wsrep_client_service::store_globals() { - DBUG_ENTER("Wsrep_client_service::store_globals"); - m_thd->store_globals(); - DBUG_VOID_RETURN; + wsrep_store_threadvars(m_thd); } void Wsrep_client_service::reset_globals() { - DBUG_ENTER("Wsrep_client_service::reset_globals"); - m_thd->reset_globals(); - DBUG_VOID_RETURN; + wsrep_reset_threadvars(m_thd); } bool Wsrep_client_service::interrupted( diff --git a/sql/wsrep_high_priority_service.cc b/sql/wsrep_high_priority_service.cc index 68cf0d1877b..73cdbd1c217 100644 --- a/sql/wsrep_high_priority_service.cc +++ b/sql/wsrep_high_priority_service.cc @@ -379,20 +379,13 @@ int Wsrep_high_priority_service::apply_toi(const wsrep::ws_meta& ws_meta, void Wsrep_high_priority_service::store_globals() { - DBUG_ENTER("Wsrep_high_priority_service::store_globals"); - /* In addition to calling THD::store_globals(), call - wsrep::client_state::store_globals() to gain ownership of - the client state */ - m_thd->store_globals(); - m_thd->wsrep_cs().store_globals(); - DBUG_VOID_RETURN; + wsrep_store_threadvars(m_thd); + m_thd->wsrep_cs().acquire_ownership(); } void Wsrep_high_priority_service::reset_globals() { - DBUG_ENTER("Wsrep_high_priority_service::reset_globals"); - m_thd->reset_globals(); - DBUG_VOID_RETURN; + wsrep_reset_threadvars(m_thd); } void Wsrep_high_priority_service::switch_execution_context(wsrep::high_priority_service& orig_high_priority_service) @@ -572,11 +565,14 @@ Wsrep_replayer_service::Wsrep_replayer_service(THD* replayer_thd, THD* orig_thd) thd_proc_info(orig_thd, "wsrep replaying trx"); /* - Swith execution context to replayer_thd and prepare it for + Switch execution context to replayer_thd and prepare it for replay execution. */ - orig_thd->reset_globals(); - replayer_thd->store_globals(); + /* Copy thd vars from orig_thd before reset, otherwise reset + for orig thd clears thread local storage before copy. */ + wsrep_assign_from_threadvars(replayer_thd); + wsrep_reset_threadvars(orig_thd); + wsrep_store_threadvars(replayer_thd); wsrep_open(replayer_thd); wsrep_before_command(replayer_thd); replayer_thd->wsrep_cs().clone_transaction_for_replay(orig_thd->wsrep_trx()); @@ -593,8 +589,8 @@ Wsrep_replayer_service::~Wsrep_replayer_service() wsrep_after_apply(replayer_thd); wsrep_after_command_ignore_result(replayer_thd); wsrep_close(replayer_thd); - replayer_thd->reset_globals(); - orig_thd->store_globals(); + wsrep_reset_threadvars(replayer_thd); + wsrep_store_threadvars(orig_thd); DBUG_ASSERT(!orig_thd->get_stmt_da()->is_sent()); DBUG_ASSERT(!orig_thd->get_stmt_da()->is_set()); diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc index 0a9adc5fa2c..fde29ae434d 100644 --- a/sql/wsrep_mysqld.cc +++ b/sql/wsrep_mysqld.cc @@ -2243,6 +2243,7 @@ static void wsrep_close_thread(THD *thd) { thd->set_killed(KILL_CONNECTION); MYSQL_CALLBACK(thread_scheduler, post_kill_notification, (thd)); + mysql_mutex_lock(&thd->LOCK_thd_kill); if (thd->mysys_var) { thd->mysys_var->abort=1; @@ -2255,6 +2256,7 @@ static void wsrep_close_thread(THD *thd) } mysql_mutex_unlock(&thd->mysys_var->mutex); } + mysql_mutex_unlock(&thd->LOCK_thd_kill); } static my_bool have_committing_connections(THD *thd, void *) @@ -2658,7 +2660,8 @@ void* start_wsrep_THD(void *arg) /* now that we've called my_thread_init(), it is safe to call DBUG_* */ thd->thread_stack= (char*) &thd; - if (thd->store_globals()) + wsrep_assign_from_threadvars(thd); + if (wsrep_store_threadvars(thd)) { close_connection(thd, ER_OUT_OF_RESOURCES); statistic_increment(aborted_connects,&LOCK_status); @@ -2703,7 +2706,7 @@ void* start_wsrep_THD(void *arg) /* Wsrep may reset globals during thread context switches, store globals before cleanup. */ - thd->store_globals(); + wsrep_store_threadvars(thd); close_connection(thd, 0); diff --git a/sql/wsrep_schema.cc b/sql/wsrep_schema.cc index 064b6cf9f46..066ea124fb7 100644 --- a/sql/wsrep_schema.cc +++ b/sql/wsrep_schema.cc @@ -29,6 +29,7 @@ #include "wsrep_binlog.h" #include "wsrep_high_priority_service.h" #include "wsrep_storage_service.h" +#include "wsrep_thd.h" #include #include @@ -145,13 +146,13 @@ public: : m_orig_thd(orig_thd) , m_cur_thd(cur_thd) { - m_orig_thd->reset_globals(); - m_cur_thd->store_globals(); + wsrep_reset_threadvars(m_orig_thd); + wsrep_store_threadvars(m_cur_thd); } ~thd_context_switch() { - m_cur_thd->reset_globals(); - m_orig_thd->store_globals(); + wsrep_reset_threadvars(m_cur_thd); + wsrep_store_threadvars(m_orig_thd); } private: THD *m_orig_thd; @@ -595,7 +596,8 @@ static void wsrep_init_thd_for_schema(THD *thd) thd->variables.option_bits |= OPTION_LOG_OFF; /* Read committed isolation to avoid gap locking */ thd->variables.tx_isolation= ISO_READ_COMMITTED; - thd->store_globals(); + wsrep_assign_from_threadvars(thd); + wsrep_store_threadvars(thd); } int Wsrep_schema::init() @@ -1123,6 +1125,7 @@ int Wsrep_schema::replay_transaction(THD* orig_thd, THD thd(next_thread_id(), true); thd.thread_stack= (orig_thd ? orig_thd->thread_stack : (char*) &thd); + wsrep_assign_from_threadvars(&thd); Wsrep_schema_impl::wsrep_off wsrep_off(&thd); Wsrep_schema_impl::binlog_off binlog_off(&thd); @@ -1228,6 +1231,7 @@ int Wsrep_schema::recover_sr_transactions(THD *orig_thd) THD storage_thd(next_thread_id(), true); storage_thd.thread_stack= (orig_thd ? orig_thd->thread_stack : (char*) &storage_thd); + wsrep_assign_from_threadvars(&storage_thd); TABLE* frag_table= 0; TABLE* cluster_table= 0; Wsrep_storage_service storage_service(&storage_thd); @@ -1333,12 +1337,7 @@ int Wsrep_schema::recover_sr_transactions(THD *orig_thd) transaction_id))) { DBUG_ASSERT(wsrep::starts_transaction(flags)); - THD* thd= new THD(next_thread_id(), true); - thd->thread_stack= (char*)&storage_thd; - - thd->real_id= pthread_self(); - - applier= new Wsrep_applier_service(thd); + applier = wsrep_create_streaming_applier(&storage_thd, "recovery"); server_state.start_streaming_applier(server_id, transaction_id, applier); applier->start_transaction(wsrep::ws_handle(transaction_id, 0), @@ -1364,6 +1363,7 @@ int Wsrep_schema::recover_sr_transactions(THD *orig_thd) Wsrep_schema_impl::end_scan(frag_table); Wsrep_schema_impl::finish_stmt(&storage_thd); trans_commit(&storage_thd); + storage_thd.set_mysys_var(0); out: DBUG_RETURN(ret); } diff --git a/sql/wsrep_server_service.cc b/sql/wsrep_server_service.cc index 42856862db3..bfb85e3d0ab 100644 --- a/sql/wsrep_server_service.cc +++ b/sql/wsrep_server_service.cc @@ -26,6 +26,7 @@ #include "wsrep_mysqld.h" #include "wsrep_schema.h" #include "wsrep_utils.h" +#include "wsrep_thd.h" #include "log.h" /* sql_print_xxx() */ #include "sql_class.h" /* system variables */ @@ -50,6 +51,10 @@ wsrep::storage_service* Wsrep_server_service::storage_service( init_service_thd(thd, cs.m_thd->thread_stack); WSREP_DEBUG("Created storage service with thread id %llu", thd->thread_id); + /* Use variables from the current thd attached to client_service. + This is because we need to be able to BF abort storage access + operations. */ + wsrep_assign_from_threadvars(thd); return new Wsrep_storage_service(thd); } @@ -62,6 +67,7 @@ wsrep::storage_service* Wsrep_server_service::storage_service( init_service_thd(thd, hps.m_thd->thread_stack); WSREP_DEBUG("Created high priority storage service with thread id %llu", thd->thread_id); + wsrep_assign_from_threadvars(thd); return new Wsrep_storage_service(thd); } @@ -71,21 +77,48 @@ void Wsrep_server_service::release_storage_service( Wsrep_storage_service* ss= static_cast(storage_service); THD* thd= ss->m_thd; + wsrep_reset_threadvars(thd); delete ss; delete thd; } +Wsrep_applier_service* +wsrep_create_streaming_applier(THD *orig_thd, const char *ctx) +{ + /* Reset variables to allow creating new variables in thread local + storage for new THD if needed. Note that reset must be done for + current_thd, as orig_thd may not be in effect. This may be the case when + streaming transaction is BF aborted and streaming applier + is created from BF aborter context. */ + Wsrep_threadvars saved_threadvars(wsrep_save_threadvars()); + wsrep_reset_threadvars(saved_threadvars.cur_thd); + THD *thd= 0; + Wsrep_applier_service *ret= 0; + if (!wsrep_create_threadvars() && + (thd= new THD(next_thread_id(), true))) + { + init_service_thd(thd, orig_thd->thread_stack); + wsrep_assign_from_threadvars(thd); + WSREP_DEBUG("Created streaming applier service in %s context with " + "thread id %llu", ctx, thd->thread_id); + if (!(ret= new (std::nothrow) Wsrep_applier_service(thd))) + { + delete thd; + } + } + /* Restore original thread local storage state before returning. */ + wsrep_restore_threadvars(saved_threadvars); + wsrep_store_threadvars(saved_threadvars.cur_thd); + return ret; +} + wsrep::high_priority_service* Wsrep_server_service::streaming_applier_service( wsrep::client_service& orig_client_service) { Wsrep_client_service& orig_cs= static_cast(orig_client_service); - THD* thd= new THD(next_thread_id(), true); - init_service_thd(thd, orig_cs.m_thd->thread_stack); - WSREP_DEBUG("Created streaming applier service in local context with " - "thread id %llu", thd->thread_id); - return new Wsrep_applier_service(thd); + return wsrep_create_streaming_applier(orig_cs.m_thd, "local"); } wsrep::high_priority_service* @@ -94,11 +127,7 @@ Wsrep_server_service::streaming_applier_service( { Wsrep_high_priority_service& orig_hps(static_cast(orig_high_priority_service)); - THD* thd= new THD(next_thread_id(), true); - init_service_thd(thd, orig_hps.m_thd->thread_stack); - WSREP_DEBUG("Created streaming applier service in high priority " - "context with thread id %llu", thd->thread_id); - return new Wsrep_applier_service(thd); + return wsrep_create_streaming_applier(orig_hps.m_thd, "high priority"); } void Wsrep_server_service::release_high_priority_service(wsrep::high_priority_service* high_priority_service) @@ -107,7 +136,9 @@ void Wsrep_server_service::release_high_priority_service(wsrep::high_priority_se static_cast(high_priority_service); THD* thd= hps->m_thd; delete hps; + wsrep_store_threadvars(thd); delete thd; + wsrep_delete_threadvars(); } void Wsrep_server_service::background_rollback(wsrep::client_state& client_state) diff --git a/sql/wsrep_server_service.h b/sql/wsrep_server_service.h index b8f1f009cde..6336fe2c473 100644 --- a/sql/wsrep_server_service.h +++ b/sql/wsrep_server_service.h @@ -77,5 +77,14 @@ private: Wsrep_server_state& m_server_state; }; +/** + Helper method to create new streaming applier. + + @param orig_thd Original thd context to copy operation context from. + @param ctx Context string for debug logging. + */ +class Wsrep_applier_service; +Wsrep_applier_service* +wsrep_create_streaming_applier(THD *orig_thd, const char *ctx); #endif /* WSREP_SERVER_SERVICE */ diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index e2878211d7e..62b30c0d67d 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -27,6 +27,8 @@ #include "wsrep_priv.h" #include "wsrep_utils.h" #include "wsrep_xid.h" +#include "wsrep_thd.h" + #include #include @@ -237,7 +239,7 @@ void wsrep_sst_received (THD* thd, wsrep thread pool. Restore original thd context before returning. */ if (thd) { - thd->store_globals(); + wsrep_store_threadvars(thd); } else { my_pthread_setspecific_ptr(THR_THD, NULL); @@ -509,7 +511,8 @@ err: thd->system_thread= SYSTEM_THREAD_GENERIC; thd->real_id= pthread_self(); - thd->store_globals(); + wsrep_assign_from_threadvars(thd); + wsrep_store_threadvars(thd); /* */ thd->variables.wsrep_on = 0; diff --git a/sql/wsrep_storage_service.cc b/sql/wsrep_storage_service.cc index e164114b733..6dfe3eee448 100644 --- a/sql/wsrep_storage_service.cc +++ b/sql/wsrep_storage_service.cc @@ -196,18 +196,10 @@ int Wsrep_storage_service::rollback(const wsrep::ws_handle& ws_handle, void Wsrep_storage_service::store_globals() { - DBUG_ENTER("Wsrep_storage_service::store_globals"); - DBUG_PRINT("info", ("Wsrep_storage_service::store_globals(%llu, %p)", - m_thd->thread_id, m_thd)); - m_thd->store_globals(); - DBUG_VOID_RETURN; + wsrep_store_threadvars(m_thd); } void Wsrep_storage_service::reset_globals() { - DBUG_ENTER("Wsrep_storage_service::reset_globals"); - DBUG_PRINT("info", ("Wsrep_storage_service::reset_globals(%llu, %p)", - m_thd->thread_id, m_thd)); - m_thd->reset_globals(); - DBUG_VOID_RETURN; + wsrep_reset_threadvars(m_thd); } diff --git a/sql/wsrep_thd.cc b/sql/wsrep_thd.cc index addf98561de..50f0376f674 100644 --- a/sql/wsrep_thd.cc +++ b/sql/wsrep_thd.cc @@ -31,8 +31,9 @@ #include "rpl_rli.h" #include "rpl_mi.h" +extern "C" pthread_key(struct st_my_thread_var*, THR_KEY_mysys); + static Wsrep_thd_queue* wsrep_rollback_queue= 0; -static Wsrep_thd_queue* wsrep_post_rollback_queue= 0; static Atomic_counter wsrep_bf_aborts_counter; @@ -149,6 +150,122 @@ void wsrep_create_appliers(long threads) } } +static void wsrep_rollback_streaming_aborted_by_toi(THD *thd) +{ + WSREP_INFO("wsrep_rollback_streaming_aborted_by_toi"); + /* Set thd->event_scheduler.data temporarily to NULL to avoid + callbacks to threadpool wait_begin() during rollback. */ + auto saved_esd= thd->event_scheduler.data; + thd->event_scheduler.data= 0; + if (thd->wsrep_cs().mode() == wsrep::client_state::m_high_priority) + { + DBUG_ASSERT(!saved_esd); + DBUG_ASSERT(thd->wsrep_applier_service); + thd->wsrep_applier_service->rollback(wsrep::ws_handle(), + wsrep::ws_meta()); + thd->wsrep_applier_service->after_apply(); + /* Will free THD */ + Wsrep_server_state::instance().server_service(). + release_high_priority_service(thd->wsrep_applier_service); + } + else + { + mysql_mutex_lock(&thd->LOCK_thd_data); + /* prepare THD for rollback processing */ + thd->reset_for_next_command(true); + thd->lex->sql_command= SQLCOM_ROLLBACK; + mysql_mutex_unlock(&thd->LOCK_thd_data); + /* Perform a client rollback, restore globals and signal + the victim only when all the resources have been + released */ + thd->wsrep_cs().client_service().bf_rollback(); + wsrep_reset_threadvars(thd); + /* Assign saved event_scheduler.data back before letting + client to continue. */ + thd->event_scheduler.data= saved_esd; + thd->wsrep_cs().sync_rollback_complete(); + } +} + +static void wsrep_rollback_high_priority(THD *thd) +{ + WSREP_INFO("rollbacker aborting SR thd: (%lld %llu)", + thd->thread_id, (long long)thd->real_id); + DBUG_ASSERT(thd->wsrep_cs().mode() == Wsrep_client_state::m_high_priority); + /* Must be streaming and must have been removed from the + server state streaming appliers map. */ + DBUG_ASSERT(thd->wsrep_trx().is_streaming()); + DBUG_ASSERT(!Wsrep_server_state::instance().find_streaming_applier( + thd->wsrep_trx().server_id(), + thd->wsrep_trx().id())); + DBUG_ASSERT(thd->wsrep_applier_service); + + /* Fragment removal should happen before rollback to make + the transaction non-observable in SR table after the rollback + completes. For correctness the order does not matter here, + but currently it is mandated by checks in some MTR tests. */ + wsrep::transaction_id transaction_id(thd->wsrep_trx().id()); + Wsrep_storage_service* storage_service= + static_cast( + Wsrep_server_state::instance().server_service().storage_service( + *thd->wsrep_applier_service)); + storage_service->store_globals(); + storage_service->adopt_transaction(thd->wsrep_trx()); + storage_service->remove_fragments(); + storage_service->commit(wsrep::ws_handle(transaction_id, 0), + wsrep::ws_meta()); + Wsrep_server_state::instance().server_service().release_storage_service(storage_service); + wsrep_store_threadvars(thd); + thd->wsrep_applier_service->rollback(wsrep::ws_handle(), + wsrep::ws_meta()); + thd->wsrep_applier_service->after_apply(); + /* Will free THD */ + Wsrep_server_state::instance().server_service() + .release_high_priority_service(thd->wsrep_applier_service); +} + +static void wsrep_rollback_local(THD *thd) +{ + WSREP_INFO("Wsrep_rollback_local"); + if (thd->wsrep_trx().is_streaming()) + { + wsrep::transaction_id transaction_id(thd->wsrep_trx().id()); + Wsrep_storage_service* storage_service= + static_cast( + Wsrep_server_state::instance().server_service(). + storage_service(thd->wsrep_cs().client_service())); + + storage_service->store_globals(); + storage_service->adopt_transaction(thd->wsrep_trx()); + storage_service->remove_fragments(); + storage_service->commit(wsrep::ws_handle(transaction_id, 0), + wsrep::ws_meta()); + Wsrep_server_state::instance().server_service(). + release_storage_service(storage_service); + wsrep_store_threadvars(thd); + } + /* Set thd->event_scheduler.data temporarily to NULL to avoid + callbacks to threadpool wait_begin() during rollback. */ + auto saved_esd= thd->event_scheduler.data; + thd->event_scheduler.data= 0; + mysql_mutex_lock(&thd->LOCK_thd_data); + /* prepare THD for rollback processing */ + thd->reset_for_next_command(); + thd->lex->sql_command= SQLCOM_ROLLBACK; + mysql_mutex_unlock(&thd->LOCK_thd_data); + /* Perform a client rollback, restore globals and signal + the victim only when all the resources have been + released */ + thd->wsrep_cs().client_service().bf_rollback(); + wsrep_reset_threadvars(thd); + /* Assign saved event_scheduler.data back before letting + client to continue. */ + thd->event_scheduler.data= saved_esd; + thd->wsrep_cs().sync_rollback_complete(); + WSREP_DEBUG("rollbacker aborted thd: (%llu %llu)", + thd->thread_id, (long long)thd->real_id); +} + static void wsrep_rollback_process(THD *rollbacker, void *arg __attribute__((unused))) { @@ -170,119 +287,36 @@ static void wsrep_rollback_process(THD *rollbacker, WSREP_DEBUG("rollbacker thd already aborted: %llu state: %d", (long long)thd->real_id, tx.state()); - mysql_mutex_unlock(&thd->LOCK_thd_data); continue; } mysql_mutex_unlock(&thd->LOCK_thd_data); + wsrep_reset_threadvars(rollbacker); + wsrep_store_threadvars(thd); + thd->wsrep_cs().acquire_ownership(); + thd_proc_info(rollbacker, "wsrep aborter active"); - wsrep::transaction_id transaction_id(thd->wsrep_trx().id()); + /* Rollback methods below may free thd pointer. Do not try + to access it after method returns. */ if (thd->wsrep_trx().is_streaming() && thd->wsrep_trx().bf_aborted_in_total_order()) { - thd->store_globals(); - thd->wsrep_cs().store_globals(); - if (thd->wsrep_cs().mode() == wsrep::client_state::m_high_priority) - { - DBUG_ASSERT(thd->wsrep_applier_service); - thd->wsrep_applier_service->rollback(wsrep::ws_handle(), - wsrep::ws_meta()); - thd->wsrep_applier_service->after_apply(); - /* Will free THD */ - Wsrep_server_state::instance().server_service(). - release_high_priority_service(thd->wsrep_applier_service); - } - else - { - mysql_mutex_lock(&thd->LOCK_thd_data); - /* prepare THD for rollback processing */ - thd->reset_for_next_command(true); - thd->lex->sql_command= SQLCOM_ROLLBACK; - mysql_mutex_unlock(&thd->LOCK_thd_data); - /* Perform a client rollback, restore globals and signal - the victim only when all the resources have been - released */ - thd->wsrep_cs().client_service().bf_rollback(); - thd->reset_globals(); - thd->wsrep_cs().sync_rollback_complete(); - } + wsrep_rollback_streaming_aborted_by_toi(thd); } else if (wsrep_thd_is_applying(thd)) { - WSREP_DEBUG("rollbacker aborting SR thd: (%lld %llu)", - thd->thread_id, (long long)thd->real_id); - DBUG_ASSERT(thd->wsrep_cs().mode() == Wsrep_client_state::m_high_priority); - /* Must be streaming and must have been removed from the - server state streaming appliers map. */ - DBUG_ASSERT(thd->wsrep_trx().is_streaming()); - DBUG_ASSERT(!Wsrep_server_state::instance().find_streaming_applier( - thd->wsrep_trx().server_id(), - thd->wsrep_trx().id())); - DBUG_ASSERT(thd->wsrep_applier_service); - - /* Fragment removal should happen before rollback to make - the transaction non-observable in SR table after the rollback - completes. For correctness the order does not matter here, - but currently it is mandated by checks in some MTR tests. */ - Wsrep_storage_service* storage_service= - static_cast( - Wsrep_server_state::instance().server_service().storage_service( - *thd->wsrep_applier_service)); - storage_service->store_globals(); - storage_service->adopt_transaction(thd->wsrep_trx()); - storage_service->remove_fragments(); - storage_service->commit(wsrep::ws_handle(transaction_id, 0), - wsrep::ws_meta()); - Wsrep_server_state::instance().server_service().release_storage_service(storage_service); - thd->store_globals(); - thd->wsrep_cs().store_globals(); - thd->wsrep_applier_service->rollback(wsrep::ws_handle(), - wsrep::ws_meta()); - thd->wsrep_applier_service->after_apply(); - /* Will free THD */ - Wsrep_server_state::instance().server_service() - .release_high_priority_service(thd->wsrep_applier_service); - + wsrep_rollback_high_priority(thd); } else { - if (thd->wsrep_trx().is_streaming()) - { - Wsrep_storage_service* storage_service= - static_cast( - Wsrep_server_state::instance().server_service(). - storage_service(thd->wsrep_cs().client_service())); - - storage_service->store_globals(); - storage_service->adopt_transaction(thd->wsrep_trx()); - storage_service->remove_fragments(); - storage_service->commit(wsrep::ws_handle(transaction_id, 0), - wsrep::ws_meta()); - Wsrep_server_state::instance().server_service(). - release_storage_service(storage_service); - } - thd->store_globals(); - thd->wsrep_cs().store_globals(); - mysql_mutex_lock(&thd->LOCK_thd_data); - /* prepare THD for rollback processing */ - thd->reset_for_next_command(); - thd->lex->sql_command= SQLCOM_ROLLBACK; - mysql_mutex_unlock(&thd->LOCK_thd_data); - /* Perform a client rollback, restore globals and signal - the victim only when all the resources have been - released */ - thd->wsrep_cs().client_service().bf_rollback(); - thd->reset_globals(); - thd->wsrep_cs().sync_rollback_complete(); - WSREP_DEBUG("rollbacker aborted thd: (%llu %llu)", - thd->thread_id, (long long)thd->real_id); + wsrep_rollback_local(thd); } - + wsrep_store_threadvars(rollbacker); thd_proc_info(rollbacker, "wsrep aborter idle"); } - + delete wsrep_rollback_queue; wsrep_rollback_queue= NULL; @@ -293,39 +327,6 @@ static void wsrep_rollback_process(THD *rollbacker, DBUG_VOID_RETURN; } -static void wsrep_post_rollback_process(THD *post_rollbacker, - void *arg __attribute__((unused))) -{ - DBUG_ENTER("wsrep_post_rollback_process"); - THD* thd= NULL; - - WSREP_INFO("Starting post rollbacker thread %llu", post_rollbacker->thread_id); - DBUG_ASSERT(!wsrep_post_rollback_queue); - wsrep_post_rollback_queue= new Wsrep_thd_queue(post_rollbacker); - - while ((thd= wsrep_post_rollback_queue->pop_front()) != NULL) - { - thd->store_globals(); - wsrep::client_state& cs(thd->wsrep_cs()); - mysql_mutex_lock(&thd->LOCK_thd_data); - DBUG_ASSERT(thd->wsrep_trx().state() == wsrep::transaction::s_aborting); - WSREP_DEBUG("post rollbacker calling post rollback for thd %llu, conf %s", - thd->thread_id, wsrep_thd_transaction_state_str(thd)); - - cs.after_rollback(); - DBUG_ASSERT(thd->wsrep_trx().state() == wsrep::transaction::s_aborted); - mysql_mutex_unlock(&thd->LOCK_thd_data); - } - - delete wsrep_post_rollback_queue; - wsrep_post_rollback_queue= NULL; - - DBUG_ASSERT(post_rollbacker->killed != NOT_KILLED); - DBUG_PRINT("wsrep",("wsrep post rollbacker thread exiting")); - WSREP_INFO("post rollbacker thread exiting %llu", post_rollbacker->thread_id); - DBUG_VOID_RETURN; -} - void wsrep_create_rollbacker() { if (wsrep_cluster_address && wsrep_cluster_address[0] != 0) @@ -337,14 +338,6 @@ void wsrep_create_rollbacker() /* create rollbacker */ if (create_wsrep_THD(args)) WSREP_WARN("Can't create thread to manage wsrep rollback"); - - /* create post_rollbacker */ - args= new Wsrep_thd_args(wsrep_post_rollback_process, - WSREP_ROLLBACKER_THREAD, - pthread_self()); - - if (create_wsrep_THD(args)) - WSREP_WARN("Can't create thread to manage wsrep post rollback"); } } @@ -438,3 +431,84 @@ void wsrep_thd_auto_increment_variables(THD* thd, *offset= thd->variables.auto_increment_offset; *increment= thd->variables.auto_increment_increment; } + +int wsrep_create_threadvars() +{ + int ret= 0; + if (thread_handling == SCHEDULER_TYPES_COUNT) + { + /* Caller should have called wsrep_reset_threadvars() before this + method. */ + DBUG_ASSERT(!pthread_getspecific(THR_KEY_mysys)); + pthread_setspecific(THR_KEY_mysys, 0); + ret= my_thread_init(); + } + return ret; +} + +void wsrep_delete_threadvars() +{ + if (thread_handling == SCHEDULER_TYPES_COUNT) + { + /* The caller should have called wsrep_store_threadvars() before + this method. */ + DBUG_ASSERT(pthread_getspecific(THR_KEY_mysys)); + /* Reset psi state to avoid deallocating applier thread + psi_thread. */ + PSI_thread *psi_thread= PSI_CALL_get_thread(); +#ifdef HAVE_PSI_INTERFACE + if (PSI_server) + { + PSI_server->set_thread(0); + } +#endif /* HAVE_PSI_INTERFACE */ + my_thread_end(); + PSI_CALL_set_thread(psi_thread); + pthread_setspecific(THR_KEY_mysys, 0); + } +} + +void wsrep_assign_from_threadvars(THD *thd) +{ + if (thread_handling == SCHEDULER_TYPES_COUNT) + { + st_my_thread_var *mysys_var= (st_my_thread_var *)pthread_getspecific(THR_KEY_mysys); + DBUG_ASSERT(mysys_var); + thd->set_mysys_var(mysys_var); + } +} + +Wsrep_threadvars wsrep_save_threadvars() +{ + return Wsrep_threadvars{ + current_thd, + (st_my_thread_var*) pthread_getspecific(THR_KEY_mysys) + }; +} + +void wsrep_restore_threadvars(const Wsrep_threadvars& globals) +{ + set_current_thd(globals.cur_thd); + pthread_setspecific(THR_KEY_mysys, globals.mysys_var); +} + +int wsrep_store_threadvars(THD *thd) +{ + if (thread_handling == SCHEDULER_TYPES_COUNT) + { + pthread_setspecific(THR_KEY_mysys, thd->mysys_var); + } + return thd->store_globals(); +} + +void wsrep_reset_threadvars(THD *thd) +{ + if (thread_handling == SCHEDULER_TYPES_COUNT) + { + pthread_setspecific(THR_KEY_mysys, 0); + } + else + { + thd->reset_globals(); + } +} diff --git a/sql/wsrep_thd.h b/sql/wsrep_thd.h index 2eceb3223a8..872570cd028 100644 --- a/sql/wsrep_thd.h +++ b/sql/wsrep_thd.h @@ -82,13 +82,8 @@ private: mysql_cond_t COND_wsrep_thd_queue; }; -void wsrep_prepare_bf_thd(THD*, struct wsrep_thd_shadow*); -void wsrep_return_from_bf_mode(THD*, struct wsrep_thd_shadow*); - int wsrep_show_bf_aborts (THD *thd, SHOW_VAR *var, char *buff, enum enum_var_type scope); -void wsrep_client_rollback(THD *thd, bool rollbacker = false); -void wsrep_replay_transaction(THD *thd); void wsrep_create_appliers(long threads); void wsrep_create_rollbacker(); @@ -96,8 +91,83 @@ bool wsrep_bf_abort(const THD*, THD*); int wsrep_abort_thd(void *bf_thd_ptr, void *victim_thd_ptr, my_bool signal); extern void wsrep_thd_set_PA_safe(void *thd_ptr, my_bool safe); -THD* wsrep_start_SR_THD(char *thread_stack); -void wsrep_end_SR_THD(THD* thd); + +/* + Helper methods to deal with thread local storage. + The purpose of these methods is to hide the details of thread + local storage handling when operating with wsrep storage access + and streaming applier THDs + + With one-thread-per-connection thread handling thread specific + variables are allocated when the thread is started and deallocated + before thread exits (my_thread_init(), my_thread_end()). However, + with pool-of-threads thread handling new thread specific variables + are allocated for each THD separately (see threadpool_add_connection()), + and the variables in thread local storage are assigned from + currently active thread (see thread_attach()). This must be taken into + account when storing/resetting thread local storage and when creating + streaming applier THDs. +*/ + +/** + Create new variables for thread local storage. With + one-thread-per-connection thread handling this is a no op, + with pool-of-threads new variables are created via my_thread_init(). + It is assumed that the caller has called wsrep_reset_threadvars() to clear + the thread local storage before this call. + + @return Zero in case of success, non-zero otherwise. +*/ +int wsrep_create_threadvars(); + +/** + Delete variables which were created by wsrep_create_threadvars(). + The caller must store variables into thread local storage before + this call via wsrep_store_threadvars(). +*/ +void wsrep_delete_threadvars(); + +/** + Assign variables from current thread local storage into THD. + This should be called for THDs whose lifetime is limited to single + thread execution or which may share the operation context with some + parent THD (e.g. storage access) and thus don't require separately + allocated globals. + + With one-thread-per-connection thread handling this is a no-op, + with pool-of-threads the variables which are currently stored into + thread local storage are assigned to THD. +*/ +void wsrep_assign_from_threadvars(THD *); + +/** + Helper struct to save variables from thread local storage. + */ +struct Wsrep_threadvars +{ + THD* cur_thd; + st_my_thread_var* mysys_var; +}; + +/** + Save variables from thread local storage into Wsrep_threadvars struct. + */ +Wsrep_threadvars wsrep_save_threadvars(); + +/** + Restore variables into thread local storage from Wsrep_threadvars struct. +*/ +void wsrep_restore_threadvars(const Wsrep_threadvars&); + +/** + Store variables into thread local storage. +*/ +int wsrep_store_threadvars(THD *); + +/** + Reset thread local storage. +*/ +void wsrep_reset_threadvars(THD *); /** Helper functions to override error status diff --git a/sql/wsrep_trans_observer.h b/sql/wsrep_trans_observer.h index 64ac80783c9..6bb26c40064 100644 --- a/sql/wsrep_trans_observer.h +++ b/sql/wsrep_trans_observer.h @@ -422,6 +422,17 @@ static inline void wsrep_close(THD* thd) DBUG_VOID_RETURN; } +static inline void +wsrep_wait_rollback_complete_and_acquire_ownership(THD *thd) +{ + DBUG_ENTER("wsrep_wait_rollback_complete_and_acquire_ownership"); + if (thd->wsrep_cs().state() != wsrep::client_state::s_none) + { + thd->wsrep_cs().wait_rollback_complete_and_acquire_ownership(); + } + DBUG_VOID_RETURN; +} + static inline int wsrep_before_command(THD* thd) { return (thd->wsrep_cs().state() != wsrep::client_state::s_none ? diff --git a/sql/wsrep_utils.cc b/sql/wsrep_utils.cc index 52949a95e5d..49ea78a3872 100644 --- a/sql/wsrep_utils.cc +++ b/sql/wsrep_utils.cc @@ -25,6 +25,7 @@ #include "wsrep_api.h" #include "wsrep_utils.h" #include "wsrep_mysqld.h" +#include "wsrep_thd.h" #include @@ -421,7 +422,8 @@ thd::thd (my_bool won) : init(), ptr(new THD(0)) if (ptr) { ptr->thread_stack= (char*) &ptr; - ptr->store_globals(); + wsrep_assign_from_threadvars(ptr); + wsrep_store_threadvars(ptr); ptr->variables.option_bits&= ~OPTION_BIN_LOG; // disable binlog ptr->variables.wsrep_on= won; ptr->security_ctx->master_access= ~(ulong)0; -- cgit v1.2.1 From ef76f81c982bdbcfa4797ce26224db9c016ddebd Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Thu, 29 Aug 2019 15:37:49 +0300 Subject: MDEV-20109: Optimizer ignores distinct key created for materialized... (Backported to 10.3, addressed review input) Sj_materialization_picker::check_qep(): fix error in cost/fanout calculations: - for each join prefix, add #prefix_rows / TIME_FOR_COMPARE to the cost, like best_extension_by_limited_search does - Remove the fanout produced by the subquery tables. - Also take into account join condition selectivity optimize_wo_join_buffering() (used by LooseScan and FirstMatch) - also add #prefix_rows / TIME_FOR_COMPARE to the cost of each prefix. - Also take into account join condition selectivity --- sql/opt_subselect.cc | 7 +++++-- sql/sql_class.h | 5 ++++- sql/sql_select.cc | 15 +++++++++++++-- 3 files changed, 22 insertions(+), 5 deletions(-) (limited to 'sql') diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc index 2a491688c58..6dd7de63612 100644 --- a/sql/opt_subselect.cc +++ b/sql/opt_subselect.cc @@ -2387,7 +2387,7 @@ bool optimize_semijoin_nests(JOIN *join, table_map all_table_map) &subjoin_out_rows); sjm->materialization_cost.convert_from_cost(subjoin_read_time); - sjm->rows= subjoin_out_rows; + sjm->rows_with_duplicates= sjm->rows= subjoin_out_rows; // Don't use the following list because it has "stale" items. use // ref_pointer_array instead: @@ -3011,11 +3011,14 @@ bool Sj_materialization_picker::check_qep(JOIN *join, disable_jbuf, prefix_rec_count, &curpos, &dummy); prefix_rec_count= COST_MULT(prefix_rec_count, curpos.records_read); prefix_cost= COST_ADD(prefix_cost, curpos.read_time); + prefix_cost= COST_ADD(prefix_cost, + prefix_rec_count / (double) TIME_FOR_COMPARE); + //TODO: take into account join condition selectivity here } *strategy= SJ_OPT_MATERIALIZE_SCAN; *read_time= prefix_cost; - *record_count= prefix_rec_count; + *record_count= prefix_rec_count / mat_info->rows_with_duplicates; *handled_fanout= mat_nest->sj_inner_tables; return TRUE; } diff --git a/sql/sql_class.h b/sql/sql_class.h index e5de03d4911..3157f19351c 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -5870,7 +5870,10 @@ public: uint tables; /* Number of tables in the sj-nest */ - /* Expected #rows in the materialized table */ + /* Number of rows in the materialized table, before the de-duplication */ + double rows_with_duplicates; + + /* Expected #rows in the materialized table, after de-duplication */ double rows; /* diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 9d74505cb74..0cbb480e245 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -8244,6 +8244,7 @@ void JOIN::get_prefix_cost_and_fanout(uint n_tables, record_count= COST_MULT(record_count, best_positions[i].records_read); read_time= COST_ADD(read_time, best_positions[i].read_time); } + /* TODO: Take into account condition selectivities here */ } *read_time_arg= read_time;// + record_count / TIME_FOR_COMPARE; *record_count_arg= record_count; @@ -15975,10 +15976,20 @@ void optimize_wo_join_buffering(JOIN *join, uint first_tab, uint last_tab, reopt_remaining_tables &= ~rs->table->map; rec_count= COST_MULT(rec_count, pos.records_read); cost= COST_ADD(cost, pos.read_time); - - + cost= COST_ADD(cost, rec_count / (double) TIME_FOR_COMPARE); + //TODO: take into account join condition selectivity here + double pushdown_cond_selectivity= 1.0; + table_map real_table_bit= rs->table->map; + if (join->thd->variables.optimizer_use_condition_selectivity > 1) + { + pushdown_cond_selectivity= table_cond_selectivity(join, i, rs, + reopt_remaining_tables & + ~real_table_bit); + } + (*outer_rec_count) *= pushdown_cond_selectivity; if (!rs->emb_sj_nest) *outer_rec_count= COST_MULT(*outer_rec_count, pos.records_read); + } join->cur_sj_inner_tables= save_cur_sj_inner_tables; -- cgit v1.2.1 From f42a23178edee5dd1fb6e0f76bbe88ea2a98a045 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 30 Aug 2019 12:51:37 +0300 Subject: MDEV-20425: Fix -Wimplicit-fallthrough With --skip-debug-assert, DBUG_ASSERT(false) will allow execution to continue. Hence, we will need /* fall through */ after them. Some DBUG_ASSERT(0) were replaced by break; when the switch () statement was followed by DBUG_ASSERT(0). --- sql/create_options.cc | 5 +++-- sql/field.h | 3 ++- sql/sql_lex.cc | 1 + sql/sql_parse.cc | 1 + sql/sql_show.cc | 1 + sql/table.cc | 2 +- 6 files changed, 9 insertions(+), 4 deletions(-) (limited to 'sql') diff --git a/sql/create_options.cc b/sql/create_options.cc index 5adcb2f1e9e..a8d997efaf4 100644 --- a/sql/create_options.cc +++ b/sql/create_options.cc @@ -1,4 +1,4 @@ -/* Copyright (C) 2010, 2017, MariaDB Corporation Ab +/* Copyright (C) 2010, 2019, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -132,7 +132,8 @@ static bool set_one_value(ha_create_table_option *opt, switch (opt->type) { case HA_OPTION_TYPE_SYSVAR: - DBUG_ASSERT(0); // HA_OPTION_TYPE_SYSVAR's are replaced in resolve_sysvars() + // HA_OPTION_TYPE_SYSVAR's are replaced in resolve_sysvars() + break; // to DBUG_ASSERT(0) case HA_OPTION_TYPE_ULL: { ulonglong *val= (ulonglong*)value_ptr(base, opt); diff --git a/sql/field.h b/sql/field.h index 67e20c5a1b4..3228d51b748 100644 --- a/sql/field.h +++ b/sql/field.h @@ -1,7 +1,7 @@ #ifndef FIELD_INCLUDED #define FIELD_INCLUDED /* Copyright (c) 2000, 2015, Oracle and/or its affiliates. - Copyright (c) 2008, 2017, MariaDB Corporation. + Copyright (c) 2008, 2019, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -461,6 +461,7 @@ inline bool is_temporal_type_with_date(enum_field_types type) case MYSQL_TYPE_DATETIME2: case MYSQL_TYPE_TIMESTAMP2: DBUG_ASSERT(0); // field->real_type() should not get to here. + /* fall through */ default: return false; } diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 35fc14a0744..8d8ef73fcd1 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -2911,6 +2911,7 @@ void st_select_lex_unit::print(String *str, enum_query_type query_type) { default: DBUG_ASSERT(0); + /* fall through */ case UNION_TYPE: str->append(STRING_WITH_LEN(" union ")); if (union_all) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 022c6519a8a..86def348125 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -3579,6 +3579,7 @@ mysql_execute_command(THD *thd) case GET_NO_ARG: case GET_DISABLED: DBUG_ASSERT(0); + /* fall through */ case 0: case GET_FLAGSET: case GET_ENUM: diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 77eb51f6368..d551ae4761e 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -2569,6 +2569,7 @@ static const LEX_CSTRING *view_algorithm(TABLE_LIST *table) return &merge; default: DBUG_ASSERT(0); // never should happen + /* fall through */ case VIEW_ALGORITHM_UNDEFINED: return &undefined; } diff --git a/sql/table.cc b/sql/table.cc index b75107ab489..4e11ce26658 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -9114,7 +9114,7 @@ bool vers_select_conds_t::eq(const vers_select_conds_t &conds) const case SYSTEM_TIME_ALL: return true; case SYSTEM_TIME_BEFORE: - DBUG_ASSERT(0); + break; case SYSTEM_TIME_AS_OF: return start.eq(conds.start); case SYSTEM_TIME_FROM_TO: -- cgit v1.2.1 From 9dae991a95729c4a20498b35d6d42cbb6e0c3e79 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Fri, 30 Aug 2019 17:06:53 +0200 Subject: MDEV-19200 Do not print "Thread did not exit" message for system/background THDs --- sql/mysqld.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 63d1d07cfaa..fc7e82936ea 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -1592,7 +1592,7 @@ static my_bool kill_thread_phase_2(THD *thd, void *) /* associated with the kill thread phase 1 */ static my_bool warn_threads_active_after_phase_1(THD *thd, void *) { - if (!thd->is_binlog_dump_thread()) + if (!thd->is_binlog_dump_thread() && thd->vio_ok()) sql_print_warning("%s: Thread %llu (user : '%s') did not exit\n", my_progname, (ulonglong) thd->thread_id, (thd->main_security_ctx.user ? -- cgit v1.2.1 From d1ef02e9592b744d6c8dbca9231152f0a58d4384 Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Fri, 30 Aug 2019 09:58:24 -0700 Subject: MDEV-20265 Unknown column in field list This patch corrects the fix of the patch for mdev-19421 that resolved the problem of parsing some embedded join expressions such as t1 join t2 left join t3 on t2.a=t3.a on t1.a=t2.a. Yet the patch contained a bug that prevented proper context analysis of the queries where such expressions were used together with comma separated table references in from clauses. --- sql/sql_parse.cc | 101 ++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 73 insertions(+), 28 deletions(-) (limited to 'sql') diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index ccc180607ed..1f612ff2266 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -8324,9 +8324,9 @@ TABLE_LIST *st_select_lex::nest_last_join(THD *thd) DBUG_ENTER("nest_last_join"); TABLE_LIST *head= join_list->head(); - if (head->nested_join && head->nested_join->nest_type & REBALANCED_NEST) + if (head->nested_join && (head->nested_join->nest_type & REBALANCED_NEST)) { - join_list->empty(); + head= join_list->pop(); DBUG_RETURN(head); } @@ -8410,13 +8410,13 @@ void st_select_lex::add_joined_table(TABLE_LIST *table) context and right-associative in another context. In this query - SELECT * FROM t1 JOIN t2 LEFT JOIN t3 ON t2.a=t3.a (Q1) + SELECT * FROM t1 JOIN t2 LEFT JOIN t3 ON t2.a=t3.a (Q1) JOIN is left-associative and the query Q1 is interpreted as - SELECT * FROM (t1 JOIN t2) LEFT JOIN t3 ON t2.a=t3.a. + SELECT * FROM (t1 JOIN t2) LEFT JOIN t3 ON t2.a=t3.a. While in this query - SELECT * FROM t1 JOIN t2 LEFT JOIN t3 ON t2.a=t3.a ON t1.b=t2.b (Q2) + SELECT * FROM t1 JOIN t2 LEFT JOIN t3 ON t2.a=t3.a ON t1.b=t2.b (Q2) JOIN is right-associative and the query Q2 is interpreted as - SELECT * FROM t1 JOIN (t2 LEFT JOIN t3 ON t2.a=t3.a) ON t1.b=t2.b + SELECT * FROM t1 JOIN (t2 LEFT JOIN t3 ON t2.a=t3.a) ON t1.b=t2.b JOIN is right-associative if it is used with ON clause or with USING clause. Otherwise it is left-associative. @@ -8462,9 +8462,9 @@ void st_select_lex::add_joined_table(TABLE_LIST *table) J LJ - ON / \ / \ - t1 LJ - ON (TQ3*) => J t2 - / \ / \ - t3 t2 t1 t3 + t1 LJ - ON (TQ3*) => t3 J + / \ / \ + t3 t2 t1 t2 With several left associative JOINs SELECT * FROM t1 JOIN t2 JOIN t3 LEFT JOIN t4 ON t3.a=t4.a (Q4) @@ -8472,15 +8472,15 @@ void st_select_lex::add_joined_table(TABLE_LIST *table) J1 LJ - ON / \ / \ - t1 LJ - ON J2 t4 + t1 J2 J2 t4 / \ => / \ - J2 t4 J1 t3 - / \ / \ - t2 t3 t1 t2 + t2 LJ - ON J1 t3 + / \ / \ + t3 t4 t1 t2 - Here's another example: - SELECT * - FROM t1 JOIN t2 LEFT JOIN t3 JOIN t4 ON t3.a=t4.a ON t2.b=t3.b (Q5) + Here's another example: + SELECT * + FROM t1 JOIN t2 LEFT JOIN t3 JOIN t4 ON t3.a=t4.a ON t2.b=t3.b (Q5) J LJ - ON / \ / \ @@ -8490,15 +8490,58 @@ void st_select_lex::add_joined_table(TABLE_LIST *table) / \ t3 t4 - If the transformed nested join node node is a natural join node like in - the following query - SELECT * FROM t1 JOIN t2 LEFT JOIN t3 USING(a) (Q6) - the transformation additionally has to take care about setting proper - references in the field natural_join for both operands of the natural - join operation. - The function also has to change the name resolution context for ON - expressions used in the transformed join expression to take into - account the tables of the left_op node. + If the transformed nested join node node is a natural join node like in + the following query + SELECT * FROM t1 JOIN t2 LEFT JOIN t3 USING(a) (Q6) + the transformation additionally has to take care about setting proper + references in the field natural_join for both operands of the natural + join operation. + + The queries that combine comma syntax for join operation with + JOIN expression require a special care. Consider the query + SELECT * FROM t1, t2 JOIN t3 LEFT JOIN t4 ON t3.a=t4.a (Q7) + This query is equivalent to the query + SELECT * FROM (t1, t2) JOIN t3 LEFT JOIN t4 ON t3.a=t4.a + The latter is transformed in the same way as query Q1 + + J LJ - ON + / \ / \ + (t1,t2) LJ - ON => J t4 + / \ / \ + t3 t4 (t1,t2) t3 + + A transformation similar to the transformation for Q3 is done for + the following query with RIGHT JOIN + SELECT * FROM t1, t2 JOIN t3 RIGHT JOIN t4 ON t3.a=t4.a (Q8) + + J LJ - ON + / \ / \ + t3 LJ - ON => t4 J + / \ / \ + t4 (t1,t2) (t1,t2) t3 + + The function also has to change the name resolution context for ON + expressions used in the transformed join expression to take into + account the tables of the left_op node. + + TODO: + A more elegant solution would be to implement the transformation that + eliminates nests for cross join operations. For Q7 it would work like this: + + J LJ - ON + / \ / \ + (t1,t2) LJ - ON => (t1,t2,t3) t4 + / \ + t3 t4 + + For Q8 with RIGHT JOIN the transformation would work similarly: + + J LJ - ON + / \ / \ + t3 LJ - ON => t4 (t1,t2,t3) + / \ + t4 (t1,t2) + */ bool st_select_lex::add_cross_joined_table(TABLE_LIST *left_op, @@ -8523,11 +8566,9 @@ bool st_select_lex::add_cross_joined_table(TABLE_LIST *left_op, } TABLE_LIST *tbl; - List *jl= &right_op->nested_join->join_list; + List *right_op_jl= right_op->join_list; TABLE_LIST *cj_nest; - add_joined_table(right_op); - /* Create the node NJ for a new nested join for the future inclusion of left_op in it. Initially the nest is empty. @@ -8542,6 +8583,8 @@ bool st_select_lex::add_cross_joined_table(TABLE_LIST *left_op, List *cjl= &cj_nest->nested_join->join_list; cjl->empty(); + List *jl= &right_op->nested_join->join_list; + DBUG_ASSERT(jl->elements == 2); /* Look for the left most node tbl of the right_op tree */ for ( ; ; ) { @@ -8614,6 +8657,8 @@ bool st_select_lex::add_cross_joined_table(TABLE_LIST *left_op, create a new top level nested join node. */ right_op->nested_join->nest_type|= REBALANCED_NEST; + if (unlikely(right_op_jl->push_front(right_op))) + DBUG_RETURN(true); DBUG_RETURN(false); } -- cgit v1.2.1 From a3e49c0d36c0347e9dee43f9996ca7c407836a51 Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Tue, 27 Aug 2019 23:01:12 +0300 Subject: MDEV-18501 Partition pruning doesn't work for historical queries (cleanup) Cleanup removes useless allocation. --- sql/partition_info.cc | 9 +-------- sql/partition_info.h | 3 +-- sql/sql_yacc.yy | 8 ++++---- sql/sql_yacc_ora.yy | 8 ++++---- 4 files changed, 10 insertions(+), 18 deletions(-) (limited to 'sql') diff --git a/sql/partition_info.cc b/sql/partition_info.cc index ba6fc8a49ec..65cf0298f4f 100644 --- a/sql/partition_info.cc +++ b/sql/partition_info.cc @@ -1473,15 +1473,8 @@ void partition_info::print_no_partition_found(TABLE *table_arg, myf errflag) FALSE Success */ -bool partition_info::set_part_expr(THD *thd, char *start_token, Item *item_ptr, - char *end_token, bool is_subpart) +bool partition_info::set_part_expr(THD *thd, Item *item_ptr, bool is_subpart) { - size_t expr_len= end_token - start_token; - char *func_string= (char*) thd->memdup(start_token, expr_len); - - if (unlikely(!func_string)) - return TRUE; - if (is_subpart) { list_of_subpart_fields= FALSE; diff --git a/sql/partition_info.h b/sql/partition_info.h index d6a6214c172..253e88174dd 100644 --- a/sql/partition_info.h +++ b/sql/partition_info.h @@ -366,8 +366,7 @@ public: void init_col_val(part_column_list_val *col_val, Item *item); int reorganize_into_single_field_col_val(THD *thd); part_column_list_val *add_column_value(THD *thd); - bool set_part_expr(THD *thd, char *start_token, Item *item_ptr, - char *end_token, bool is_subpart); + bool set_part_expr(THD *thd, Item *item_ptr, bool is_subpart); bool set_up_charset_field_preps(THD *thd); bool check_partition_field_length(); bool init_column_part(THD *thd); diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index c1b2fd2294b..563de8a0368 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -5541,10 +5541,10 @@ part_column_list: part_func: - '(' remember_name part_func_expr remember_end ')' + '(' part_func_expr ')' { partition_info *part_info= Lex->part_info; - if (unlikely(part_info->set_part_expr(thd, $2 + 1, $3, $4, FALSE))) + if (unlikely(part_info->set_part_expr(thd, $2, FALSE))) MYSQL_YYABORT; part_info->num_columns= 1; part_info->column_list= FALSE; @@ -5552,9 +5552,9 @@ part_func: ; sub_part_func: - '(' remember_name part_func_expr remember_end ')' + '(' part_func_expr ')' { - if (unlikely(Lex->part_info->set_part_expr(thd, $2 + 1, $3, $4, TRUE))) + if (unlikely(Lex->part_info->set_part_expr(thd, $2, TRUE))) MYSQL_YYABORT; } ; diff --git a/sql/sql_yacc_ora.yy b/sql/sql_yacc_ora.yy index 28d3d1bf90b..b31f2a0f890 100644 --- a/sql/sql_yacc_ora.yy +++ b/sql/sql_yacc_ora.yy @@ -5387,10 +5387,10 @@ part_column_list: part_func: - '(' remember_name part_func_expr remember_end ')' + '(' part_func_expr ')' { partition_info *part_info= Lex->part_info; - if (unlikely(part_info->set_part_expr(thd, $2 + 1, $3, $4, FALSE))) + if (unlikely(part_info->set_part_expr(thd, $2, FALSE))) MYSQL_YYABORT; part_info->num_columns= 1; part_info->column_list= FALSE; @@ -5398,9 +5398,9 @@ part_func: ; sub_part_func: - '(' remember_name part_func_expr remember_end ')' + '(' part_func_expr ')' { - if (unlikely(Lex->part_info->set_part_expr(thd, $2 + 1, $3, $4, TRUE))) + if (unlikely(Lex->part_info->set_part_expr(thd, $2, TRUE))) MYSQL_YYABORT; } ; -- cgit v1.2.1 From c3f35ea55a7150b186284e2ef717d12e6f81e13e Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Wed, 28 Aug 2019 11:57:16 +0300 Subject: MDEV-18501 Partition pruning doesn't work for historical queries (refactoring) SYSTEM_TYPE partitioning: COLUMN properties removed. Partitioning is now pure RANGE based on UNIX_TIMESTAMP(row_end). DECIMAL type is now allowed as RANGE partitioning, we can partition by UNIX_TIMESTAMP() (but not for DATETIME which depends on local timezone of course). --- sql/partition_element.h | 23 ++++++----------------- sql/partition_info.cc | 22 +++++++++++----------- sql/partition_info.h | 4 ++-- sql/sql_lex.cc | 4 ++-- sql/sql_partition.cc | 16 ++++++++-------- 5 files changed, 29 insertions(+), 40 deletions(-) (limited to 'sql') diff --git a/sql/partition_element.h b/sql/partition_element.h index a3eb6953be1..ff0d0d59fc4 100644 --- a/sql/partition_element.h +++ b/sql/partition_element.h @@ -98,7 +98,7 @@ enum stat_trx_field class partition_element :public Sql_alloc { public: - enum elem_type + enum elem_type_enum { CONVENTIONAL= 0, CURRENT, @@ -125,19 +125,7 @@ public: bool max_value; // MAXVALUE range uint32 id; bool empty; - - // TODO: subclass partition_element by partitioning type to avoid such semantic - // mixup - elem_type type() - { - return (elem_type)(int(signed_flag) << 1 | int(max_value)); - } - - void type(elem_type val) - { - max_value= (bool)(val & 1); - signed_flag= (bool)(val & 2); - } + elem_type_enum type; partition_element() : part_max_rows(0), part_min_rows(0), range_value(0), @@ -148,7 +136,8 @@ public: nodegroup_id(UNDEF_NODEGROUP), has_null_value(FALSE), signed_flag(FALSE), max_value(FALSE), id(UINT_MAX32), - empty(true) + empty(true), + type(CONVENTIONAL) {} partition_element(partition_element *part_elem) : part_max_rows(part_elem->part_max_rows), @@ -164,13 +153,13 @@ public: nodegroup_id(part_elem->nodegroup_id), has_null_value(FALSE), id(part_elem->id), - empty(part_elem->empty) + empty(part_elem->empty), + type(CONVENTIONAL) {} ~partition_element() {} part_column_list_val& get_col_val(uint idx) { - DBUG_ASSERT(type() == CONVENTIONAL || list_val_list.elements == 1); part_elem_value *ev= list_val_list.head(); DBUG_ASSERT(ev); DBUG_ASSERT(ev->col_val_array); diff --git a/sql/partition_info.cc b/sql/partition_info.cc index 65cf0298f4f..f9669931eab 100644 --- a/sql/partition_info.cc +++ b/sql/partition_info.cc @@ -894,15 +894,16 @@ bool partition_info::vers_setup_expression(THD * thd, uint32 alter_add) DBUG_ASSERT(part_type == VERSIONING_PARTITION); DBUG_ASSERT(table->versioned(VERS_TIMESTAMP)); - DBUG_ASSERT(num_columns == 1); if (!alter_add) { Field *row_end= table->vers_end_field(); - part_field_list.push_back(row_end->field_name.str, thd->mem_root); - DBUG_ASSERT(part_field_list.elements == 1); // needed in handle_list_of_fields() row_end->flags|= GET_FIXED_FIELDS_FLAG; + Name_resolution_context *context= &thd->lex->current_select->context; + Item *row_end_item= new (thd->mem_root) Item_field(thd, context, row_end); + Item *row_end_ts= new (thd->mem_root) Item_func_unix_timestamp(thd, row_end_item); + set_part_expr(thd, row_end_ts, false); } if (alter_add) @@ -911,12 +912,12 @@ bool partition_info::vers_setup_expression(THD * thd, uint32 alter_add) partition_element *el; for(uint32 id= 0; ((el= it++)); id++) { - DBUG_ASSERT(el->type() != partition_element::CONVENTIONAL); + DBUG_ASSERT(el->type != partition_element::CONVENTIONAL); /* Newly added element is inserted before AS_OF_NOW. */ - if (el->id == UINT_MAX32 || el->type() == partition_element::CURRENT) + if (el->id == UINT_MAX32 || el->type == partition_element::CURRENT) { el->id= id; - if (el->type() == partition_element::CURRENT) + if (el->type == partition_element::CURRENT) break; } } @@ -1343,13 +1344,13 @@ bool partition_info::check_partition_info(THD *thd, handlerton **eng_type, } if (part_type == VERSIONING_PARTITION) { - if (part_elem->type() == partition_element::HISTORY) + if (part_elem->type == partition_element::HISTORY) { hist_parts++; } else { - DBUG_ASSERT(part_elem->type() == partition_element::CURRENT); + DBUG_ASSERT(part_elem->type == partition_element::CURRENT); now_parts++; } } @@ -2690,9 +2691,8 @@ bool check_partition_dirs(partition_info *part_info) bool partition_info::vers_init_info(THD * thd) { part_type= VERSIONING_PARTITION; - list_of_part_fields= TRUE; - column_list= TRUE; - num_columns= 1; + list_of_part_fields= true; + column_list= false; vers_info= new (thd->mem_root) Vers_part_info; if (unlikely(!vers_info)) return true; diff --git a/sql/partition_info.h b/sql/partition_info.h index 253e88174dd..2aed6cbc5db 100644 --- a/sql/partition_info.h +++ b/sql/partition_info.h @@ -55,11 +55,11 @@ struct Vers_part_info : public Sql_alloc if (now_part) { DBUG_ASSERT(now_part->id != UINT_MAX32); - DBUG_ASSERT(now_part->type() == partition_element::CURRENT); + DBUG_ASSERT(now_part->type == partition_element::CURRENT); if (hist_part) { DBUG_ASSERT(hist_part->id != UINT_MAX32); - DBUG_ASSERT(hist_part->type() == partition_element::HISTORY); + DBUG_ASSERT(hist_part->type == partition_element::HISTORY); } return true; } diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 8d8ef73fcd1..1825ce7c1ce 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -8168,7 +8168,7 @@ bool LEX::part_values_current(THD *thd) create_last_non_select_table->table_name.str); return true; } - elem->type(partition_element::CURRENT); + elem->type= partition_element::CURRENT; DBUG_ASSERT(part_info->vers_info); part_info->vers_info->now_part= elem; if (unlikely(part_info->init_column_part(thd))) @@ -8202,7 +8202,7 @@ bool LEX::part_values_history(THD *thd) create_last_non_select_table->table_name.str); return true; } - elem->type(partition_element::HISTORY); + elem->type= partition_element::HISTORY; if (unlikely(part_info->init_column_part(thd))) return true; return false; diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc index 34f8a4e3aee..ba6d0f14aeb 100644 --- a/sql/sql_partition.cc +++ b/sql/sql_partition.cc @@ -1975,7 +1975,6 @@ bool fix_partition_func(THD *thd, TABLE *table, bool is_create_table_ind) } } DBUG_ASSERT(part_info->part_type != NOT_A_PARTITION); - DBUG_ASSERT(part_info->part_type != VERSIONING_PARTITION || part_info->column_list); /* Partition is defined. We need to verify that partitioning function is correct. @@ -2008,15 +2007,15 @@ bool fix_partition_func(THD *thd, TABLE *table, bool is_create_table_ind) { if (part_info->column_list) { - if (part_info->part_type == VERSIONING_PARTITION && - part_info->vers_setup_expression(thd)) - goto end; List_iterator it(part_info->part_field_list); if (unlikely(handle_list_of_fields(thd, it, table, part_info, FALSE))) goto end; } else { + if (part_info->part_type == VERSIONING_PARTITION && + part_info->vers_setup_expression(thd)) + goto end; if (unlikely(fix_fields_part_func(thd, part_info->part_expr, table, FALSE, is_create_table_ind))) goto end; @@ -2032,7 +2031,8 @@ bool fix_partition_func(THD *thd, TABLE *table, bool is_create_table_ind) goto end; } if (unlikely(!part_info->column_list && - part_info->part_expr->result_type() != INT_RESULT)) + part_info->part_expr->result_type() != INT_RESULT && + part_info->part_expr->result_type() != DECIMAL_RESULT)) { part_info->report_part_expr_error(FALSE); goto end; @@ -2541,7 +2541,7 @@ static int add_partition_values(String *str, partition_info *part_info, } else if (part_info->part_type == VERSIONING_PARTITION) { - switch (p_elem->type()) + switch (p_elem->type) { case partition_element::CURRENT: err+= str->append(STRING_WITH_LEN(" CURRENT")); @@ -5321,7 +5321,7 @@ that are reorganised. partition_element *el; while ((el= it++)) { - if (el->type() == partition_element::CURRENT) + if (el->type == partition_element::CURRENT) { it.remove(); now_part= el; @@ -5417,7 +5417,7 @@ that are reorganised. { if (tab_part_info->part_type == VERSIONING_PARTITION) { - if (part_elem->type() == partition_element::CURRENT) + if (part_elem->type == partition_element::CURRENT) { my_error(ER_VERS_WRONG_PARTS, MYF(0), table->s->table_name.str); goto err; -- cgit v1.2.1 From 6a490ca0fbcf0ceeaeee55bd6c98eb1ea8bcb8c5 Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Wed, 28 Aug 2019 19:51:23 +0300 Subject: MDEV-18501 Partition pruning doesn't work for historical queries (fix) Pruning fix for SYSTEM_TIME INTERVAL partitioning. Allocating one more element in range_int_array for CURRENT partition is required for RANGE pruning to work correctly (get_partition_id_range_for_endpoint()). --- sql/sql_lex.cc | 4 ---- sql/sql_partition.cc | 13 +++++++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) (limited to 'sql') diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 1825ce7c1ce..d3c98115659 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -8171,8 +8171,6 @@ bool LEX::part_values_current(THD *thd) elem->type= partition_element::CURRENT; DBUG_ASSERT(part_info->vers_info); part_info->vers_info->now_part= elem; - if (unlikely(part_info->init_column_part(thd))) - return true; return false; } @@ -8203,8 +8201,6 @@ bool LEX::part_values_history(THD *thd) return true; } elem->type= partition_element::HISTORY; - if (unlikely(part_info->init_column_part(thd))) - return true; return false; } diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc index ba6d0f14aeb..f6afb7955b5 100644 --- a/sql/sql_partition.cc +++ b/sql/sql_partition.cc @@ -1563,7 +1563,7 @@ static bool check_vers_constants(THD *thd, partition_info *part_info) return 0; part_info->range_int_array= - (longlong*) thd->alloc(hist_parts * sizeof(longlong)); + (longlong*) thd->alloc(part_info->num_parts * sizeof(longlong)); MYSQL_TIME ltime; List_iterator it(part_info->partitions); @@ -1582,6 +1582,9 @@ static bool check_vers_constants(THD *thd, partition_info *part_info) if (vers_info->hist_part->range_value <= thd->query_start()) vers_info->hist_part= el; } + DBUG_ASSERT(el == vers_info->now_part); + el->max_value= true; + part_info->range_int_array[el->id]= el->range_value= LONGLONG_MAX; return 0; err: my_error(ER_DATA_OUT_OF_RANGE, MYF(0), "TIMESTAMP", "INTERVAL"); @@ -7671,6 +7674,11 @@ static void set_up_range_analysis_info(partition_info *part_info) partitioning */ switch (part_info->part_type) { + case VERSIONING_PARTITION: + if (!part_info->vers_info->interval.is_set()) + { + break; + } case RANGE_PARTITION: case LIST_PARTITION: if (!part_info->column_list) @@ -8107,7 +8115,8 @@ static int get_part_iter_for_interval_via_mapping(partition_info *part_info, part_iter->ret_null_part= part_iter->ret_null_part_orig= FALSE; part_iter->ret_default_part= part_iter->ret_default_part_orig= FALSE; - if (part_info->part_type == RANGE_PARTITION) + if (part_info->part_type == RANGE_PARTITION || + part_info->part_type == VERSIONING_PARTITION) { if (part_info->part_charset_field_array) get_endpoint= get_partition_id_range_for_endpoint_charset; -- cgit v1.2.1 From 1bbc593eaeeb2675ad3126e9753a5e945150b7ee Mon Sep 17 00:00:00 2001 From: Monty Date: Sun, 1 Sep 2019 15:42:23 +0300 Subject: Fixed compiler warning that broke builds gcc 7.4.1 --- sql/sql_partition.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'sql') diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc index f6afb7955b5..a3dbadd9025 100644 --- a/sql/sql_partition.cc +++ b/sql/sql_partition.cc @@ -7676,9 +7676,8 @@ static void set_up_range_analysis_info(partition_info *part_info) switch (part_info->part_type) { case VERSIONING_PARTITION: if (!part_info->vers_info->interval.is_set()) - { break; - } + /* Fall through */ case RANGE_PARTITION: case LIST_PARTITION: if (!part_info->column_list) -- cgit v1.2.1 From 597b070f8254439aa5f555c6868f3d9b8d518238 Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Sun, 1 Sep 2019 17:56:47 +0300 Subject: Compilation fix Caused by: MDEV-18501 Partition pruning doesn't work for historical queries (cleanup) --- sql/partition_info.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'sql') diff --git a/sql/partition_info.cc b/sql/partition_info.cc index f9669931eab..66ec6a70b12 100644 --- a/sql/partition_info.cc +++ b/sql/partition_info.cc @@ -2648,12 +2648,9 @@ part_column_list_val *partition_info::add_column_value(THD *thd) return NULL; } -bool partition_info::set_part_expr(THD *thd, char *start_token, Item *item_ptr, - char *end_token, bool is_subpart) +bool partition_info::set_part_expr(THD *thd, Item *item_ptr, bool is_subpart) { - (void)start_token; (void)item_ptr; - (void)end_token; (void)is_subpart; return FALSE; } -- cgit v1.2.1 From 29b13d114e4c4a8a880ddf052351f7bfdc5e1fa3 Mon Sep 17 00:00:00 2001 From: Varun Gupta Date: Mon, 2 Sep 2019 00:06:31 +0530 Subject: Followup fix for MDEV-20440 --- sql/opt_subselect.cc | 3 +++ 1 file changed, 3 insertions(+) (limited to 'sql') diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc index a10c8de417f..5d1e417d259 100644 --- a/sql/opt_subselect.cc +++ b/sql/opt_subselect.cc @@ -2950,6 +2950,9 @@ void advance_sj_state(JOIN *join, table_map remaining_tables, uint idx, case SJ_OPT_DUPS_WEEDOUT: sname= "DuplicateWeedout"; break; + case SJ_OPT_LOOSE_SCAN: + sname= "LosseScan"; + break; default: DBUG_ASSERT(0); sname="Invalid"; -- cgit v1.2.1 From b1e377997e987b66c999713bdb7e6cfdb87797ae Mon Sep 17 00:00:00 2001 From: Varun Gupta Date: Mon, 2 Sep 2019 11:21:52 +0530 Subject: Fixed a typo --- sql/opt_subselect.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc index 5d1e417d259..e0873185461 100644 --- a/sql/opt_subselect.cc +++ b/sql/opt_subselect.cc @@ -2951,7 +2951,7 @@ void advance_sj_state(JOIN *join, table_map remaining_tables, uint idx, sname= "DuplicateWeedout"; break; case SJ_OPT_LOOSE_SCAN: - sname= "LosseScan"; + sname= "LooseScan"; break; default: DBUG_ASSERT(0); -- cgit v1.2.1