List:Commits« Previous MessageNext Message »
From:Tor Didriksen Date:May 18 2011 9:12am
Subject:Re: bzr commit into mysql-trunk branch (jorgen.loland:3307) Bug#12430646
View as plain text  
On Mon, May 16, 2011 at 10:01 AM, Jorgen Loland <jorgen.loland@stripped>wrote:

> #At
>
> file:///export/home/jl208045/mysql/wl4800/mysql-next-mr-opt-backporting-wl4800-12430646/
> based on revid:guilhem.bichot@stripped
>
>  3307 Jorgen Loland     2011-05-16
>      BUG#12430646 - SEL_ARG::LEFT AND RIGHT POINTERS INCORRECTLY USED.
>                     CRASHES OPTIMIZER TRACING
>
>      SEL_ARGs are the building blocks of range conditions. Multiple
>      conditions are stored as SEL_ARGs in a RedBlack tree, in which
>      left and righ points to children SEL_ARGs.
>
>      When a SEL_ARG does not have a left or right child, left/right
>      normally points to null_element. This is, however, not the case
>      with a special SEL_ARG of type MAYBE_KEY. For this type, the
>      left and right pointers are required to be NULL pointers. This
>      is inconsistent, but many functions rely on this fact
>      (e.g. eq_tree(), SEL_ARG::first()).
>
>      Two bugs are fixed by this changeset:
>       1) One of the SEL_ARG constructors did not initialize
>          prev, next, left and right pointers.
>       2) and_all_keys() sets left and right pointers to
>          &null_element for SEL_ARG::MAYBE_KEY even though it is
>          explicitly stated that these shall be NULL.
>     @ mysql-test/r/optimizer_trace_bugs.result
>        New regression test result file for statements that failed with
>        optimizer tracing enabled.
>        Added test for BUG#12430646
>     @ mysql-test/t/optimizer_trace_bugs.test
>        New regression test file for statements that failed with
>        optimizer tracing enabled.
>        Added test for BUG#12430646
>     @ sql/opt_range.cc
>        * One of the SEL_ARG constructors did not initialize prev,
>          next, left and right pointers.
>        * and_all_keys() sets left and right pointers to &null_element
>          for SEL_ARG::MAYBE_KEY even though it is explicitly stated
>          that these shall be NULL.
>
>    added:
>      mysql-test/r/optimizer_trace_bugs.result
>      mysql-test/t/optimizer_trace_bugs.test
>    modified:
>      sql/opt_range.cc
> === added file 'mysql-test/r/optimizer_trace_bugs.result'
> --- a/mysql-test/r/optimizer_trace_bugs.result  1970-01-01 00:00:00 +0000
> +++ b/mysql-test/r/optimizer_trace_bugs.result  2011-05-16 08:00:59 +0000
> @@ -0,0 +1,33 @@
> +SET optimizer_trace="enabled=on,end_marker=on,one_line=off";
> +#
> +# BUG#12430646 - SEL_ARG::LEFT AND RIGHT POINTERS INCORRECTLY
> +#                USED. CRASHES OPTIMIZER TRACING
> +#
> +CREATE TABLE t1 (
> +a INT,
> +b CHAR(2),
> +c INT,
> +d INT,
> +KEY ( c ),
> +KEY ( d, a, b ( 2 ) ),
> +KEY ( b ( 1 ) )
> +);
> +INSERT INTO t1 VALUES ( NULL, 'a', 1, 2 ), ( NULL, 'a', 1, 2 ),
> +( 1,    'a', 1, 2 ), ( 1,    'a', 1, 2 );
> +CREATE TABLE t2 (
> +a INT,
> +c INT,
> +e INT,
> +KEY ( e )
> +);
> +INSERT INTO t2 VALUES ( 1, 1, NULL ), ( 1, 1, NULL );
> +SELECT 1
> +FROM t1, t2
> +WHERE t1.d <> '1' AND t1.b > '1'
> +AND t1.a = t2.a AND t1.c = t2.c;
> +1
> +1
> +1
> +1
> +1
> +DROP TABLE t1, t2;
>
> === added file 'mysql-test/t/optimizer_trace_bugs.test'
> --- a/mysql-test/t/optimizer_trace_bugs.test    1970-01-01 00:00:00 +0000
> +++ b/mysql-test/t/optimizer_trace_bugs.test    2011-05-16 08:00:59 +0000
> @@ -0,0 +1,38 @@
> +# Regressiontest for statements that failed with optimizer tracing
> enabled.
> +
> +--source include/have_optimizer_trace.inc
> +SET optimizer_trace="enabled=on,end_marker=on,one_line=off";
> +
> +--echo #
> +--echo # BUG#12430646 - SEL_ARG::LEFT AND RIGHT POINTERS INCORRECTLY
> +--echo #                USED. CRASHES OPTIMIZER TRACING
> +--echo #
> +
> +CREATE TABLE t1 (
> +  a INT,
> +  b CHAR(2),
> +  c INT,
> +  d INT,
> +  KEY ( c ),
> +  KEY ( d, a, b ( 2 ) ),
> +  KEY ( b ( 1 ) )
> +);
> +
> +INSERT INTO t1 VALUES ( NULL, 'a', 1, 2 ), ( NULL, 'a', 1, 2 ),
> +                      ( 1,    'a', 1, 2 ), ( 1,    'a', 1, 2 );
> +
> +CREATE TABLE t2 (
> +  a INT,
> +  c INT,
> +  e INT,
> +  KEY ( e )
> +);
> +
> +INSERT INTO t2 VALUES ( 1, 1, NULL ), ( 1, 1, NULL );
> +
> +SELECT 1
> +FROM t1, t2
> +WHERE t1.d <> '1' AND t1.b > '1'
> +AND t1.a = t2.a AND t1.c = t2.c;
> +
> +DROP TABLE t1, t2;
>
> === modified file 'sql/opt_range.cc'
> --- a/sql/opt_range.cc  2011-05-13 14:17:54 +0000
> +++ b/sql/opt_range.cc  2011-05-16 08:00:59 +0000
> @@ -376,7 +376,9 @@ public:
>   uchar *min_value,*max_value;                 // Pointer to range
>
>   /*
> -    eq_tree() requires that left == right == 0 if the type is MAYBE_KEY.
> +    eq_tree(), first(), last() etc requires that left == right == 0 if the
> type
>

requires => require


> +    is MAYBE_KEY. Todo: fix this so SEL_ARGs without R-B children are
> handled
> +    consistently. See related WL#5894
>    */
>   SEL_ARG *left,*right;   /* R-B tree children */
>   SEL_ARG *next,*prev;    /* Links for bi-directional interval list */
> @@ -396,8 +398,12 @@ public:
>   SEL_ARG(Field *,const uchar *, const uchar *);
>   SEL_ARG(Field *field, uint8 part, uchar *min_value, uchar *max_value,
>          uint8 min_flag, uint8 max_flag, uint8 maybe_flag);
> +  /*
> +    Used to consturct MAYBE_KEY SEL_ARGs, so left and right must be
> +    NULL. See todo for left/right pointers.
> +  */
>   SEL_ARG(enum Type type_arg)
> -
>  :min_flag(0),elements(1),use_count(1),left(0),right(0),next_key_part(0),
> +
>  :min_flag(0),elements(1),use_count(1),left(NULL),right(NULL),next_key_part(0),
>

