maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Varun,

Things that look apparently wrong:

1. The patch checks left_expr->null_value without first having made a call 
that will fill that member.
The calling convention is something like this:

  item->val_XXX()  // val_str or val_int()
  // check item->null_value.

If one doesn't need string or integer value, there is item->is_null() call.

2. The patch checks null_value for any kind of left_expr. This is wrong. For 
example, consider a query:

  select
    t1.col IN (select t2.col FROM t2 )
  from t1

Here, it doesn't make any sense to check {t1.col}->null_value at query
planning/rewrite time.  The value (and NULL-ness) of t1.col is different for 
different rows of t1.

As discussed on Slack: please only check for NULL value for items which are 
- constant,
- and "not expensive"
and so have a fixed value which we are allowed to compute at query optimization
phase.

3. I see the patch handles single-value comparisons like

    expr IN (select inner_expr FROM ...)

but what about tuple-based comparisons like:

    (expr1, expr2) IN (select inner_expr1, inner_expr2 FROM ...)

?

I guess they should have a similar issue. If they do not, we need
- an explanation why
- test coverage

On Sun, Feb 26, 2017 at 09:04:40PM +0530, Varun wrote:
> revision-id: c6d172f785f31641832087a432d1966b67efc47a (mariadb-10.2.3-283-gc6d172f)
> parent(s): 78153cf641aea41166d3e79ae99b57b154f6a027
> author: Varun Gupta
> committer: Varun Gupta
> timestamp: 2017-02-26 21:03:29 +0530
> message:
> 
> MDEV-11078: NULL NOT IN (non-empty subquery) should never return results
> 
> Setting the value of cond guard variables during the creation of Tricond Item for null values
> 
> ---
>  mysql-test/r/subselect4.result | 31 +++++++++++++++++++++++++++++++
>  mysql-test/t/subselect4.test   | 18 ++++++++++++++++++
>  sql/item_subselect.cc          | 10 ++++++++++
>  3 files changed, 59 insertions(+)
> 
> diff --git a/mysql-test/r/subselect4.result b/mysql-test/r/subselect4.result
> index 6bbb80c..c3bd50a 100644
> --- a/mysql-test/r/subselect4.result
> +++ b/mysql-test/r/subselect4.result
> @@ -2454,3 +2454,34 @@ x
>  c1
>  1
>  drop table t1;
> +#
> +# MDEV-11078: NULL NOT IN (non-empty subquery) should never return results
> +#
> +create table t1(a int,b int);
> +create table t2(a int,b int);
> +insert into t1 value (NULL,2);
> +select 1 in (1,2, NULL);
> +1 in (1,2, NULL)
> +1
> +select (null)  in (select 1 from t1);
> +(null)  in (select 1 from t1)
> +NULL
> +select (null)  in (select 1 from t2);
> +(null)  in (select 1 from t2)
> +0
> +select 1 in (select 1 from t1);
> +1 in (select 1 from t1)
> +1
> +select 1 in (select 1 from t2);
> +1 in (select 1 from t2)
> +0
> +select 1 from dual where null in (select 1 from t1);
> +1
> +select 1 from dual where null in (select 1 from t2);
> +1
> +select 1 from dual where null not in (select 1 from t1);
> +1
> +select 1 from dual where null not in (select 1 from t2);
> +1
> +1
> +drop table t1,t2;
> diff --git a/mysql-test/t/subselect4.test b/mysql-test/t/subselect4.test
> index 253160c..785b79d 100644
> --- a/mysql-test/t/subselect4.test
> +++ b/mysql-test/t/subselect4.test
> @@ -2009,3 +2009,21 @@ insert into t1 values(2,1),(1,2);
>  select (select c1 from t1 group by c1,c2 order by c1 limit 1) as x;
>  (select c1 from t1 group by c1,c2 order by c1 limit 1);
>  drop table t1;
> +
> +--echo #
> +--echo # MDEV-11078: NULL NOT IN (non-empty subquery) should never return results
> +--echo #
> +
> +create table t1(a int,b int);
> +create table t2(a int,b int);
> +insert into t1 value (NULL,2);
> +select 1 in (1,2, NULL);
> +select (null)  in (select 1 from t1);
> +select (null)  in (select 1 from t2);
> +select 1 in (select 1 from t1);
> +select 1 in (select 1 from t2);
> +select 1 from dual where null in (select 1 from t1);
> +select 1 from dual where null in (select 1 from t2);
> +select 1 from dual where null not in (select 1 from t1);
> +select 1 from dual where null not in (select 1 from t2);
> +drop table t1,t2;
> diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc
> index 94bc71c..b9c798e 100644
> --- a/sql/item_subselect.cc
> +++ b/sql/item_subselect.cc
> @@ -2113,6 +2113,8 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join,
>          We can encounter "NULL IN (SELECT ...)". Wrap the added condition
>          within a trig_cond.
>        */
> +      if(left_expr->null_value)
> +        set_cond_guard_var(0,FALSE);
>        item= new (thd->mem_root) Item_func_trig_cond(thd, item, get_cond_guard(0));
>      }
>  
> @@ -2137,6 +2139,9 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join,
>  	having= new (thd->mem_root) Item_is_not_null_test(thd, this, having);
>          if (left_expr->maybe_null)
>          {
> +          if(left_expr->null_value)
> +            set_cond_guard_var(0,FALSE);
> +
>            if (!(having= new (thd->mem_root) Item_func_trig_cond(thd, having,
>                                                              get_cond_guard(0))))
>              DBUG_RETURN(true);
> @@ -2155,6 +2160,9 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join,
>        */
>        if (!abort_on_null && left_expr->maybe_null)
>        {
> +        if(left_expr->null_value)
> +            set_cond_guard_var(0, FALSE);
> +
>          if (!(item= new (thd->mem_root) Item_func_trig_cond(thd, item,
>                                                              get_cond_guard(0))))
>            DBUG_RETURN(true);
> @@ -2184,6 +2192,8 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join,
>                                                    (char *)"<result>"));
>          if (!abort_on_null && left_expr->maybe_null)
>          {
> +          if(left_expr->null_value)
> +            set_cond_guard_var(0,FALSE);
>            if (!(new_having= new (thd->mem_root) Item_func_trig_cond(thd, new_having,
>                                                              get_cond_guard(0))))
>              DBUG_RETURN(true);
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

-- 
BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog