- commits - lists.mariadb.org

[Commits] 33ede50f207: MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value
by psergey 28 Jan '21

28 Jan '21

revision-id: 33ede50f207552df835d7606f990fa9ccc4e0d12 (mariadb-10.2.31-720-g33ede50f207) parent(s): 3a89ae3364ce415caed2ab5b008a454ce6204f59 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-01-28 20:46:13 +0300 message: MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value Apply the patch based on the patch by Varun Gupta: PARAM::is_ror_scan might be used unitialized when check_quick_select() is invoked for a "degenerate" SEL_ARG tree (e.g. one having type SEL_ARG::IMPOSSIBLE). Make check_quick_select() always initialize PARAM::is_ror_scan. --- mysql-test/r/range.result | 17 +++++++++++++++++ mysql-test/r/range_mrr_icp.result | 17 +++++++++++++++++ mysql-test/t/range.test | 14 ++++++++++++++ sql/opt_range.cc | 1 + 4 files changed, 49 insertions(+) diff --git a/mysql-test/r/range.result b/mysql-test/r/range.result index 1d07cb04c06..6a3850c0ed9 100644 --- a/mysql-test/r/range.result +++ b/mysql-test/r/range.result @@ -3184,5 +3184,22 @@ SELECT * FROM t1 JOIN t2 ON (t2.code = t1.b) WHERE t1.a NOT IN ('baz', 'qux') OR id a b code num DROP TABLE t1, t2; # +# MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value +# +create table t1 (pk int, i int, v int, primary key (pk), key(v)); +insert into t1 (pk,i,v) values (1,1,2),(2,2,4),(3,3,6),(4,4,8),(5,5,10),(6,6,12),(7,7,14),(8,8,16); +create table t2 (a int, b int); +insert into t2 values (1,2),(2,4); +EXPLAIN +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 ALL NULL NULL NULL NULL 2 +1 SIMPLE t1 ALL PRIMARY,v NULL NULL NULL 8 Range checked for each record (index map: 0x3) +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +pk i v a b +1 1 2 1 2 +2 2 4 2 4 +drop table t1, t2; +# # End of 10.2 tests # diff --git a/mysql-test/r/range_mrr_icp.result b/mysql-test/r/range_mrr_icp.result index f3203fea73d..24f42f34ce5 100644 --- a/mysql-test/r/range_mrr_icp.result +++ b/mysql-test/r/range_mrr_icp.result @@ -3196,6 +3196,23 @@ SELECT * FROM t1 JOIN t2 ON (t2.code = t1.b) WHERE t1.a NOT IN ('baz', 'qux') OR id a b code num DROP TABLE t1, t2; # +# MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value +# +create table t1 (pk int, i int, v int, primary key (pk), key(v)); +insert into t1 (pk,i,v) values (1,1,2),(2,2,4),(3,3,6),(4,4,8),(5,5,10),(6,6,12),(7,7,14),(8,8,16); +create table t2 (a int, b int); +insert into t2 values (1,2),(2,4); +EXPLAIN +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 ALL NULL NULL NULL NULL 2 +1 SIMPLE t1 ALL PRIMARY,v NULL NULL NULL 8 Range checked for each record (index map: 0x3) +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +pk i v a b +1 1 2 1 2 +2 2 4 2 4 +drop table t1, t2; +# # End of 10.2 tests # set optimizer_switch=@mrr_icp_extra_tmp; diff --git a/mysql-test/t/range.test b/mysql-test/t/range.test index 2f55889afec..890377ed977 100644 --- a/mysql-test/t/range.test +++ b/mysql-test/t/range.test @@ -2217,6 +2217,20 @@ SELECT * FROM t1 JOIN t2 ON (t2.code = t1.b) WHERE t1.a NOT IN ('baz', 'qux') OR DROP TABLE t1, t2; + +--echo # +--echo # MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value +--echo # + +create table t1 (pk int, i int, v int, primary key (pk), key(v)); +insert into t1 (pk,i,v) values (1,1,2),(2,2,4),(3,3,6),(4,4,8),(5,5,10),(6,6,12),(7,7,14),(8,8,16); +create table t2 (a int, b int); +insert into t2 values (1,2),(2,4); +EXPLAIN +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +drop table t1, t2; + --echo # --echo # End of 10.2 tests --echo # diff --git a/sql/opt_range.cc b/sql/opt_range.cc index 7785c768fbc..f3f184367c9 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -10385,6 +10385,7 @@ ha_rows check_quick_select(PARAM *param, uint idx, bool index_only, uint keynr= param->real_keynr[idx]; DBUG_ENTER("check_quick_select"); + param->is_ror_scan= FALSE; /* Handle cases when we don't have a valid non-empty list of range */ if (!tree) DBUG_RETURN(HA_POS_ERROR);

1 0

[Commits] 46271662a71: MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value
by varun 28 Jan '21

28 Jan '21

revision-id: 46271662a71c4ba7826a7760059c9853586ed424 (mariadb-10.2.31-719-g46271662a71) parent(s): 20f6c403eb976a6dd25cb58d0ce17f6da2566253 author: Varun Gupta committer: Varun Gupta timestamp: 2021-01-28 18:09:58 +0530 message: MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value The parameter is_ror_scan was not initialized. It should be initialized whenever we try to call check_quick_select() for a SEL_ARG. --- mysql-test/r/range.result | 17 +++++++++++++++++ mysql-test/r/range_mrr_icp.result | 17 +++++++++++++++++ mysql-test/t/range.test | 14 ++++++++++++++ sql/opt_range.cc | 2 ++ 4 files changed, 50 insertions(+) diff --git a/mysql-test/r/range.result b/mysql-test/r/range.result index 1d07cb04c06..6a3850c0ed9 100644 --- a/mysql-test/r/range.result +++ b/mysql-test/r/range.result @@ -3184,5 +3184,22 @@ SELECT * FROM t1 JOIN t2 ON (t2.code = t1.b) WHERE t1.a NOT IN ('baz', 'qux') OR id a b code num DROP TABLE t1, t2; # +# MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value +# +create table t1 (pk int, i int, v int, primary key (pk), key(v)); +insert into t1 (pk,i,v) values (1,1,2),(2,2,4),(3,3,6),(4,4,8),(5,5,10),(6,6,12),(7,7,14),(8,8,16); +create table t2 (a int, b int); +insert into t2 values (1,2),(2,4); +EXPLAIN +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 ALL NULL NULL NULL NULL 2 +1 SIMPLE t1 ALL PRIMARY,v NULL NULL NULL 8 Range checked for each record (index map: 0x3) +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +pk i v a b +1 1 2 1 2 +2 2 4 2 4 +drop table t1, t2; +# # End of 10.2 tests # diff --git a/mysql-test/r/range_mrr_icp.result b/mysql-test/r/range_mrr_icp.result index f3203fea73d..24f42f34ce5 100644 --- a/mysql-test/r/range_mrr_icp.result +++ b/mysql-test/r/range_mrr_icp.result @@ -3196,6 +3196,23 @@ SELECT * FROM t1 JOIN t2 ON (t2.code = t1.b) WHERE t1.a NOT IN ('baz', 'qux') OR id a b code num DROP TABLE t1, t2; # +# MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value +# +create table t1 (pk int, i int, v int, primary key (pk), key(v)); +insert into t1 (pk,i,v) values (1,1,2),(2,2,4),(3,3,6),(4,4,8),(5,5,10),(6,6,12),(7,7,14),(8,8,16); +create table t2 (a int, b int); +insert into t2 values (1,2),(2,4); +EXPLAIN +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 ALL NULL NULL NULL NULL 2 +1 SIMPLE t1 ALL PRIMARY,v NULL NULL NULL 8 Range checked for each record (index map: 0x3) +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +pk i v a b +1 1 2 1 2 +2 2 4 2 4 +drop table t1, t2; +# # End of 10.2 tests # set optimizer_switch=@mrr_icp_extra_tmp; diff --git a/mysql-test/t/range.test b/mysql-test/t/range.test index 2f55889afec..890377ed977 100644 --- a/mysql-test/t/range.test +++ b/mysql-test/t/range.test @@ -2217,6 +2217,20 @@ SELECT * FROM t1 JOIN t2 ON (t2.code = t1.b) WHERE t1.a NOT IN ('baz', 'qux') OR DROP TABLE t1, t2; + +--echo # +--echo # MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value +--echo # + +create table t1 (pk int, i int, v int, primary key (pk), key(v)); +insert into t1 (pk,i,v) values (1,1,2),(2,2,4),(3,3,6),(4,4,8),(5,5,10),(6,6,12),(7,7,14),(8,8,16); +create table t2 (a int, b int); +insert into t2 values (1,2),(2,4); +EXPLAIN +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +drop table t1, t2; + --echo # --echo # End of 10.2 tests --echo # diff --git a/sql/opt_range.cc b/sql/opt_range.cc index 7785c768fbc..b98d773aa3a 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -2457,6 +2457,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, param.remove_false_where_parts= remove_false_parts_of_where; param.force_default_mrr= ordered_output; param.possible_keys.clear_all(); + param.is_ror_scan= FALSE; thd->no_errors=1; // Don't warn about NULL init_sql_alloc(&alloc, thd->variables.range_alloc_block_size, 0, @@ -6819,6 +6820,7 @@ static TRP_RANGE *get_key_scans_params(PARAM *param, SEL_TREE *tree, double found_read_time; uint mrr_flags, buf_size; INDEX_SCAN_INFO *index_scan; + param->is_ror_scan= FALSE; uint keynr= param->real_keynr[idx]; if (key->type == SEL_ARG::MAYBE_KEY || key->maybe_flag)

2 2

[Commits] 1d73c64d0e9: Address review input part #2:
by Sergei Petrunia 27 Jan '21

27 Jan '21

revision-id: 1d73c64d0e9ca4b6d63d8dcebb69d9789632bd6f (mariadb-10.4.11-424-g1d73c64d0e9) parent(s): 36791ad69aa1d970c3621a113affa7f50fade0d0 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-01-27 19:40:47 +0300 message: Address review input part #2: - Do not recompute new_arg's weight in key_and(). It is auto-computed when the tree is built. --- sql/opt_range.cc | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/sql/opt_range.cc b/sql/opt_range.cc index e23da39b796..9f53bbec160 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -9871,7 +9871,6 @@ key_and(RANGE_OPT_PARAM *param, SEL_ARG *key1, SEL_ARG *key2, uint clone_flag) key2->use_count--; SEL_ARG *e1=key1->first(), *e2=key2->first(), *new_tree=0; uint max_part_no= MY_MAX(key1->max_part_no, key2->max_part_no); - uint new_weight= 0; // Weight of the result tree while (e1 && e2) { @@ -9893,12 +9892,9 @@ key_and(RANGE_OPT_PARAM *param, SEL_ARG *key1, SEL_ARG *key2, uint clone_flag) if (!new_arg) return &null_element; // End of memory new_arg->next_key_part=next; - /* - We are going to add the 'new_arg' range to the new_tree. - the new_arg on its own has weight=1. - if it has non-null new_arg->next_key_part (=next), add its weight, too. - */ - new_weight += 1 + (next? next->weight: 0); + if (new_arg->next_key_part) + new_arg->weight += new_arg->next_key_part->weight; + if (!new_tree) { new_tree=new_arg; @@ -9916,7 +9912,6 @@ key_and(RANGE_OPT_PARAM *param, SEL_ARG *key1, SEL_ARG *key2, uint clone_flag) if (!new_tree) return &null_element; // Impossible range new_tree->max_part_no= max_part_no; - new_tree->weight= new_weight; return new_tree; } @@ -10901,8 +10896,13 @@ SEL_ARG::insert(SEL_ARG *key) SEL_ARG *root=rb_insert(key); // rebalance tree root->use_count=this->use_count; // copy root info root->elements= this->elements+1; - // Add the weight: weight of this element (=1) + next_key_part's weight - root->weight += 1 + (next_key_part? next_key_part->weight: 0); + /* + The new weight is: + old root's weight + +1 for the weight of the added element + + next_key_part's weight of the added element + */ + root->weight = weight + 1 + (key->next_key_part? key->next_key_part->weight: 0); root->maybe_flag=this->maybe_flag; return root; }

1 0

[Commits] 36791ad69aa: Fix a typo
by Sergei Petrunia 27 Jan '21

27 Jan '21

revision-id: 36791ad69aa1d970c3621a113affa7f50fade0d0 (mariadb-10.4.11-423-g36791ad69aa) parent(s): 5beb962ec229d13e2bab2c18427a915ae92c7209 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-01-27 15:09:13 +0300 message: Fix a typo --- sql/opt_range.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/opt_range.cc b/sql/opt_range.cc index f5e757c5c81..e23da39b796 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -10968,8 +10968,8 @@ SEL_ARG::tree_delete(SEL_ARG *key) uint new_weight= root->weight - (1 + (key->next_key_part? key->next_key_part->weight : 0)); - DBUG_ASSERT(root->weight > (1 + (key->next_key_part ? - key->next_key_part->weight : 0))); + DBUG_ASSERT(root->weight >= (1 + (key->next_key_part ? + key->next_key_part->weight : 0))); /* Unlink from list */ if (key->prev)

1 0

[Commits] 5beb962ec22: Address review input, part #1 (obvious fixes)
by Sergei Petrunia 27 Jan '21

27 Jan '21

