List:Commits« Previous MessageNext Message »
From:Jorgen Loland Date:February 9 2011 2:57pm
Subject:Re: bzr commit into mysql-5.1 branch (martin.hansson:3587) Bug#59839
View as plain text  
Hi Martin,

What a cute little patch ;) Code approved, but see comment re. documentation

On 02/09/2011 10:58 AM, Martin Hansson wrote:
> #At file:///data0/martin/bzrroot/bug59839/5.1-commit/ based on
> revid:dmitry.shulga@stripped
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc	2011-02-08 11:52:33 +0000
> +++ b/sql/sql_select.cc	2011-02-09 09:58:43 +0000
> @@ -281,46 +281,54 @@ bool handle_select(THD *thd, LEX *lex, s
>   /*
>     Fix fields referenced from inner selects.
>
> -  SYNOPSIS
> -    fix_inner_refs()
> -    thd               Thread handle
> -    all_fields        List of all fields used in select
> -    select            Current select
> -    ref_pointer_array Array of references to Items used in current select
> -    group_list        GROUP BY list (is NULL by default)
> +  @param thd               Thread handle
> +  @param all_fields        List of all fields used in select
> +  @param select            Current select
> +  @param ref_pointer_array Array of references to Items used in current select
> +  @param group_list        GROUP BY list (is NULL by default)
>
> -  DESCRIPTION
> -    The function serves 3 purposes - adds fields referenced from inner
> -    selects to the current select list, resolves which class to use
> -    to access referenced item (Item_ref of Item_direct_ref) and fixes
> -    references (Item_ref objects) to these fields.
> +  @details
> +    The function serves 3 purposes
> +
> +    - adds fields referenced from inner query blocks to the current select list
>
> -    If a field isn't already in the select list and the ref_pointer_array
> +    - resolves which class to use to access referenced item (Item_ref or
> +      Item_direct_ref)
> +
> +    - fixes references (Item_ref objects) to these fields.
> +
> +    If a field isn't already on the select list and the ref_pointer_array
>       is provided then it is added to the all_fields list and the pointer to
>       it is saved in the ref_pointer_array.
>
>       The class to access the outer field is determined by the following rules:
> -    1. If the outer field isn't used under an aggregate function
> -      then the Item_ref class should be used.
> -    2. If the outer field is used under an aggregate function and this
> -      function is aggregated in the select where the outer field was
> -      resolved or in some more inner select then the Item_direct_ref
> -      class should be used.
> -      Also it should be used if we are grouping by a subquery containing
> -      the outer field.
> +
> +    -#. If the outer field isn't used under an aggregate function then the
> +        Item_ref class should be used.
> +
> +    -#. If the outer field is used under an aggregate function and this
> +        function is, in turn, aggregated in the query block where the outer
> +        field was resolved or some query nested therein, then the
> +        Item_direct_ref class should be used. Also it should be used if we are
> +        grouping by a subquery containing the outer field.
> +
>       The resolution is done here and not at the fix_fields() stage as
> -    it can be done only after sum functions are fixed and pulled up to
> -    selects where they are have to be aggregated.
> +    it can be done only after aggregate functions are fixed and pulled up to
> +    selects where they are to be aggregated.
> +
>       When the class is chosen it substitutes the original field in the
>       Item_outer_ref object.
>
>       After this we proceed with fixing references (Item_outer_ref objects) to
>       this field from inner subqueries.
>
> -  RETURN
> -    TRUE  an error occured
> -    FALSE ok
> -*/
> +  @return Status
> +  @retval true An error occured.
> +  @retval false OK.
> +
> +  @todo This comment must be restructured. It is jumping back and forth
> +        between abstraction levels and it uses inconsistent terminology.
> + */

You've modified almost the entire documentation for this function. Why don't you 
go ahead an restructure it now that you have the code fresh in mind. Noone else 
is going to do it ;)

-- 
Jørgen Løland | Senior Software Engineer | +47 73842138
Oracle MySQL
Trondheim, Norway
Thread
bzr commit into mysql-5.1 branch (martin.hansson:3587) Bug#59839Martin Hansson9 Feb
Re: bzr commit into mysql-5.1 branch (martin.hansson:3587) Bug#59839Jorgen Loland9 Feb
  • Re: bzr commit into mysql-5.1 branch (martin.hansson:3587) Bug#59839Martin Hansson9 Feb
Re: bzr commit into mysql-5.1 branch (martin.hansson:3587) Bug#59839Roy Lyseng15 Feb
  • Re: bzr commit into mysql-5.1 branch (martin.hansson:3587) Bug#59839Martin Hansson15 Feb
    • Re: bzr commit into mysql-5.1 branch (martin.hansson:3587) Bug#59839Roy Lyseng15 Feb
      • Re: bzr commit into mysql-5.1 branch (martin.hansson:3587) Bug#59839Martin Hansson15 Feb