Skip to content
Permalink
Browse files
MDEV-25020: Range optimizer regression for key IN (const, ....)
(addressed review input)
The issue was introduced by @@optimizer_max_sel_arg_weight code.
key_or() calls SEL_ARG::update_weight_locally().  That function
takes O(tree->elements) time.
Without that call, key_or(big_tree, one_element_tree) would take
O(log(big_tree)) when one_element_tree doesn't overlap with elements
of big_tree.
This means, update_weight_locally() can cause a big slowdown.

The fix:
1. key_or() actually doesn't need to call update_weight_locally().
  It calls SEL_ARG::tree_delete() and SEL_ARG::insert(). These functions
  update SEL_ARG::weight.
  It also manipulates the SEL_ARG objects directly, but these
  modifications do not change the weight of the tree.
  I've just removed the update_weight_locally() call.
2. and_all_keys() also calls update_weight_locally(). It manipulates the
  SEL_ARG graph directly.
  Removed that call and added the code to update the SEL_ARG graph weight.

Tests main.range and main.range_not_embedded already contain the queries
that have test coverage for the affected code.
  • Loading branch information
spetrunia committed Aug 1, 2022
1 parent 098c0f2 commit 90ba999
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 30 deletions.
@@ -9855,34 +9855,37 @@ and_all_keys(RANGE_OPT_PARAM *param, SEL_ARG *key1, SEL_ARG *key2,
return key2;
key1->right= key1->left= &null_element;
key1->next= key1->prev= 0;
key1->weight= 1 + (key1->next_key_part? key1->next_key_part->weight: 0);
}

for (next=key1->first(); next ; next=next->next)
{
if (next->next_key_part)
{
uint old_weight= next->next_key_part->weight;
SEL_ARG *tmp= key_and(param, next->next_key_part, key2, clone_flag);
if (tmp && tmp->type == SEL_ARG::IMPOSSIBLE)
{
key1=key1->tree_delete(next);
continue;
}
next->next_key_part=tmp;
key1->weight+= (tmp? tmp->weight: 0) - old_weight;
if (use_count)
next->increment_use_count(use_count);
if (param->alloced_sel_args > SEL_ARG::MAX_SEL_ARGS)
break;
}
else
{
next->next_key_part=key2;
key1->weight += key2->weight;
}
}
if (!key1)
return &null_element; // Impossible ranges
key1->use_count++;

/* Re-compute the result tree's weight. */
key1->update_weight_locally();

key1->max_part_no= MY_MAX(key2->max_part_no, key2->part+1);
return key1;
}
@@ -10046,29 +10049,6 @@ get_range(SEL_ARG **e1,SEL_ARG **e2,SEL_ARG *root1)
return 0;
}

/*
@brief
Update the tree weight.

@detail
Utility function to be called on a SEL_ARG tree root after doing local
modifications concerning changes at this key part.
Assumes that the weight of the graphs connected via next_key_part is
up to dayte.
*/
void SEL_ARG::update_weight_locally()
{
uint new_weight= 0;
const SEL_ARG *sl;
for (sl= first(); sl ; sl= sl->next)
{
new_weight++;
if (sl->next_key_part)
new_weight += sl->next_key_part->weight;
}
weight= new_weight;
}


#ifndef DBUG_OFF
/*
@@ -10828,9 +10808,6 @@ key_or(RANGE_OPT_PARAM *param, SEL_ARG *key1,SEL_ARG *key2)
}
key1->use_count++;

/* Re-compute the result tree's weight. */
key1->update_weight_locally();

key1->max_part_no= max_part_no;
return key1;
}
@@ -317,7 +317,6 @@ class SEL_ARG :public Sql_alloc
uint weight;
enum { MAX_WEIGHT = 32000 };

void update_weight_locally();
#ifndef DBUG_OFF
uint verify_weight();
#endif

0 comments on commit 90ba999

Please sign in to comment.