Hi Varun, - Please add a testcase to the testsuite - Please address a few cosmetic comments below - Ok to push after addressed.
> commit 5e448b77a3812e65623c0a1214049322ace3aacf > Author: Varun Gupta <varun.gu...@mariadb.com> > Date: Tue May 5 20:44:43 2020 +0530 > > MDEV-22461: JOIN::make_aggr_tables_info(): Assertion `select_options & > (1ULL << 17)' failed. > > A temporary table is needed for window function computation but if only a > NAMED WINDOW SPEC > is used and there is no window function, then there is no need to create > a temporary > table as there is no stage to compute WINDOW FUNCTION > > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index c601946cfa0..0a1d0c2dbcc 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -2014,11 +2014,16 @@ JOIN::optimize_inner() > } > > need_tmp= test_if_need_tmp_table(); > - //TODO this could probably go in test_if_need_tmp_table. > - if (this->select_lex->window_specs.elements > 0) { > - need_tmp= TRUE; > + > + /* > + If window functions are present then we can't have simple_order set to > + TRUE as the window function needs a temp table for compuatation. typo: compuatation. > + ORDER BY is computed after the window function computation is done, so > + the sort would be done on the temp table. > + */ > + if (this->select_lex->have_window_funcs()) Is there a need to have "this->select_lex"? I think not, please remove, as it only confuses the reader. > simple_order= FALSE; > - } > + > > /* > If the hint FORCE INDEX FOR ORDER BY/GROUP BY is used for the table > diff --git a/sql/sql_select.h b/sql/sql_select.h > index 0e011c9267a..7a892c1af89 100644 > --- a/sql/sql_select.h > +++ b/sql/sql_select.h > @@ -1645,7 +1645,8 @@ class JOIN :public Sql_alloc > ((select_distinct || !simple_order || !simple_group) || > (group_list && order) || > MY_TEST(select_options & OPTION_BUFFER_RESULT))) || > - (rollup.state != ROLLUP::STATE_NONE && select_distinct)); > + (rollup.state != ROLLUP::STATE_NONE && select_distinct) || > + select_lex->have_window_funcs()); Please amend the function comment above this code to cover the new condition as well. > } > bool choose_subquery_plan(table_map join_tables); > void get_partial_cost_and_fanout(int end_tab_idx, > BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp