From 90ba999e800957c230290bd438a3e73628222a74 Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Wed, 27 Jul 2022 17:37:03 +0300 Subject: [PATCH] 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. --- sql/opt_range.cc | 35 ++++++----------------------------- sql/opt_range.h | 1 - 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/sql/opt_range.cc b/sql/opt_range.cc index e3287e1bbea..1cc573f90f2 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -9855,12 +9855,14 @@ 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) { @@ -9868,21 +9870,22 @@ and_all_keys(RANGE_OPT_PARAM *param, SEL_ARG *key1, SEL_ARG *key2, 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 @@ end: } key1->use_count++; - /* Re-compute the result tree's weight. */ - key1->update_weight_locally(); - key1->max_part_no= max_part_no; return key1; } diff --git a/sql/opt_range.h b/sql/opt_range.h index 1014176ecc5..5594397e709 100644 --- a/sql/opt_range.h +++ b/sql/opt_range.h @@ -317,7 +317,6 @@ public: uint weight; enum { MAX_WEIGHT = 32000 }; - void update_weight_locally(); #ifndef DBUG_OFF uint verify_weight(); #endif