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= ¶m->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= ¶m; + 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