maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Vicențiu,

> item_windowfunc.cc|168| peer_tracker = new Group_bound_tracker(thd, window_spec->order_list);
> item_windowfunc.cc|176| peer_tracker = new Group_bound_tracker(thd, window_spec->order_list);
> item_windowfunc.cc|218| peer_tracker = new Group_bound_tracker(thd, window_spec->order_list);

Coding style violation: Use peer_tracker=. 

> sql_window.cc:
>    // TODO check if this can be placed outside the loop.
>    err= tbl->file->ha_update_row(tbl->record[1], tbl->record[0]);

Yes, it should be. Can you try it? (and if it doesn't work that's surprising
and we could discuss why..)

> grep fech_prev_row *.h *.cc
> sql_window.cc|633| bool fetch_prev_row()

This function is not used anymore?

> static bool is_computed_with_remove(Item_sum::Sumfunctype sum_func)

What is the difference between this function and Item_sum::supports_removal ?
Does one imply/require the other?

> sql_window.h
> /*
>   This handles computation of one window function.
>
>   Currently, we make a spearate filesort() call for each window function.
> */
>
> class Window_func_runner : public Sql_alloc

Both statements in the comment are not true anymore, right? 
Can you update the comment?

> sql_window.cc
>  /*
>     Regular frame cursors add or remove values from the sum functions they
>     manage. By calling this method, they will only perform the required
>     movement within the table, but no adding/removing will happen.
>  */
>  void set_no_action()
>  {
>    perform_no_action= true;
>  }

Is there any difference between perform_no_action=true and
Frame_cursor::sum_functions being an empty list? Please clarify.

> sql_window.cc
> /*
>   A class that owns cursor objects associated with a specific window function.
> */
> class Cursor_manager
>

But as I understand, your recent changes make window functions share the frame
cursors. 

That is, window frame has its cursors, and all window functions that are
sharing the frame, share the cursors.

Is Cursor_manager still needed?

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