maria-developers team mailing list archive

[Vicențiu Ciorbaru <vicentiu@xxxxxxxxxxx>]

Hi Varun!

Here's a final review for Custom Aggregate Functions

* You haven't run the full test suite, there are a few failing tests.
Please update those.
  rpl.rpl_sp funcs_1.is_columns_mysql funcs_1.storedproc
* Push your code to a BB branch and rebase it on top of current 10.3.
* Fix commits messages while rebasing. Use the reword flag to
clean those up. The rule is this:
1. Have one line that doesn't exceed 80 Characters as a Title.
2. Leave one empty line
3. Add the rest of the commit message. Don't exceed 80 Characters per line.

> diff --git a/sql/sp_head.h b/sql/sp_head.h
> index bb516187a57..d447807bb3b 100644
> --- a/sql/sp_head.h
> +++ b/sql/sp_head.h
> @@ -334,6 +334,11 @@ class sp_head :private Query_arena,
>    execute_function(THD *thd, Item **args, uint argcount, Field
*return_fld);
>
>    bool
Rename this to execute_function please and delete the now dead-code from the
current execute_function. Update comments for this function appropriately.
Please run all tests again after this and make sure you update results for
all the failing ones.
> +  execute_aggregate_function(THD *thd, Item **args, uint argcount,
> +                             Field *return_fld, sp_rcontext **nctx,
> +                             MEM_ROOT *caller_mem_root);
> +
> +  bool
>    execute_procedure(THD *thd, List<Item> *args);
>
>    static void
> @@ -813,6 +818,9 @@ class sp_head :private Query_arena,
>    bool
>    execute(THD *thd, bool merge_da_on_success);
>
This is no longer used please remove!
> +  bool
> +  execute_agg(THD *thd, bool merge_da_on_success);
> +
>    /**
>      Perform a forward flow analysis in the generated code.
>      Mark reachable instructions, for the optimizer.

> +++ b/mysql-test/t/sp_agg.test
A lot of empty lines at the end of this test. Please delete those.

Ping me once you have a green buildbot before pushing.

Vicentiu