From: Jorgen Loland
Date: November 4 2010 3:05pm
Subject: Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem:3330)
 Bug#57928
List-Archive: http://lists.mysql.com/commits/122850
Message-Id: <4CD2CBB1.9080709@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

Guilhem,

The code looks good with Tor's suggestions. Thanks for making keyuse a class. I 
have two suggestions about documenting the class. Please see inline.

> === modified file 'sql/sql_select.h'
> --- a/sql/sql_select.h	2010-09-23 12:16:36 +0000
> +++ b/sql/sql_select.h	2010-11-03 13:54:05 +0000
> @@ -39,17 +39,34 @@
>   #define KEY_OPTIMIZE_EXISTS		1
>   #define KEY_OPTIMIZE_REF_OR_NULL	2
>
> -typedef struct keyuse_t {
> +/**
> +  @note such objects are stored in DYNAMIC_ARRAY which uses sizeof(), so keep
> +  this class as POD as possible.
> +*/

Can you add more to the class documentation?

> +class KEYUSE {
> +public:
> +  KEYUSE(TABLE *table_arg, Item *val_arg, table_map used_tables_arg,
> +         uint key_arg, uint keypart_arg, uint optimize_arg,
> +         key_part_map keypart_map_arg, ha_rows ref_table_rows_arg,
> +         bool null_rejecting_arg, bool *cond_guard_arg,
> +         uint sj_pred_no_arg) :
> +  table(table_arg), val(val_arg), used_tables(used_tables_arg), key(key_arg),
> +  keypart(keypart_arg), optimize(optimize_arg), keypart_map(keypart_map_arg),
> +  ref_table_rows(ref_table_rows_arg), null_rejecting(null_rejecting_arg),
> +  cond_guard(cond_guard_arg), sj_pred_no(sj_pred_no_arg) {}

I know you didn't change most of these variables, but if you have more info on 
each of them without spending hours, it would be good to get doxygen comments on 
all.
-- 
Jørgen Løland | Senior Software Engineer | +47 73842138
Oracle MySQL
Trondheim, Norway