long line

to verify the comment, I did this:
  {
    DBUG_ASSERT(type_arg == MAYBE_KEY);
  }

Which makes the server fail during startup.
This CTOR is also used for:

static SEL_ARG null_element(SEL_ARG::IMPOSSIBLE);

static objects are bad ....

This assert however, passes all tests:

    DBUG_ASSERT(type_arg == MAYBE_KEY || type_arg == IMPOSSIBLE);



>     color(BLACK), type(type_arg)
>   {}
>   inline bool is_same(SEL_ARG *arg)
> @@ -1797,6 +1803,9 @@ QUICK_RANGE::QUICK_RANGE()
>
>  SEL_ARG::SEL_ARG(SEL_ARG &arg) :Sql_alloc()
>  {
> +  DBUG_ASSERT(arg.type != SEL_ARG::MAYBE_KEY);  // Would need
> left=right=NULL
>

  DBUG_ASSERT(arg.type != MAYBE_KEY);  // Would need left=right=NULL



> +  left=right= &null_element;
> +  prev=next= NULL;
>   type=arg.type;
>   min_flag=arg.min_flag;
>   max_flag=arg.max_flag;
> @@ -1815,7 +1824,7 @@ inline void SEL_ARG::make_root()
>  {
>   left=right= &null_element;
>   color=BLACK;
> -  next=prev=0;
> +  next=prev= NULL;
>   use_count=0; elements=1;
>  }
>
> @@ -1823,7 +1832,7 @@ SEL_ARG::SEL_ARG(Field *f,const uchar *m
>                  const uchar *max_value_arg)
>   :min_flag(0), max_flag(0), maybe_flag(0),
> maybe_null(f->real_maybe_null()),
>    elements(1), use_count(1), field(f), min_value((uchar*) min_value_arg),
> -   max_value((uchar*) max_value_arg), next(0),prev(0),
> +   max_value((uchar*) max_value_arg), next(NULL),prev(NULL),
>    next_key_part(0),color(BLACK),type(KEY_RANGE)
>  {
>   left=right= &null_element;
> @@ -1835,7 +1844,7 @@ SEL_ARG::SEL_ARG(Field *field_,uint8 par
>   :min_flag(min_flag_),max_flag(max_flag_),maybe_flag(maybe_flag_),
>    part(part_),maybe_null(field_->real_maybe_null()),
> elements(1),use_count(1),
>    field(field_), min_value(min_value_), max_value(max_value_),
> -   next(0),prev(0),next_key_part(0),color(BLACK),type(KEY_RANGE)
> +   next(NULL),prev(NULL),next_key_part(0),color(BLACK),type(KEY_RANGE)
>  {
>   left=right= &null_element;
>  }
> @@ -6950,7 +6959,9 @@ and_all_keys(RANGE_OPT_PARAM *param, SEL
>   }
>   if (key1->type == SEL_ARG::MAYBE_KEY)
>   {
> -    key1->right= key1->left= &null_element;
> +    // See todo for left/right pointers
> +    DBUG_ASSERT(!key1->left);
> +    DBUG_ASSERT(!key1->right);
>     key1->next= key1->prev= 0;
>   }
>   for (next=key1->first(); next ; next=next->next)
>
>
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:
> http://lists.mysql.com/commits?unsub=1
>

Thread
bzr commit into mysql-trunk branch (jorgen.loland:3307) Bug#12430646Jorgen Loland16 May
  • Re: bzr commit into mysql-trunk branch (jorgen.loland:3307) Bug#12430646Olav Sandstaa16 May
  • Re: bzr commit into mysql-trunk branch (jorgen.loland:3307) Bug#12430646Tor Didriksen19 May