revision-id: 5beb962ec229d13e2bab2c18427a915ae92c7209 (mariadb-10.4.11-422-g5beb962ec22) parent(s): 5cac06a04a468b421e39fff3daab7f93c28063f5 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-01-27 14:40:27 +0300 message: Address review input, part #1 (obvious fixes) --- sql/opt_range.cc | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/sql/opt_range.cc b/sql/opt_range.cc index df9d6ba68e0..f5e757c5c81 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -9769,8 +9769,8 @@ and_all_keys(RANGE_OPT_PARAM *param, SEL_ARG *key1, SEL_ARG *key2, if (!key1) return &null_element; // Impossible ranges key1->use_count++; - key1->max_part_no= MY_MAX(key2->max_part_no, key2->part+1); key1->weight= new_weight; + key1->max_part_no= MY_MAX(key2->max_part_no, key2->part+1); return key1; } @@ -9840,7 +9840,7 @@ key_and(RANGE_OPT_PARAM *param, SEL_ARG *key1, SEL_ARG *key2, uint clone_flag) key1->next_key_part=key_and(param, key1->next_key_part, key2->next_key_part, clone_flag); - key1->weight = 1 + (key1->next_key_part? key1->next_key_part->weight : 0); + key1->weight= 1 + (key1->next_key_part? key1->next_key_part->weight : 0); if (key1->next_key_part && key1->next_key_part->type == SEL_ARG::IMPOSSIBLE) @@ -9893,6 +9893,11 @@ key_and(RANGE_OPT_PARAM *param, SEL_ARG *key1, SEL_ARG *key2, uint clone_flag) if (!new_arg) return &null_element; // End of memory new_arg->next_key_part=next; + /* + We are going to add the 'new_arg' range to the new_tree. + the new_arg on its own has weight=1. + if it has non-null new_arg->next_key_part (=next), add its weight, too. + */ new_weight += 1 + (next? next->weight: 0); if (!new_tree) { @@ -9948,7 +9953,7 @@ uint SEL_ARG::verify_weight() { for (SEL_ARG *arg= first_arg; arg; arg= arg->next) { - computed_weight += 1; + computed_weight++; if (arg->next_key_part) computed_weight+= arg->next_key_part->verify_weight(); } @@ -9966,6 +9971,7 @@ uint SEL_ARG::verify_weight() { sql_print_error("SEL_ARG weight mismatch: computed %u have %u\n", computed_weight, weight); + DBUG_ASSERT(computed_weight == weight); // Fail an assertion } return computed_weight; } @@ -10715,7 +10721,7 @@ uint SEL_ARG::get_max_key_part() const if (cur->next_key_part) { uint mp= cur->next_key_part->get_max_key_part(); - max_part = MY_MAX(part, mp); + max_part= MY_MAX(part, mp); } } return max_part; @@ -10746,10 +10752,9 @@ void prune_sel_arg_graph(SEL_ARG *sel_arg, uint max_part) } else { - uint old_weight = cur->next_key_part->weight; + uint old_weight= cur->next_key_part->weight; prune_sel_arg_graph(cur->next_key_part, max_part); - old_weight -= cur->next_key_part->weight; - sel_arg->weight -= old_weight; + sel_arg->weight -= (old_weight - cur->next_key_part->weight); } } } @@ -10791,12 +10796,17 @@ SEL_ARG *enforce_sel_arg_weight_limit(RANGE_OPT_PARAM *param, uint keyno, while (1) { - if (sel_arg->weight <= param->thd->variables.optimizer_max_sel_arg_weight) + if (likely(sel_arg->weight <= param->thd->variables. + optimizer_max_sel_arg_weight)) break; uint max_part= sel_arg->get_max_key_part(); if (max_part == sel_arg->part) { + /* + We don't return NULL right away as we want to have the information + about the changed tree in the optimizer trace. + */ sel_arg= NULL; break; } @@ -10950,11 +10960,17 @@ SEL_ARG::tree_delete(SEL_ARG *key) root=this; this->parent= 0; - // Compute the weight the tree will have after the element is removed - // We remove the element itself (weight=1) - // and the sub-graph connected to its next_key_part. + /* + Compute the weight the tree will have after the element is removed. + We remove the element itself (weight=1) + and the sub-graph connected to its next_key_part. + */ uint new_weight= root->weight - (1 + (key->next_key_part? key->next_key_part->weight : 0)); + + DBUG_ASSERT(root->weight > (1 + (key->next_key_part ? + key->next_key_part->weight : 0))); + /* Unlink from list */ if (key->prev) key->prev->next=key->next; @@ -11006,7 +11022,7 @@ SEL_ARG::tree_delete(SEL_ARG *key) test_rb_tree(root,root->parent); root->use_count=this->use_count; // Fix root counters - root->weight = new_weight; + root->weight= new_weight; root->elements=this->elements-1; root->maybe_flag=this->maybe_flag; DBUG_RETURN(root);

1 0

[Commits] 81787adb79a: Address review input, part #1 (obvious fixes)
by Sergei Petrunia 27 Jan '21

27 Jan '21

revision-id: 81787adb79a71e47a506a3c780019fdbba3aafe2 (mariadb-10.4.11-422-g81787adb79a) parent(s): 5cac06a04a468b421e39fff3daab7f93c28063f5 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-01-27 14:32:09 +0300 message: Address review input, part #1 (obvious fixes) --- sql/opt_range.cc | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/sql/opt_range.cc b/sql/opt_range.cc index df9d6ba68e0..48ac16e1de9 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -9769,8 +9769,8 @@ and_all_keys(RANGE_OPT_PARAM *param, SEL_ARG *key1, SEL_ARG *key2, if (!key1) return &null_element; // Impossible ranges key1->use_count++; - key1->max_part_no= MY_MAX(key2->max_part_no, key2->part+1); key1->weight= new_weight; + key1->max_part_no= MY_MAX(key2->max_part_no, key2->part+1); return key1; } @@ -9840,7 +9840,7 @@ key_and(RANGE_OPT_PARAM *param, SEL_ARG *key1, SEL_ARG *key2, uint clone_flag) key1->next_key_part=key_and(param, key1->next_key_part, key2->next_key_part, clone_flag); - key1->weight = 1 + (key1->next_key_part? key1->next_key_part->weight : 0); + key1->weight= 1 + (key1->next_key_part? key1->next_key_part->weight : 0); if (key1->next_key_part && key1->next_key_part->type == SEL_ARG::IMPOSSIBLE) @@ -9893,6 +9893,11 @@ key_and(RANGE_OPT_PARAM *param, SEL_ARG *key1, SEL_ARG *key2, uint clone_flag) if (!new_arg) return &null_element; // End of memory new_arg->next_key_part=next; + /* + We are going to add the 'new_arg' range to the new_tree. + the new_arg on its own has weight=1. + if it has non-null new_arg->next_key_part (=next), add its weight, too. + */ new_weight += 1 + (next? next->weight: 0); if (!new_tree) { @@ -9948,7 +9953,7 @@ uint SEL_ARG::verify_weight() { for (SEL_ARG *arg= first_arg; arg; arg= arg->next) { - computed_weight += 1; + computed_weight++; if (arg->next_key_part) computed_weight+= arg->next_key_part->verify_weight(); } @@ -9966,6 +9971,7 @@ uint SEL_ARG::verify_weight() { sql_print_error("SEL_ARG weight mismatch: computed %u have %u\n", computed_weight, weight); + DBUG_ASSERT(computed_weight == weight); // Fail an assertion } return computed_weight; } @@ -10715,7 +10721,7 @@ uint SEL_ARG::get_max_key_part() const if (cur->next_key_part) { uint mp= cur->next_key_part->get_max_key_part(); - max_part = MY_MAX(part, mp); + max_part= MY_MAX(part, mp); } } return max_part; @@ -10746,10 +10752,9 @@ void prune_sel_arg_graph(SEL_ARG *sel_arg, uint max_part) } else { - uint old_weight = cur->next_key_part->weight; + uint old_weight= cur->next_key_part->weight; prune_sel_arg_graph(cur->next_key_part, max_part); - old_weight -= cur->next_key_part->weight; - sel_arg->weight -= old_weight; + sel_arg->weight -= (old_weight - cur->next_key_part->weight); } } } @@ -10791,12 +10796,17 @@ SEL_ARG *enforce_sel_arg_weight_limit(RANGE_OPT_PARAM *param, uint keyno, while (1) { - if (sel_arg->weight <= param->thd->variables.optimizer_max_sel_arg_weight) + if (likely(sel_arg->weight <= param->thd->variables. + optimizer_max_sel_arg_weight)) break; uint max_part= sel_arg->get_max_key_part(); if (max_part == sel_arg->part) { + /* + We don't return NULL right away as we want to have the information + about the changed tree in the optimizer trace. + */ sel_arg= NULL; break; } @@ -10950,11 +10960,17 @@ SEL_ARG::tree_delete(SEL_ARG *key) root=this; this->parent= 0; - // Compute the weight the tree will have after the element is removed - // We remove the element itself (weight=1) - // and the sub-graph connected to its next_key_part. + /* + Compute the weight the tree will have after the element is removed. + We remove the element itself (weight=1) + and the sub-graph connected to its next_key_part. + */ uint new_weight= root->weight - (1 + (key->next_key_part? key->next_key_part->weight : 0)); + + DBUG_ASSERT(root->weight > (1 + (key->next_key_part ? + key->next_key_part->weight : 0)); + /* Unlink from list */ if (key->prev) key->prev->next=key->next; @@ -11006,7 +11022,7 @@ SEL_ARG::tree_delete(SEL_ARG *key) test_rb_tree(root,root->parent); root->use_count=this->use_count; // Fix root counters - root->weight = new_weight; + root->weight= new_weight; root->elements=this->elements-1; root->maybe_flag=this->maybe_flag; DBUG_RETURN(root);

1 0

[Commits] f3fc8db0065: MDEV-23291: SUM column from a derived table returns invalid values
by varun 27 Jan '21

27 Jan '21

revision-id: f3fc8db00653371972c777a2ee40b20d9ea6b62b (mariadb-10.2.31-712-gf3fc8db0065) parent(s): 7c9ce660cf715e06cf7b4b69e9c1011b01abdf74 author: Varun Gupta committer: Varun Gupta timestamp: 2021-01-27 14:32:52 +0530 message: MDEV-23291: SUM column from a derived table returns invalid values The issue here was the read_set bitmap was not set for a field which was used as a reference in an inner select. We need to make sure that if we are in an inner select and we have references from outer select then we update the table bitmaps for such references. Introduced a function in the class Item_subselect that would update bitmaps of table for the references within a subquery that are defined in outer selects. --- mysql-test/r/view.result | 15 +++++++++++++++ mysql-test/t/view.test | 16 ++++++++++++++++ sql/item_subselect.cc | 20 ++++++++++++++++++++ sql/item_subselect.h | 1 + 4 files changed, 52 insertions(+) diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index e48a99f6aff..ef4f0a48534 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -6818,5 +6818,20 @@ DROP PROCEDURE sp1; DROP VIEW v1; DROP TABLE t1, t2; # +# MDEV-23291: SUM column from a derived table returns invalid values +# +CREATE TABLE t1(a INT, b INT); +INSERT INTO t1 VALUES (1,1), (2,2); +CREATE view v1 AS +SELECT a as x, (select x) as y, (select y) as z FROM t1; +SELECT sum(z) FROM (SELECT a as x, (select x) as y, (select y) as z FROM t1) q; +sum(z) +3 +SELECT sum(z) FROM v1; +sum(z) +3 +DROP TABLE t1; +DROP VIEW v1; +# # End of 10.2 tests # diff --git a/mysql-test/t/view.test b/mysql-test/t/view.test index b5ce6a1cabf..8cb00f7a6f4 100644 --- a/mysql-test/t/view.test +++ b/mysql-test/t/view.test @@ -6543,6 +6543,22 @@ DROP PROCEDURE sp1; DROP VIEW v1; DROP TABLE t1, t2; +--echo # +--echo # MDEV-23291: SUM column from a derived table returns invalid values +--echo # + +CREATE TABLE t1(a INT, b INT); +INSERT INTO t1 VALUES (1,1), (2,2); + +CREATE view v1 AS +SELECT a as x, (select x) as y, (select y) as z FROM t1; + +SELECT sum(z) FROM (SELECT a as x, (select x) as y, (select y) as z FROM t1) q; +SELECT sum(z) FROM v1; + +DROP TABLE t1; +DROP VIEW v1; + --echo # --echo # End of 10.2 tests --echo # diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index 802bfca64b7..6bd07c7d963 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -436,6 +436,26 @@ bool Item_subselect::mark_as_dependent(THD *thd, st_select_lex *select, } +/* + @brief + Update the table bitmaps for the outer references used within a subquery +*/ + +bool Item_subselect::update_table_bitmaps_processor(void *arg) +{ + List_iterator<Ref_to_outside> it(upper_refs); + Ref_to_outside *upper; + + while ((upper= it++)) + { + if (upper->item && + upper->item->walk(&Item::update_table_bitmaps_processor, FALSE, arg)) + return TRUE; + } + return FALSE; +} + + /* Adjust attributes after our parent select has been merged into grandparent diff --git a/sql/item_subselect.h b/sql/item_subselect.h index 2292c22480f..9116238c640 100644 --- a/sql/item_subselect.h +++ b/sql/item_subselect.h @@ -249,6 +249,7 @@ class Item_subselect :public Item_result_field, @retval FALSE otherwise */ bool is_expensive_processor(void *arg) { return is_expensive(); } + bool update_table_bitmaps_processor(void *arg); /** Get the SELECT_LEX structure associated with this Item.

1 0

[Commits] b588a1e: MDEV-24675 Server crash when table value constructor uses a subselect
by IgorBabaev 26 Jan '21

26 Jan '21

revision-id: b588a1ee72770d3ad8116576bdc24041bccba0df (mariadb-10.3.26-73-gb588a1e) parent(s): 75538f94ca06915ddc22458b82b8e148e51dd0db author: Igor Babaev committer: Igor Babaev timestamp: 2021-01-25 15:21:52 -0800 message: MDEV-24675 Server crash when table value constructor uses a subselect This patch actually fixes the bug MDEV-24675 and the bug MDEV-24618: Assertion failure when TVC uses a row in the context expecting scalar value The cause of these bugs is the same wrong call of the function that fixes value expressions in the value list of a table value constructor. The assertion failure happened when an expression in the value list is of the row type. In this case an error message was expected, but it was not issued because the function fix_fields_if_needed() was called for to check fields of value expressions in a TVC instead of the function fix_fields_if_needed_for_scalar() that would also check that the value expressions are are of a scalar type. The first bug happened when a table value expression used an expression returned by single-row subselect. In this case the call of the fix_fields_if_needed_for_scalar virtual function must be provided with and address to which the single-row subselect has to be attached. Test cases were added for each of the bugs. Approved by Oleksandr Byelkin <sanja(a)mariadb.com> --- mysql-test/main/table_value_constr.result | 54 +++++++++++++++++++++++++++++++ mysql-test/main/table_value_constr.test | 49 ++++++++++++++++++++++++++++ sql/sql_tvc.cc | 4 +-- 3 files changed, 105 insertions(+), 2 deletions(-) diff --git a/mysql-test/main/table_value_constr.result b/mysql-test/main/table_value_constr.result index b95f928..69c75dd 100644 --- a/mysql-test/main/table_value_constr.result +++ b/mysql-test/main/table_value_constr.result @@ -2621,3 +2621,57 @@ EXECUTE IMMEDIATE 'VALUES (?)' USING IGNORE; ERROR HY000: 'ignore' is not allowed in this context EXECUTE IMMEDIATE 'VALUES (?)' USING DEFAULT; ERROR HY000: 'default' is not allowed in this context +# +# MDEV-24675: TVC using subqueries +# +values((select 1)); +(select 1) +1 +values (2), ((select 1)); +2 +2 +1 +values ((select 1)), (2), ((select 3)); +(select 1) +1 +2 +3 +values ((select 1), 2), (3,4), (5, (select 6)); +(select 1) 2 +1 2 +3 4 +5 6 +create table t1 (a int, b int); +insert into t1 values (1,3), (2,3), (3,2), (1,2); +values((select max(a) from t1)); +(select max(a) from t1) +3 +values((select min(b) from t1)); +(select min(b) from t1) +2 +values ((select max(a) from t1), (select min(b) from t1)); +(select max(a) from t1) (select min(b) from t1) +3 2 +values((select * from (select max(b) from t1) as t)); +(select * from (select max(b) from t1) as t) +3 +drop table t1; +# +# MDEV-24618: TVC contains extra parenthesis for row expressions +# in value list +# +create table t1 (a int, b int); +insert into t1 values (1,3), (2,3); +insert into t1 values ((5,4)); +ERROR 21000: Operand should contain 1 column(s) +values ((1,2)); +ERROR 21000: Operand should contain 1 column(s) +select * from (values ((1,2))) dt; +ERROR 21000: Operand should contain 1 column(s) +values (1,2); +1 2 +1 2 +values ((select min(a), max(b) from t1)); +ERROR 21000: Operand should contain 1 column(s) +drop table t1; +End of 10.3 tests diff --git a/mysql-test/main/table_value_constr.test b/mysql-test/main/table_value_constr.test index 11d553f..e8697be 100644 --- a/mysql-test/main/table_value_constr.test +++ b/mysql-test/main/table_value_constr.test @@ -1353,3 +1353,52 @@ VALUES (DEFAULT); EXECUTE IMMEDIATE 'VALUES (?)' USING IGNORE; --error ER_UNKNOWN_ERROR EXECUTE IMMEDIATE 'VALUES (?)' USING DEFAULT; + +--echo # +--echo # MDEV-24675: TVC using subqueries +--echo # + +values((select 1)); + +values (2), ((select 1)); + +values ((select 1)), (2), ((select 3)); + +values ((select 1), 2), (3,4), (5, (select 6)); + +create table t1 (a int, b int); +insert into t1 values (1,3), (2,3), (3,2), (1,2); + +values((select max(a) from t1)); + +values((select min(b) from t1)); + +values ((select max(a) from t1), (select min(b) from t1)); + +values((select * from (select max(b) from t1) as t)); + +drop table t1; + +--echo # +--echo # MDEV-24618: TVC contains extra parenthesis for row expressions +--echo # in value list +--echo # + +create table t1 (a int, b int); +insert into t1 values (1,3), (2,3); +--error ER_OPERAND_COLUMNS +insert into t1 values ((5,4)); + +--error ER_OPERAND_COLUMNS +values ((1,2)); + +--error ER_OPERAND_COLUMNS +select * from (values ((1,2))) dt; + +values (1,2); +--error ER_OPERAND_COLUMNS +values ((select min(a), max(b) from t1)); + +drop table t1; + +--echo End of 10.3 tests diff --git a/sql/sql_tvc.cc b/sql/sql_tvc.cc index 10a279b..1f91539 100644 --- a/sql/sql_tvc.cc +++ b/sql/sql_tvc.cc @@ -47,7 +47,7 @@ bool fix_fields_for_tvc(THD *thd, List_iterator_fast<List_item> &li) while ((lst= li++)) { - List_iterator_fast<Item> it(*lst); + List_iterator<Item> it(*lst); Item *item; while ((item= it++)) @@ -59,7 +59,7 @@ bool fix_fields_for_tvc(THD *thd, List_iterator_fast<List_item> &li) while replacing their values to NAME_CONST()s. So fix only those that have not been. */ - if (item->fix_fields_if_needed(thd, 0) || + if (item->fix_fields_if_needed_for_scalar(thd, it.ref()) || item->check_is_evaluable_expression_or_error()) DBUG_RETURN(true); }

