MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Evgeny Potemkin Date:October 29 2009 3:16pm
Subject:Re: bzr commit into mysql-5.1 branch (jorgen.loland:3180) Bug#48177
View as plain text  
Hello Jorgen,

In general the fix is ok. There is few minor comments (see inline).
It's ok to push after those being addressed.

Regards, Evgen.

Jorgen Loland wrote:
> #At file:///localhome/jl208045/mysql/mysql-5.1-branches/mysql-5.1/ based on
> revid:bjorn.munch@stripped
> 
>  3180 Jorgen Loland	2009-10-26
>       Bug#48177 - SELECTs with NOT IN subqueries containing NULL 
>                   values return too many records
[skip]
> +
> +# Handler_read_rnd_next should be 18 
> +SHOW STATUS LIKE '%Handler_read_rnd_next';
> +Variable_name	Value
> +Handler_read_rnd_next	18
It's not clear why here should be exactly 18 reads. If number of reads changes 
by some reason it would be hard to find the reason.
Thus I suggest you to show number of reads with only one (null,null) row,
then insert copule more, flush status and show number of reads again.
Or you can use user-variable to produce a side-effect for the subquery and
show that it isn't evaluated for second (null,null) row.
I think the last one will be even better.

> +DROP TABLE t1,t2;
> +End of 5.1 tests
> 
> === modified file 'mysql-test/t/subselect3.test'
> --- a/mysql-test/t/subselect3.test	2009-08-31 14:09:09 +0000
> +++ b/mysql-test/t/subselect3.test	2009-10-26 11:49:16 +0000
> @@ -728,3 +728,55 @@ where
>                                    from t4, t5 limit 2));
[skip]
>        /*
> -        We're evaluating "NULL IN (SELECT ...)". The result can be NULL or
> -        FALSE, and we can return one instead of another. Just return NULL.
> +        The case where a NULL value in the outer_value_list means that
> +        the result shall be NULL/FALSE (makes no difference). The
> +        cached value is NULL, so just return NULL.
After reading this comment one may think that there is no difference between 
FALSE/NULLbeing returned at all. But this is true only when 
subquery->top_level_item() is true also. Please, re-phrase it.

>        */
>        null_value= 1;
>      }
>      else
>      {
[skip]
> +      if (!((Item_in_subselect*)args[1])->is_correlated && 
> +          all_left_cols_null && result_for_null_param != UNKNOWN)
You can remove either this or the check below because
result_for_null_param depends on the all_left_cols_null variable.
> +      {
> +        /* 
> +           This is a non-correlated subquery, all values in the outer
> +           value list are NULL, and we have already evaluated the
> +           subquery for all NULL values: Return the same result we
> +           did last time without evaluating the subquery.
> +        */
>          null_value= result_for_null_param;
> -      }
> -      else
> +      } 
> +      else 
[skip]
> -          (void) args[1]->val_bool_result();
> -          result_for_null_param= null_value= !item_subs->engine->no_rows();
> -          
> -          /* Turn all predicates back on */
> -          for (i= 0; i < ncols; i++)
> -            item_subs->set_cond_guard_var(i, TRUE);
> -        }
> +        /* The subquery has to be evaluated */
> +        (void) args[1]->val_bool_result();
> +        null_value= !item_subs->engine->no_rows();
> +        if (all_left_cols_null)
This check      ^^^^
> +          result_for_null_param= null_value;
>        }
> +
> +      /* Turn all predicates back on */
> +      for (i= 0; i < ncols; i++)
> +        item_subs->set_cond_guard_var(i, TRUE);
>      }
>      return 0;
>    }
Thread
bzr commit into mysql-5.1 branch (jorgen.loland:3180) Bug#48177Jorgen Loland26 Oct
  • Re: bzr commit into mysql-5.1 branch (jorgen.loland:3180) Bug#48177Evgeny Potemkin29 Oct
    • Re: bzr commit into mysql-5.1 branch (jorgen.loland:3180) Bug#48177Øystein Grøvlen30 Oct
      • Re: bzr commit into mysql-5.1 branch (jorgen.loland:3180) Bug#48177Evgeny Potemkin30 Oct
    • Re: bzr commit into mysql-5.1 branch (jorgen.loland:3180) Bug#48177Jørgen Løland3 Nov
  • Re: bzr commit into mysql-5.1 branch (jorgen.loland:3180) Bug#48177Øystein Grøvlen30 Oct
    • Re: bzr commit into mysql-5.1 branch (jorgen.loland:3180) Bug#48177Jørgen Løland3 Nov