April 2019 - developers - lists.mariadb.org

Re: [Maria-developers] 404 galera-arbitrator-3_25.3.26-xenial_amd64.deb
by Jongmin Kim 12 Apr '19

12 Apr '19

Ping to the Cc'ed folks, and add more Cc: our new mailing list (harukasan-ftp(a)lists.launchpad.net) On Thu, Apr 11, 2019 at 11:33:33PM +0900, Jongmin Kim wrote: > Hello, > > Thanks for reporting your failure! > > On Thu, Apr 11, 2019 at 03:51:17PM +0300, Sergei Shvarts wrote: > > Hi guys! > > > > looks like there is no galera-arbitraro-3 for xenial > > > > ERROR: unable to update: download errors: > > HTTP code 404 while fetching http://ftp.harukasan.org/mariadb/repo/10.3/ubuntu/pool/main/g/galera-3/gale… > > HTTP code 404 while fetching http://ftp.harukasan.org/mariadb/repo/10.3/ubuntu/pool/main/g/galera-3/gale… > > > > We digged some, and we found that: > - the package with 'Filename' is provided in Xenial 'Packages' [1], > - but the file is not exist at 'Filename' [2]. > We don't have idea about this issue. An official upstream mirror does have same issue [3,4]. > > [1] http://ftp.harukasan.org/mariadb/repo/10.3/ubuntu/dists/xenial/main/binary-… > [2] http://ftp.harukasan.org/mariadb/repo/10.3/ubuntu/pool/main/g/galera-3/ > [3] http://ftp.osuosl.org/pub/mariadb/repo/10.3/ubuntu/dists/xenial/main/binary… > [4] http://ftp.osuosl.org/pub/mariadb/repo/10.3/ubuntu/pool/main/g/galera-3/ > > So I'm forwarding this error to the package maintainer of 'galera-arbitrator-3': > - Codership Oy <info(a)codership.com> > > Also forwarding to the repository master of MariaDB repository: > - MariaDB Developers <maria-developers(a)lists.launchpad.net> > > Dear the package maintainer and MariaDB Developers, would you please dig this problem? > > Thank you! -- Jongmin Kim OpenPGP key located at https://jongmin.dev/pgp OpenPGP fingerprint: 012E 4A06 79E1 4EFC DAAE 9472 D39D 8D29 BAF3 6DF8

2 3

[Maria-developers] GSoC 2019 proposal
by Rajiv nayan 09 Apr '19

09 Apr '19

Hello. My name is Rajiv Nayan. I would like to contribute to mariaDB during GSoC 2019. More information can be found in my draft of proposal on the attached google doc. Thanks Proposal for "Implement UPDATE with result set, and Implement multiple-table UPDATE/DELETE returning a result set” for GSoC’19 https://docs.google.com/document/d/1vb9ixQlhOcHj4RpmaAuhMNNHw4jE_j1wjnzKuNW…

1 0

[Maria-developers] (no subject)
by Rajiv nayan 09 Apr '19

09 Apr '19

Hello. My name is Rajiv Nayan. I would like to contribute to mariaDB during GSoC 2019. More information can be found in my draft of proposal on the attached google doc. Thanks Proposal for "Implement UPDATE with result set, and Implement multiple-table UPDATE/DELETE returning a result set” for GSoC’19 https://docs.google.com/document/d/1vb9ixQlhOcHj4RpmaAuhMNNHw4jE_j1wjnzKuNW…

1 0

[Maria-developers] set_time() on slave and unversioned -> versioned replication
by Aleksey Midenkov 08 Apr '19

08 Apr '19

Hello, Sergei! In unversioned -> versioned scenario in the code below it first gets to Set time 4, creates some records (on slave) and then seconds on slave increased (X+1) while on master seconds are not yet increased (X). Then we get to Set time 3 and reset time on slave to X.0 therefore we get back in time and all stored records with timestamp X.n will be in future. 'n' came from ++system_time.sec_part in Set time 4. Why did you decided to use such logic of getting seconds from master and microseconds from slave? Since microseconds sooner or later reset to 0 it's not better than just assigning some random number. What sending microseconds from master conditionally only is good for? And for RBR since you don't send microseconds at all I see no good solution for this. inline void set_time(my_time_t t, ulong sec_part) { if (opt_secure_timestamp > (slave_thread ? SECTIME_REPL : SECTIME_SUPER)) { set_time(); // note that BINLOG itself requires SUPER DBUG_EXECUTE("time", print_start_time("Set time 1");); } else { if (sec_part <= TIME_MAX_SECOND_PART) { start_time= system_time.sec= t; start_time_sec_part= system_time.sec_part= sec_part; DBUG_EXECUTE("time", print_start_time("Set time 2");); } else if (t != system_time.sec) { DBUG_EXECUTE("time", print_system_time("System time"););; start_time= system_time.sec= t; start_time_sec_part= system_time.sec_part= 0; DBUG_EXECUTE("time", print_start_time("Set time 3");); } else { start_time= t; start_time_sec_part= ++system_time.sec_part; DBUG_EXECUTE("time", print_start_time("Set time 4");); } user_time.val= hrtime_from_time(start_time) + start_time_sec_part; PSI_CALL_set_thread_start_time(start_time); start_utime= utime_after_lock= microsecond_interval_timer(); } } -- All the best, Aleksey Midenkov @midenok

2 7

[Maria-developers] GSoC'19 proposal
by Rucha Deodhar 05 Apr '19

05 Apr '19

Hello. My name is Rucha Deodhar. I would like to contribute to mariaDB during GSoC 2019. More information can be found in my draft of proposal on the attached google doc. Any feedback would be appreciated. Thanks Proposal for “ADD RETURNING TO INSERT” for GSoC’19 <https://docs.google.com/document/d/1vE52kqZI-bhbRKbGBxeIrQKss8xXaGUU-0_PJHq…>

1 0

[Maria-developers] MDEV-18822: possible slowdown with optimizer trace
by Sergey Petrunia 04 Apr '19

04 Apr '19

Hi Varun, So I was thinking of the impact of the optimizer trace, and decided to look at how the most common check "if optimizer trace is enabled" is done. The code at SQL layer does this kind of check if (thd->trace_started()) (it is wrapped somewhere with "unlikely" but this is not consistent) THD::trace_started is defined as: bool THD::trace_started() { return opt_trace.is_started(); } which opt_trace here is Opt_trace_context. looking there: class Opt_trace_context { ... bool is_started() { return current_trace && is_enabled(); } following: bool Opt_trace_context::is_enabled() { if (current_trace) return current_trace->is_enabled(); return false; } current_trace is Opt_trace_stmt, looking there: bool Opt_trace_stmt::is_enabled() { return I_S_disabled == 0; } This is quite convoluted, and could probably made shorter. Disassembling sql_select.cc, I can see that the calls to thd->trace_started() are not inlined: if (unlikely(thd->trace_started())) 10ac9: 4c 89 ff mov rdi,r15 10acc: e8 00 00 00 00 call 10ad1 <_ZL32best_extension_by_limited_searchP4JOINyjddjjj+0x1a1> 10ad1: 84 c0 test al,al 10ad3: 0f 85 fc 02 00 00 jne 10dd5 <_ZL32best_extension_by_limited_searchP4JOINyjddjjj+0x4a5> if (unlikely(thd->trace_started())) 1197b: 4c 89 f7 mov rdi,r14 1197e: e8 00 00 00 00 call 11983 <_Z11choose_planP4JOINy+0x8a3> 11983: 84 c0 test al,al 11985: 0f 85 2f 01 00 00 jne 11aba <_Z11choose_planP4JOINy+0x9da> if (thd->trace_started()) 2d6f7: 48 8b bd b0 fc ff ff mov rdi,QWORD PTR [rbp-0x350] 2d6fe: e8 00 00 00 00 call 2d703 <_ZN4JOIN14optimize_innerEv+0x1963> 2d703: 84 c0 test al,al 2d705: 0f 85 6c 09 00 00 jne 2e077 <_ZN4JOIN14optimize_innerEv+0x22d7> if (thd->trace_started()) 2d7da: 48 8b bd b0 fc ff ff mov rdi,QWORD PTR [rbp-0x350] 2d7e1: e8 00 00 00 00 call 2d7e6 <_ZN4JOIN14optimize_innerEv+0x1a46> 2d7e6: 84 c0 test al,al 2d7e8: 0f 85 ff 1a 00 00 jne 2f2ed <_ZN4JOIN14optimize_innerEv+0x354d> But does it all matter? I think the action plan should be: 1. Reproduce the slowdown as it was reported in MDEV-18822. 2. If #1 succeeds in several re-runs, declare THD::trace_started as: class THD { public: bool trace_started; // initialize this to false in constructor and never touch it inline bool trace_started() const { return trace_started; } }; and then use search-replace to change "thd->trace_started()" to "unlikely(thd->trace_started)" and then re-run the benchmark. Any thoughts? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] [Commits] a1929d001c7: MDEV-18741: Optimizer trace: multi-part key ranges are printed incorrectly
by Sergey Petrunia 04 Apr '19

04 Apr '19

Hi Varun, In general, the patch moves in the right direction, but it is not yet there. Consider this example: create table ten(a int); insert into ten values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); create table t1 ( a int not null, b int not null, c int not null, d int not null, key(a,b,c) ); insert into t1 select a,a, a,a from ten; set optimizer_trace=1; explain select * from t1 where a between 1 and 4 and b < 50; mysql> select -> JSON_DETAILED(JSON_EXTRACT(trace, '$**.analyzing_range_alternatives')) -> from INFORMATION_SCHEMA.OPTIMIZER_TRACE; { "range_scan_alternatives": [ { "index": "a", "ranges": [ "(a) < (4,50)" ], "rowid_ordered": false, "using_mrr": false, "index_only": false, "rows": 1, "cost": 2.5002, "chosen": true } ], "analyzing_roworder_intersect": { "cause": "too few roworder scans" }, "analyzing_index_merge_union": [ ] } ] | while print_quick produces: T@18 : | | | | | | | | | | >print_quick quick range select, key a, length: 8 1 <= X < 4/50 other_keys: 0x0: T@18 : | | | | | | | | | | <print_quick Please investigate why this happens. On Wed, Apr 03, 2019 at 01:16:56PM +0530, Varun wrote: > revision-id: a1929d001c7b25e26d178834a196020bade819b8 (mariadb-10.3.6-318-ga1929d001c7) Any idea why does the commit trigger show "mariadb-10.3.." ? The patch is against 10.4, isnt it? > parent(s): 0dc442ac61ac7ff1a1d6bd7d6090c0f2251fb558 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2019-04-03 13:10:07 +0530 > message: > > MDEV-18741: Optimizer trace: multi-part key ranges are printed incorrectly > > Changed the function append_range_all_keyparts to use sel_arg_range_seq_init / sel_arg_range_seq_next to produce ranges. > Also adjusted to print format for the ranges, now the ranges are printed as: > (keypart1_min, keypart2_min,..) OP (keypart1_name,keypart2_name, ..) OP (keypart1_max,keypart2_max, ..) > > Also added more tests for range and index merge access for optimizer trace ... > diff --git a/sql/opt_range.cc b/sql/opt_range.cc > index 5ab3d70214d..f53efc55158 100644 > --- a/sql/opt_range.cc > +++ b/sql/opt_range.cc > @@ -431,16 +431,18 @@ static int and_range_trees(RANGE_OPT_PARAM *param, > static bool remove_nonrange_trees(RANGE_OPT_PARAM *param, SEL_TREE *tree); > > static void print_key_value(String *out, const KEY_PART_INFO *key_part, > - const uchar *key); > + const uchar* key, uint length); > +static void print_keyparts_name(String *out, const KEY_PART_INFO *key_part, > + uint n_keypart, key_part_map keypart_map); Please fix indentation ^ > > static void append_range_all_keyparts(Json_writer_array *range_trace, > - String *range_string, > - String *range_so_far, const SEL_ARG *keypart, > + PARAM param, uint idx, > + SEL_ARG *keypart, > const KEY_PART_INFO *key_parts); > > static > -void append_range(String *out, const KEY_PART_INFO *key_parts, > - const uchar *min_key, const uchar *max_key, const uint flag); > +void append_range(String *out, const KEY_PART_INFO *key_part, > + KEY_MULTI_RANGE *range, uint n_key_parts); Please fix indentation ^ > > > /* > @@ -2273,10 +2275,7 @@ void TRP_RANGE::trace_basic_info(const PARAM *param, > // TRP_RANGE should not be created if there are no range intervals > DBUG_ASSERT(key); > > - String range_info; > - range_info.length(0); > - range_info.set_charset(system_charset_info); > - append_range_all_keyparts(&trace_range, NULL, &range_info, key, key_part); > + append_range_all_keyparts(&trace_range, *param, key_idx, key, key_part); > } > > > @@ -2489,10 +2488,8 @@ void TRP_GROUP_MIN_MAX::trace_basic_info(const PARAM *param, > // can have group quick without ranges > if (index_tree) > { > - String range_info; > - range_info.set_charset(system_charset_info); > - append_range_all_keyparts(&trace_range, NULL, &range_info, index_tree, > - key_part); > + append_range_all_keyparts(&trace_range, *param, param_idx, > + index_tree, key_part); > } > } > > @@ -6398,20 +6395,9 @@ void TRP_ROR_INTERSECT::trace_basic_info(const PARAM *param, > trace_isect_idx.add("rows", (*cur_scan)->records); > > Json_writer_array trace_range(thd, "ranges"); > - for (const SEL_ARG *current= (*cur_scan)->sel_arg->first(); current; > - current= current->next) > - { > - String range_info; > - range_info.set_charset(system_charset_info); > - for (const SEL_ARG *part= current; part; > - part= part->next_key_part ? part->next_key_part : nullptr) > - { > - const KEY_PART_INFO *cur_key_part= key_part + part->part; > - append_range(&range_info, cur_key_part, part->min_value, > - part->max_value, part->min_flag | part->max_flag); > - } > - trace_range.add(range_info.ptr(), range_info.length()); > - } > + > + append_range_all_keyparts(&trace_range, *param, (*cur_scan)->idx, > + (*cur_scan)->sel_arg->first(), key_part); Please fix indentation ^ > } > } > > @@ -7389,9 +7375,6 @@ static TRP_RANGE *get_key_scans_params(PARAM *param, SEL_TREE *tree, > const KEY &cur_key= param->table->key_info[keynr]; > const KEY_PART_INFO *key_part= cur_key.key_part; > > - String range_info; > - range_info.set_charset(system_charset_info); > - > index_scan->idx= idx; > index_scan->keynr= keynr; > index_scan->key_info= &param->table->key_info[keynr]; > @@ -7402,8 +7385,8 @@ static TRP_RANGE *get_key_scans_params(PARAM *param, SEL_TREE *tree, > *tree->index_scans_end++= index_scan; > > if (unlikely(thd->trace_started())) > - append_range_all_keyparts(&trace_range, NULL, &range_info, key, > - key_part); > + append_range_all_keyparts(&trace_range, (*param), idx, > + key, key_part); Please fix indentation ^ > trace_range.end(); > > trace_idx.add("rowid_ordered", param->is_ror_scan) > @@ -13554,11 +13537,8 @@ get_best_group_min_max(PARAM *param, SEL_TREE *tree, double read_time) > Json_writer_array trace_range(thd, "ranges"); > > const KEY_PART_INFO *key_part= cur_index_info->key_part; > - > - String range_info; > - range_info.set_charset(system_charset_info); > - append_range_all_keyparts(&trace_range, NULL, &range_info, > - cur_index_tree, key_part); > + append_range_all_keyparts(&trace_range, *param, cur_param_idx, > + cur_index_tree, key_part); Please fix indentation ^ > } > } > cost_group_min_max(table, cur_index_info, cur_used_key_parts, > @@ -15730,12 +15710,17 @@ void QUICK_GROUP_MIN_MAX_SELECT::dbug_dump(int indent, bool verbose) > } > > #endif /* !DBUG_OFF */ > + > static > void append_range(String *out, const KEY_PART_INFO *key_part, > - const uchar *min_key, const uchar *max_key, const uint flag) > + KEY_MULTI_RANGE *range, uint n_key_parts) > { > - if (out->length() > 0) > - out->append(STRING_WITH_LEN(" AND ")); > + uint flag= range->range_flag; > + String key_name; > + key_name.set_charset(system_charset_info); > + key_part_map keypart_map= range->start_key.length ? > + range->start_key.keypart_map : > + range->end_key.keypart_map; > > if (flag & GEOM_FLAG) > { > @@ -15744,22 +15729,24 @@ void append_range(String *out, const KEY_PART_INFO *key_part, > range types, so printing "col < some_geom" doesn't make sense. > Just print the column name, not operator. > */ > - out->append(key_part->field->field_name); > + print_keyparts_name(out, key_part, n_key_parts, keypart_map); > out->append(STRING_WITH_LEN(" ")); > - print_key_value(out, key_part, min_key); > + print_key_value(out, key_part, range->start_key.key, > + range->start_key.length); > return; > } > > if (!(flag & NO_MIN_RANGE)) > { > - print_key_value(out, key_part, min_key); > + print_key_value(out, key_part, range->start_key.key, > + range->start_key.length); > if (flag & NEAR_MIN) > out->append(STRING_WITH_LEN(" < ")); > else > out->append(STRING_WITH_LEN(" <= ")); > } > > - out->append(key_part->field->field_name); > + print_keyparts_name(out, key_part, n_key_parts, keypart_map); > > if (!(flag & NO_MAX_RANGE)) > { > @@ -15767,7 +15754,8 @@ void append_range(String *out, const KEY_PART_INFO *key_part, > out->append(STRING_WITH_LEN(" < ")); > else > out->append(STRING_WITH_LEN(" <= ")); > - print_key_value(out, key_part, max_key); > + print_key_value(out, key_part, range->end_key.key, > + range->end_key.length); > } > } > > @@ -15775,60 +15763,39 @@ void append_range(String *out, const KEY_PART_INFO *key_part, > > Add ranges to the trace > For ex: > - query: select * from t1 where a=2 ; > - and we have an index on a , so we create a range > - 2 <= a <= 2 > + lets say we have an index a_b(a,b) > + query: select * from t1 where a=2 and b=4 ; > + so we create a range: > + (2,4) <= (a,b) <= (2,4) > this is added to the trace > */ > > static void append_range_all_keyparts(Json_writer_array *range_trace, > - String *range_string, > - String *range_so_far, const SEL_ARG *keypart, > + PARAM param, uint idx, > + SEL_ARG *keypart, > const KEY_PART_INFO *key_parts) > { > - > - DBUG_ASSERT(keypart); > - DBUG_ASSERT(keypart && keypart != &null_element); > - > - // Navigate to first interval in red-black tree > + SEL_ARG_RANGE_SEQ seq; > + KEY_MULTI_RANGE range; > + range_seq_t seq_it; > + uint flags= 0; > + RANGE_SEQ_IF seq_if = {NULL, sel_arg_range_seq_init, > + sel_arg_range_seq_next, 0, 0}; > + KEY *keyinfo= param.table->key_info + param.real_keynr[idx]; > + uint n_key_parts= param.table->actual_n_key_parts(keyinfo); > + seq.keyno= idx; > + seq.real_keyno= param.real_keynr[idx]; > + seq.param= &param; > + seq.start= keypart; > const KEY_PART_INFO *cur_key_part= key_parts + keypart->part; > - const SEL_ARG *keypart_range= keypart->first(); > - const size_t save_range_so_far_length= range_so_far->length(); > - > + seq_it= seq_if.init((void *) &seq, 0, flags); > > - while (keypart_range) > + while (!seq_if.next(seq_it, &range)) > { > - // Append the current range predicate to the range String > - switch (keypart->type) > - { > - case SEL_ARG::Type::KEY_RANGE: > - append_range(range_so_far, cur_key_part, keypart_range->min_value, > - keypart_range->max_value, > - keypart_range->min_flag | keypart_range->max_flag); > - break; > - case SEL_ARG::Type::MAYBE_KEY: > - range_so_far->append("MAYBE_KEY"); > - break; > - case SEL_ARG::Type::IMPOSSIBLE: > - range_so_far->append("IMPOSSIBLE"); > - break; > - default: > - DBUG_ASSERT(false); > - break; > - } > - > - if (keypart_range->next_key_part && > - keypart_range->next_key_part->part == > - keypart_range->part + 1 && > - keypart_range->is_singlepoint()) > - { > - append_range_all_keyparts(range_trace, range_string, range_so_far, > - keypart_range->next_key_part, key_parts); > - } > - else > - range_trace->add(range_so_far->c_ptr_safe(), range_so_far->length()); > - keypart_range= keypart_range->next; > - range_so_far->length(save_range_so_far_length); > + String range_info; > + range_info.set_charset(system_charset_info); > + append_range(&range_info, cur_key_part, &range, n_key_parts); > + range_trace->add(range_info.c_ptr_safe(), range_info.length()); > } > } > > @@ -15838,70 +15805,110 @@ static void append_range_all_keyparts(Json_writer_array *range_trace, > @param[out] out String the key is appended to > @param[in] key_part Index components description > @param[in] key Key tuple > + @param[in] used_length length of the key tuple > */ > + > static void print_key_value(String *out, const KEY_PART_INFO *key_part, > - const uchar *key) > + const uchar* key, uint used_length) > { > + out->append(STRING_WITH_LEN("(")); > Field *field= key_part->field; > + StringBuffer<128> tmp(system_charset_info); > + TABLE *table= field->table; > + uint store_length; > + my_bitmap_map *old_sets[2]; > + dbug_tmp_use_all_columns(table, old_sets, table->read_set, table->write_set); > + const uchar *key_end= key+used_length; > > - if (field->flags & BLOB_FLAG) > + for (; key < key_end; key+=store_length, key_part++) > { > - // Byte 0 of a nullable key is the null-byte. If set, key is NULL. > - if (field->real_maybe_null() && *key) > - out->append(STRING_WITH_LEN("NULL")); > - else > - (field->type() == MYSQL_TYPE_GEOMETRY) > - ? out->append(STRING_WITH_LEN("unprintable_geometry_value")) > - : out->append(STRING_WITH_LEN("unprintable_blob_value")); > - return; > - } > + field= key_part->field; > + store_length= key_part->store_length; > + if (field->flags & BLOB_FLAG) > + { > + // Byte 0 of a nullable key is the null-byte. If set, key is NULL. > + if (field->real_maybe_null() && *key) > + out->append(STRING_WITH_LEN("NULL")); > + else > + (field->type() == MYSQL_TYPE_GEOMETRY) > + ? out->append(STRING_WITH_LEN("unprintable_geometry_value")) > + : out->append(STRING_WITH_LEN("unprintable_blob_value")); > + goto next; > + } > > - uint store_length= key_part->store_length; > + if (field->real_maybe_null()) > + { > + /* > + Byte 0 of key is the null-byte. If set, key is NULL. > + Otherwise, print the key value starting immediately after the > + null-byte > + */ > + if (*key) > + { > + out->append(STRING_WITH_LEN("NULL")); > + goto next; > + } > + key++; // Skip null byte > + store_length--; > + } > > - if (field->real_maybe_null()) > - { > /* > - Byte 0 of key is the null-byte. If set, key is NULL. > - Otherwise, print the key value starting immediately after the > - null-byte > + Binary data cannot be converted to UTF8 which is what the > + optimizer trace expects. If the column is binary, the hex > + representation is printed to the trace instead. > */ > - if (*key) > + if (field->flags & BINARY_FLAG) > { > - out->append(STRING_WITH_LEN("NULL")); > - return; > + out->append("0x"); > + for (uint i = 0; i < store_length; i++) > + { > + out->append(_dig_vec_lower[*(key + i) >> 4]); > + out->append(_dig_vec_lower[*(key + i) & 0x0F]); > + } > + goto next; > } > - key++; // Skip null byte > - store_length--; > + > + field->set_key_image(key, key_part->length); > + if (field->type() == MYSQL_TYPE_BIT) > + (void)field->val_int_as_str(&tmp, 1); // may change tmp's charset > + else > + field->val_str(&tmp); // may change tmp's charset > + out->append(tmp.ptr(), tmp.length(), tmp.charset()); > + > + next: > + if (key + store_length < key_end) > + out->append(STRING_WITH_LEN(",")); > } > + dbug_tmp_restore_column_maps(table->read_set, table->write_set, old_sets); > + out->append(STRING_WITH_LEN(")")); > +} > > - /* > - Binary data cannot be converted to UTF8 which is what the > - optimizer trace expects. If the column is binary, the hex > - representation is printed to the trace instead. > - */ > - if (field->flags & BINARY_FLAG) > +/** > + Print key parts involed in a range > + @param[out] out String the key is appended to > + @param[in] key_part Index components description > + @param[in] n_keypart Number of keyparts in index > + @param[in] keypart_map map for keyparts involved in the range > +*/ > + > +void print_keyparts_name(String *out, const KEY_PART_INFO *key_part, > + uint n_keypart, key_part_map keypart_map) > +{ > + uint i; > + out->append(STRING_WITH_LEN("(")); > + bool first_keypart= TRUE; > + for (i=0; i < n_keypart; key_part++, i++) > { > - out->append("0x"); > - for (uint i = 0; i < store_length; i++) > + if (keypart_map & (1 << i)) > { > - out->append(_dig_vec_lower[*(key + i) >> 4]); > - out->append(_dig_vec_lower[*(key + i) & 0x0F]); > + if (first_keypart) > + first_keypart= FALSE; > + else > + out->append(STRING_WITH_LEN(",")); > + out->append(key_part->field->field_name); > } > - return; > + else > + break; > } > - > - StringBuffer<128> tmp(system_charset_info); > - TABLE *table= field->table; > - my_bitmap_map *old_sets[2]; > - > - dbug_tmp_use_all_columns(table, old_sets, table->read_set, table->write_set); > - > - field->set_key_image(key, key_part->length); > - if (field->type() == MYSQL_TYPE_BIT) > - (void)field->val_int_as_str(&tmp, 1); // may change tmp's charset > - else > - field->val_str(&tmp); // may change tmp's charset > - out->append(tmp.ptr(), tmp.length(), tmp.charset()); > - > - dbug_tmp_restore_column_maps(table->read_set, table->write_set, old_sets); > -} > + out->append(STRING_WITH_LEN(")")); > +} BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] 1581f65be57: MDEV-13301 Optimize DROP INDEX, ADD INDEX into RENAME INDEX
by Sergei Golubchik 02 Apr '19

02 Apr '19

Hi, Eugene! See the review below. This looked pretty much ok, a couple of style comments. And why did you not implement the new ALTER TABLE .. .RENAME INDEX syntax? I agree that a pair of DROP/ADD should act as RENAME. But a dedicated RENAME would be also good to have, at least for compatibility reasons. On Apr 01, Eugene Kosov wrote: > revision-id: 1581f65be57 (mariadb-10.4.3-69-g1581f65be57) > parent(s): e012d266807 > author: Eugene Kosov <claprix(a)yandex.ru> > committer: Eugene Kosov <claprix(a)yandex.ru> > timestamp: 2019-03-15 18:44:05 +0300 > message: > > MDEV-13301 Optimize DROP INDEX, ADD INDEX into RENAME INDEX > > Just rename index in data dictionary and in InnoDB cache when it's possible. > Introduce ALTER_INDEX_RENAME for that purpose so that engines can optimize > such operation. > > Unused code between macro MYSQL_RENAME_INDEX was removed. > > compare_keys_but_name(): compare index definitions except for index names > > Alter_inplace_info::rename_keys: > ha_innobase_inplace_ctx::rename_keys: vector of rename indexes > > fill_alter_inplace_info():: fills Alter_inplace_info::rename_keys > > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index e1337a0f710..e1131422117 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -7062,6 +7061,43 @@ static bool fill_alter_inplace_info(THD *thd, TABLE *table, bool varchar, > new_key->option_struct; > } > > + uint &add_count= ha_alter_info->index_add_count; Coding style: no modifiable references please. write ha_alter_info->index_add_count++ same below > + for (uint i= 0; i < add_count; i++) > + { > + uint *&add_buffer= ha_alter_info->index_add_buffer; > + const KEY *new_key= ha_alter_info->key_info_buffer + add_buffer[i]; > + > + uint &drop_count= ha_alter_info->index_drop_count; > + for (uint j= 0; j < drop_count; j++) > + { > + KEY **&drop_buffer= ha_alter_info->index_drop_buffer; > + const KEY *old_key= drop_buffer[j]; > + > + if (!lex_string_cmp(system_charset_info, &old_key->name, &new_key->name)) > + continue; Why? I'd think and if it's Compare_keys::Equal and the name is the same too, you can simply remove add/drop elements without adding a rename element. > + > + if (Compare_keys::Equal != compare_keys_but_name(old_key, new_key, > + alter_info, table, > + new_pk, old_pk)) Coding style: don't invert comparisons > + { > + continue; > + } > + > + ha_alter_info->handler_flags|= ALTER_RENAME_INDEX; > + ha_alter_info->rename_keys.push_back( > + Alter_inplace_info::Rename_key_pair(old_key, new_key)); > + > + memcpy(add_buffer + i, add_buffer + i + 1, > + sizeof(add_buffer[0]) * (add_count - i - 1)); > + memcpy(drop_buffer + j, drop_buffer + j + 1, > + sizeof(drop_buffer[0]) * (drop_count - j - 1)); > + --add_count; > + --drop_count; > + --i; // this index once again > + break; > + } > + } > + Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 3

Re: [Maria-developers] 126a9b6db25: MDEV-18875 Assertion `thd->transaction.stmt.ha_list == __null || trans == &thd->transaction.stmt' failed or bogus ER_DUP_ENTRY upon ALTER TABLE with versioning
by Sergei Golubchik 02 Apr '19

02 Apr '19

Hi, Nikita! On Apr 02, Nikita Malyavin wrote: > revision-id: 126a9b6db25 (mariadb-10.3.12-101-g126a9b6db25) > parent(s): 1c60f40868f > author: Nikita Malyavin <nikitamalyavin(a)gmail.com> > committer: Nikita Malyavin <nikitamalyavin(a)gmail.com> > timestamp: 2019-03-23 03:37:10 +1000 > message: > > MDEV-18875 Assertion `thd->transaction.stmt.ha_list == __null || trans == &thd->transaction.stmt' failed or bogus ER_DUP_ENTRY upon ALTER TABLE with versioning > > diff --git a/sql/handler.cc b/sql/handler.cc > index e01392d236c..9d295afb631 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -1420,7 +1420,8 @@ int ha_commit_trans(THD *thd, bool all) > > #if 1 // FIXME: This should be done in ha_prepare(). > if (rw_trans || (thd->lex->sql_command == SQLCOM_ALTER_TABLE && > - thd->lex->alter_info.flags & ALTER_ADD_SYSTEM_VERSIONING)) > + thd->lex->alter_info.flags & ALTER_ADD_SYSTEM_VERSIONING && > + is_real_trans)) Why there's a special check for SQLCOM_ALTER_TABLE + ALTER_ADD_SYSTEM_VERSIONING in the first place? Does it mean that ALTER TABLE ... ADD SYSTEM VERSIONING does not marks a transaction as read/write? That's be weird. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

Re: [Maria-developers] 5469d88e7b4: MDEV-19049 Server crashes in check_duplicate_long_entry_key, ASAN stack-buffer-overflow in Field_blob::get_key_image
by Sergei Golubchik 02 Apr '19

02 Apr '19

Hi, Sachin! On Apr 02, Sachin Setiya wrote: > revision-id: 5469d88e7b4 (mariadb-10.4.3-104-g5469d88e7b4) > parent(s): 0bc42602266 > author: Sachin <sachin.setiya(a)mariadb.com> > committer: Sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-28 11:29:25 +0530 > message: > > MDEV-19049 Server crashes in check_duplicate_long_entry_key, ASAN stack-buffer-overflow in Field_blob::get_key_image > > Long Unique keys should always be last unique key. Why? > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index ad62ecc1103..c755a74e174 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -2796,6 +2797,14 @@ static int sort_keys(KEY *a, KEY *b) > { > if (!(b_flags & HA_NOSAME)) > return -1; > + /* > + Long Unique keys should always be last unique key. > + Before this patch they used to change order wrt to partial keys (MDEV-19049) don't write the code history "before this patch it worked like that" in the code itself. When you look at the code there is no "patch", just a comment. The concepts of "patch" and "before it used to be" belongs to the history, and logically should be in the commit comment, not in a code comment. > + */ > + if (a->algorithm == HA_KEY_ALG_LONG_HASH) > + return 1; > + if (b->algorithm == HA_KEY_ALG_LONG_HASH) > + return -1; > if ((a_flags ^ b_flags) & HA_NULL_PART_KEY) > { > /* Sort NOT NULL keys before other keys */ > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0