1 0

[Commits] 3e30071e2b5: MDEV-24666, Step #2: fix calculation of derived table cardinality
by Sergei Petrunia 25 Jan '21

25 Jan '21

revision-id: 3e30071e2b5405623a4f24f2910e9efb341c3c0d (mariadb-10.5.4-436-g3e30071e2b5) parent(s): 7c400434aac9e968696e6d6ad1b990d0ef70045e author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-01-25 21:41:48 +0300 message: MDEV-24666, Step #2: fix calculation of derived table cardinality Use JOIN::join_record_count. Do not try to compute a product of POSITION::records_read as that ignores the semi-join duplicate removal, condition selectivities, etc. --- mysql-test/main/opt_tvc.result | 4 ++-- sql/sql_select.cc | 12 +++--------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/mysql-test/main/opt_tvc.result b/mysql-test/main/opt_tvc.result index 9752aa71bfb..190f7d33c95 100644 --- a/mysql-test/main/opt_tvc.result +++ b/mysql-test/main/opt_tvc.result @@ -444,7 +444,7 @@ where b in (3,5) group by b ) as dr_table; id select_type table type possible_keys key key_len ref rows filtered Extra -1 PRIMARY <derived2> ALL NULL NULL NULL NULL 12 100.00 +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 6 100.00 2 DERIVED t1 ALL NULL NULL NULL NULL 6 100.00 Using temporary; Using filesort 2 DERIVED <subquery3> eq_ref distinct_key distinct_key 4 func 1 100.00 3 MATERIALIZED <derived4> ALL NULL NULL NULL NULL 2 100.00 @@ -464,7 +464,7 @@ as tvc_0 group by b ) as dr_table; id select_type table type possible_keys key key_len ref rows filtered Extra -1 PRIMARY <derived2> ALL NULL NULL NULL NULL 12 100.00 +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 6 100.00 2 DERIVED t1 ALL NULL NULL NULL NULL 6 100.00 Using temporary; Using filesort 2 DERIVED <subquery3> eq_ref distinct_key distinct_key 4 func 1 100.00 3 MATERIALIZED <derived4> ALL NULL NULL NULL NULL 2 100.00 diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 5422346884d..7cd399fc76b 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -5541,7 +5541,6 @@ make_join_statistics(JOIN *join, List<TABLE_LIST> &tables_list, DBUG_RETURN(TRUE); /* purecov: inspected */ { - double records= 1; SELECT_LEX_UNIT *unit= join->select_lex->master_unit(); /* Find an optimal join order of the non-constant tables. */ @@ -5568,14 +5567,9 @@ make_join_statistics(JOIN *join, List<TABLE_LIST> &tables_list, if (!(join->select_options & SELECT_DESCRIBE) && unit->derived && unit->derived->is_materialized_derived()) { - /* - Calculate estimated number of rows for materialized derived - table/view. - */ - for (i= 0; i < join->table_count ; i++) - if (double rr= join->best_positions[i].records_read) - records= COST_MULT(records, rr); - ha_rows rows= records > (double) HA_ROWS_MAX ? HA_ROWS_MAX : (ha_rows) records; + ha_rows rows= (join->join_record_count > (double) HA_ROWS_MAX)? + HA_ROWS_MAX : + (ha_rows) join->join_record_count; set_if_smaller(rows, unit->lim.get_select_limit()); join->select_lex->increase_derived_records(rows); }

1 0

[Commits] 7c400434aac: MDEV-24666, Step #1: JOIN_TAB::choose_best_splitting sets records=0
by Sergei Petrunia 25 Jan '21

25 Jan '21

revision-id: 7c400434aac9e968696e6d6ad1b990d0ef70045e (mariadb-10.5.4-435-g7c400434aac) parent(s): 49b87749513fd931eb874d4559b34ed6aeec8f05 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-01-25 15:53:51 +0300 message: MDEV-24666, Step #1: JOIN_TAB::choose_best_splitting sets records=0 If we get the estimated #records to be less than 1.0, don't set JOIN_TAB::records=0. Round it up to 1, instead. --- sql/opt_split.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sql/opt_split.cc b/sql/opt_split.cc index 6cb4a12e51f..12715ffa886 100644 --- a/sql/opt_split.cc +++ b/sql/opt_split.cc @@ -1015,6 +1015,12 @@ SplM_plan_info * JOIN_TAB::choose_best_splitting(double record_count, } else startup_cost= spl_opt_info->unsplit_cost; + + // JOIN_TAB::records is integer. Make sure we did not set it to 0 + // when converting from unsplit_card. + if (!records) + records= 1; + return spl_plan; }

1 0