maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #05144
Re: Fwd: [Commits] Rev 3622: Fix for bug MDEV-765 (LP:825075) in file:///home/tsk/mprog/src/5.3-md765/
Hi Timour,
On Mon, Feb 04, 2013 at 05:39:02PM +0200, Timour Katchaounov wrote:
>
> Sergey,
>
> Could you please review the following patch. The fix implements
> your suggestion from your previous review:
> https://lists.launchpad.net/maria-developers/msg04597.html
>
Ok to push.
> Timour
>
> ------------------------------------------------------------
> revno: 3622
> revision-id: timour@xxxxxxxxxxxx-20130204153548-njv08hcdskv6ttjk
> parent: sergii@xxxxxxxxx-20130128081223-mp9rsd3t9soz8lly
> fixes bug: https://mariadb.atlassian.net/browse/MDEV-765
> committer: timour@xxxxxxxxxxxx
> branch nick: 5.3-md765
> timestamp: Mon 2013-02-04 17:35:48 +0200
> message:
> Fix for bug MDEV-765 (LP:825075)
>
> Analys:
> The cause for the wrong result was that the optimizer
> incorrectly chose min/max loose scan when it is not
> applicable. The applicability test missed the case when
> a condition on the MIN/MAX argument was OR-ed with a
> condition on some other field. In this case, the MIN/MAX
> condition cannot be used for loose scan.
>
> Solution:
> Extend the test check_group_min_max_predicates() to check
> that the WHERE clause is of the form: "cond1 AND cond2"
> where
> cond1 - does not use min_max_column at all.
> cond2 - is an AND/OR tree with leaves in form "min_max_column $CMP$ const"
> or $CMP$ is one of the functions between, is [not] null
>
>
> === modified file 'mysql-test/r/group_min_max.result'
> --- a/mysql-test/r/group_min_max.result 2013-01-28 08:12:23 +0000
> +++ b/mysql-test/r/group_min_max.result 2013-02-04 15:35:48 +0000
> @@ -3186,3 +3186,61 @@ a b
> 109 19
> drop table t1;
> End of 5.1 tests
> +#
> +# MDEV-765: LP:825075 - Wrong result with GROUP BY + multipart key + MIN/MAX loose scan
> +#
> +CREATE TABLE t1 (a varchar(1), b varchar(1), KEY (b,a));
> +INSERT INTO t1 VALUES
> +('0',NULL),('9',NULL),('8','c'),('4','d'),('7','d'),(NULL,'f'),
> +('7','f'),('8','g'),(NULL,'j');
> +explain
> +SELECT max(a) , b FROM t1 WHERE a IS NULL OR b = 'z' GROUP BY b;
> +id select_type table type possible_keys key key_len ref rows Extra
> +1 SIMPLE t1 index b b 8 NULL 9 Using where; Using index
> +SELECT max(a) , b FROM t1 WHERE a IS NULL OR b = 'z' GROUP BY b;
> +max(a) b
> +NULL f
> +NULL j
> +explain
> +SELECT b, min(a) FROM t1 WHERE a = '7' OR b = 'z' GROUP BY b;
> +id select_type table type possible_keys key key_len ref rows Extra
> +1 SIMPLE t1 index b b 8 NULL 9 Using where; Using index
> +SELECT b, min(a) FROM t1 WHERE a = '7' OR b = 'z' GROUP BY b;
> +b min(a)
> +d 7
> +f 7
> +explain
> +SELECT b, min(a) FROM t1 WHERE (a = b OR b = 'd' OR b is NULL) GROUP BY b;
> +id select_type table type possible_keys key key_len ref rows Extra
> +1 SIMPLE t1 index b b 8 NULL 9 Using where; Using index
> +SELECT b, min(a) FROM t1 WHERE (a = b OR b = 'd' OR b is NULL) GROUP BY b;
> +b min(a)
> +NULL 0
> +d 4
> +explain
> +SELECT b, min(a) FROM t1 WHERE a > ('0' = b) AND b = 'z' GROUP BY b;
> +id select_type table type possible_keys key key_len ref rows Extra
> +1 SIMPLE t1 ref b b 4 const 1 Using where; Using index
> +SELECT b, min(a) FROM t1 WHERE a > ('0' = b) AND b = 'z' GROUP BY b;
> +b min(a)
> +explain
> +SELECT b, min(a) FROM t1 WHERE a > '0' AND (b < (a = '7')) GROUP BY b;
> +id select_type table type possible_keys key key_len ref rows Extra
> +1 SIMPLE t1 index NULL b 8 NULL 9 Using where; Using index
> +SELECT b, min(a) FROM t1 WHERE a > '0' AND (b < (a = '7')) GROUP BY b;
> +b min(a)
> +d 7
> +f 7
> +explain
> +SELECT b, min(a) FROM t1 WHERE (a > '0' AND (a > '1' OR b = 'd')) GROUP BY b;
> +id select_type table type possible_keys key key_len ref rows Extra
> +1 SIMPLE t1 index b b 8 NULL 9 Using where; Using index
> +SELECT b, min(a) FROM t1 WHERE (a > '0' AND (a > '1' OR b = 'd')) GROUP BY b;
> +b min(a)
> +NULL 9
> +c 8
> +d 4
> +f 7
> +g 8
> +drop table t1;
> +End of 5.3 tests
>
> === modified file 'mysql-test/t/group_min_max.test'
> --- a/mysql-test/t/group_min_max.test 2013-01-28 08:12:23 +0000
> +++ b/mysql-test/t/group_min_max.test 2013-02-04 15:35:48 +0000
> @@ -1203,3 +1203,40 @@ WHERE EXISTS ( SELECT DISTINCT b FROM t1
> drop table t1;
>
> --echo End of 5.1 tests
> +
> +--echo #
> +--echo # MDEV-765: LP:825075 - Wrong result with GROUP BY + multipart key + MIN/MAX loose scan
> +--echo #
> +
> +CREATE TABLE t1 (a varchar(1), b varchar(1), KEY (b,a));
> +INSERT INTO t1 VALUES
> +('0',NULL),('9',NULL),('8','c'),('4','d'),('7','d'),(NULL,'f'),
> +('7','f'),('8','g'),(NULL,'j');
> +
> +explain
> +SELECT max(a) , b FROM t1 WHERE a IS NULL OR b = 'z' GROUP BY b;
> +SELECT max(a) , b FROM t1 WHERE a IS NULL OR b = 'z' GROUP BY b;
> +
> +explain
> +SELECT b, min(a) FROM t1 WHERE a = '7' OR b = 'z' GROUP BY b;
> +SELECT b, min(a) FROM t1 WHERE a = '7' OR b = 'z' GROUP BY b;
> +
> +explain
> +SELECT b, min(a) FROM t1 WHERE (a = b OR b = 'd' OR b is NULL) GROUP BY b;
> +SELECT b, min(a) FROM t1 WHERE (a = b OR b = 'd' OR b is NULL) GROUP BY b;
> +
> +explain
> +SELECT b, min(a) FROM t1 WHERE a > ('0' = b) AND b = 'z' GROUP BY b;
> +SELECT b, min(a) FROM t1 WHERE a > ('0' = b) AND b = 'z' GROUP BY b;
> +
> +explain
> +SELECT b, min(a) FROM t1 WHERE a > '0' AND (b < (a = '7')) GROUP BY b;
> +SELECT b, min(a) FROM t1 WHERE a > '0' AND (b < (a = '7')) GROUP BY b;
> +
> +explain
> +SELECT b, min(a) FROM t1 WHERE (a > '0' AND (a > '1' OR b = 'd')) GROUP BY b;
> +SELECT b, min(a) FROM t1 WHERE (a > '0' AND (a > '1' OR b = 'd')) GROUP BY b;
> +
> +drop table t1;
> +
> +--echo End of 5.3 tests
>
> === modified file 'sql/opt_range.cc'
> --- a/sql/opt_range.cc 2013-01-28 08:12:23 +0000
> +++ b/sql/opt_range.cc 2013-02-04 15:35:48 +0000
> @@ -11485,7 +11485,8 @@ static bool get_constant_key_infix(KEY *
> KEY_PART_INFO **first_non_infix_part);
> static bool
> check_group_min_max_predicates(Item *cond, Item_field *min_max_arg_item,
> - Field::imagetype image_type);
> + Field::imagetype image_type,
> + bool *has_min_max_fld, bool *has_other_fld);
>
> static void
> cost_group_min_max(TABLE* table, KEY *index_info, uint used_key_parts,
> @@ -12005,10 +12006,12 @@ get_best_group_min_max(PARAM *param, SEL
> DBUG_RETURN(NULL);
>
> /* Check (SA3) for the where clause. */
> + bool has_min_max_fld= false, has_other_fld= false;
> if (join->conds && min_max_arg_item &&
> !check_group_min_max_predicates(join->conds, min_max_arg_item,
> (index_info->flags & HA_SPATIAL) ?
> - Field::itMBR : Field::itRAW))
> + Field::itMBR : Field::itRAW,
> + &has_min_max_fld, &has_other_fld))
> DBUG_RETURN(NULL);
>
> /* The query passes all tests, so construct a new TRP object. */
> @@ -12043,16 +12046,24 @@ get_best_group_min_max(PARAM *param, SEL
>
> SYNOPSIS
> check_group_min_max_predicates()
> - cond tree (or subtree) describing all or part of the WHERE
> - clause being analyzed
> - min_max_arg_item the field referenced by the MIN/MAX function(s)
> - min_max_arg_part the keypart of the MIN/MAX argument if any
> + cond [in] the expression tree being analyzed
> + min_max_arg [in] the field referenced by the MIN/MAX function(s)
> + image_type [in]
> + has_min_max_arg [out] true if the subtree being analyzed references min_max_arg
> + has_other_arg [out] true if the subtree being analyzed references a column
> + other min_max_arg
>
> DESCRIPTION
> The function walks recursively over the cond tree representing a WHERE
> clause, and checks condition (SA3) - if a field is referenced by a MIN/MAX
> aggregate function, it is referenced only by one of the following
> - predicates: {=, !=, <, <=, >, >=, between, is null, is not null}.
> + predicates $FUNC$:
> + {=, !=, <, <=, >, >=, between, is [not] null, multiple equal}.
> + In addition the function checks that the WHERE condition is equivalent to
> + "cond1 AND cond2" where :
> + cond1 - does not use min_max_column at all.
> + cond2 - is an AND/OR tree with leaves in form
> + "$FUNC$(min_max_column[, const])".
>
> RETURN
> TRUE if cond passes the test
> @@ -12061,7 +12072,8 @@ get_best_group_min_max(PARAM *param, SEL
>
> static bool
> check_group_min_max_predicates(Item *cond, Item_field *min_max_arg_item,
> - Field::imagetype image_type)
> + Field::imagetype image_type,
> + bool *has_min_max_arg, bool *has_other_arg)
> {
> DBUG_ENTER("check_group_min_max_predicates");
> DBUG_ASSERT(cond && min_max_arg_item);
> @@ -12073,12 +12085,24 @@ check_group_min_max_predicates(Item *con
> DBUG_PRINT("info", ("Analyzing: %s", ((Item_func*) cond)->func_name()));
> List_iterator_fast<Item> li(*((Item_cond*) cond)->argument_list());
> Item *and_or_arg;
> + Item_func::Functype func_type= ((Item_cond*) cond)->functype();
> + bool has_min_max= false, has_other= false;
> while ((and_or_arg= li++))
> {
> - if (!check_group_min_max_predicates(and_or_arg, min_max_arg_item,
> - image_type))
> + /*
> + The WHERE clause doesn't pass the condition if:
> + (1) any subtree doesn't pass the condition or
> + (2) the subtree passes the test, but it is an OR and it references both
> + the min/max argument and other columns.
> + */
> + if (!check_group_min_max_predicates(and_or_arg, min_max_arg_item, //1
> + image_type,
> + &has_min_max, &has_other) ||
> + (func_type == Item_func::COND_OR_FUNC && has_min_max && has_other))//2
> DBUG_RETURN(FALSE);
> }
> + *has_min_max_arg= has_min_max || *has_min_max_arg;
> + *has_other_arg= has_other || *has_other_arg;
> DBUG_RETURN(TRUE);
> }
>
> @@ -12112,6 +12136,10 @@ check_group_min_max_predicates(Item *con
> if (cond_type == Item::FIELD_ITEM)
> {
> DBUG_PRINT("info", ("Analyzing: %s", cond->full_name()));
> + if (min_max_arg_item->eq((Item_field*)cond, 1))
> + *has_min_max_arg= true;
> + else
> + *has_other_arg= true;
> DBUG_RETURN(TRUE);
> }
>
> @@ -12120,9 +12148,33 @@ check_group_min_max_predicates(Item *con
>
> /* Test if cond references only group-by or non-group fields. */
> Item_func *pred= (Item_func*) cond;
> + Item_func::Functype pred_type= pred->functype();
> + DBUG_PRINT("info", ("Analyzing: %s", pred->func_name()));
> + if (pred_type == Item_func::MULT_EQUAL_FUNC)
> + {
> + /*
> + Check that each field in a multiple equality is either a constant or
> + it is a reference to the min/max argument, or it doesn't contain the
> + min/max argument at all.
> + */
> + Item_equal_fields_iterator eq_it(*((Item_equal*)pred));
> + Item *eq_item;
> + bool has_min_max= false, has_other= false;
> + while ((eq_item= eq_it++))
> + {
> + if (min_max_arg_item->eq(eq_item->real_item(), 1))
> + has_min_max= true;
> + else
> + has_other= true;
> + }
> + *has_min_max_arg= has_min_max || *has_min_max_arg;
> + *has_other_arg= has_other || *has_other_arg;
> + DBUG_RETURN(!(has_min_max && has_other));
> + }
> +
> Item **arguments= pred->arguments();
> Item *cur_arg;
> - DBUG_PRINT("info", ("Analyzing: %s", pred->func_name()));
> + bool has_min_max= false, has_other= false;
> for (uint arg_idx= 0; arg_idx < pred->argument_count (); arg_idx++)
> {
> cur_arg= arguments[arg_idx]->real_item();
> @@ -12131,11 +12183,11 @@ check_group_min_max_predicates(Item *con
> {
> if (min_max_arg_item->eq(cur_arg, 1))
> {
> - /*
> - If pred references the MIN/MAX argument, check whether pred is a range
> - condition that compares the MIN/MAX argument with a constant.
> - */
> - Item_func::Functype pred_type= pred->functype();
> + has_min_max= true;
> + /*
> + If pred references the MIN/MAX argument, check whether pred is a range
> + condition that compares the MIN/MAX argument with a constant.
> + */
> if (pred_type != Item_func::EQUAL_FUNC &&
> pred_type != Item_func::LT_FUNC &&
> pred_type != Item_func::LE_FUNC &&
> @@ -12175,11 +12227,13 @@ check_group_min_max_predicates(Item *con
> min_max_arg_item->field->cmp_type() != args[1]->result_type())))
> DBUG_RETURN(FALSE);
> }
> + else
> + has_other= true;
> }
> else if (cur_arg->type() == Item::FUNC_ITEM)
> {
> - if (!check_group_min_max_predicates(cur_arg, min_max_arg_item,
> - image_type))
> + if (!check_group_min_max_predicates(cur_arg, min_max_arg_item, image_type,
> + &has_min_max, &has_other))
> DBUG_RETURN(FALSE);
> }
> else if (cur_arg->const_item())
> @@ -12192,7 +12246,11 @@ check_group_min_max_predicates(Item *con
> }
> else
> DBUG_RETURN(FALSE);
> + if(has_min_max && has_other)
> + DBUG_RETURN(FALSE);
> }
> + *has_min_max_arg= has_min_max || *has_min_max_arg;
> + *has_other_arg= has_other || *has_other_arg;
>
> DBUG_RETURN(TRUE);
> }
>
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
--
BR
Sergei
--
Sergei Petrunia, Software Developer
Monty Program AB, http://askmonty.org
Blog: http://s.petrunia.net/blog
