maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #11777
Re: [Commits] a1929d001c7: MDEV-18741: Optimizer trace: multi-part key ranges are printed incorrectly
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