From: Jorgen Loland
Date: June 23 2011 12:02pm
Subject: Re: bzr commit into mysql-trunk branch (guilhem.bichot:3386) Bug#12668294
List-Archive: http://lists.mysql.com/commits/139732
Message-Id: <4E032B57.8090705@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

Hi Guilhem,

Patch is approved, but I find the return statement unnecessary hard to read. 
Please see below.

> === modified file 'sql/sql_select.h'
> --- a/sql/sql_select.h	2011-06-17 12:45:22 +0000
> +++ b/sql/sql_select.h	2011-06-20 14:29:59 +0000
> @@ -2029,10 +2029,16 @@ public:
>     void clear();
>     bool save_join_tab();
>     bool init_save_join_tab();
> +  /**
> +     If there is an aggregate function (sum_func_count!=0), and no GROUP BY,
> +     and the HAVING clause evaluates to "true", we should send a row even if
> +     the join contains no rows. This is what the standard requires.
> +   */
>     bool send_row_on_empty_set()
>     {
>       return (do_send_rows&&  tmp_table_param.sum_func_count != 0&&
> -	    !group_list&&  select_lex->having_value != Item::COND_FALSE);
> +	    !(group_list != NULL || group_optimized_away)&&
> +            select_lex->having_value != Item::COND_FALSE);

This double-negation makes me dizzy. Instead of "!(!A || B)" you can use "A && 
!B" and get this, which I find way more readable:

     return (do_send_rows && tmp_table_param.sum_func_count != 0 &&
             group_list==NULL && !group_optimized_away &&
             select_lex->having_value != Item::COND_FALSE);

-- 
Jørgen Løland | Senior Software Engineer | +47 73842138
Oracle MySQL
Trondheim, Norway