Skip to content

Commit c30119a

Browse files
committed
Testcase fix and code cleanup for window functions
- Make queries that use multiple window functions not to leak memory - Code cleanup in sql_window.cc
1 parent 687a51f commit c30119a

File tree

3 files changed

+21
-56
lines changed

3 files changed

+21
-56
lines changed

mysql-test/r/win.result

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ insert into t1 values (3, 10, 'xxx');
2929
insert into t1 values (3, 20, 'vvv');
3030
select a, row_number() over (partition by a order by b) from t1;
3131
a row_number() over (partition by a order by b)
32-
2 2
3332
2 1
33+
2 2
3434
2 3
3535
3 1
3636
3 2

mysql-test/t/win.test

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ insert into t1 values (2, 20, 'yy');
3838
insert into t1 values (3, 10, 'xxx');
3939
insert into t1 values (3, 20, 'vvv');
4040

41+
--sorted_result
4142
select a, row_number() over (partition by a order by b) from t1;
4243

4344
select a, b, x, row_number() over (partition by a order by x) from t1;

sql/sql_window.cc

Lines changed: 19 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -235,69 +235,32 @@ bool JOIN::process_window_functions(List<Item> *curr_fields_list)
235235
Item_window_func *item_win = (Item_window_func *) item;
236236
item_win->force_return_blank= false;
237237
Window_spec *spec = item_win->window_spec;
238-
238+
/*
239+
The sorting criteria should be
240+
(spec->partition_list, spec->order_list)
241+
242+
Connect the two lists for the duration of add_sorting_to_table()
243+
call.
244+
*/
239245
DBUG_ASSERT(spec->partition_list.next[0] == NULL);
240246
*(spec->partition_list.next)= spec->order_list.first;
241-
// spec->partition_list
242-
// spec->order_list
247+
248+
/*
249+
join_tab[top_join_tab_count].table is the temp. table where join
250+
output was stored.
251+
*/
243252
add_sorting_to_table(&join_tab[top_join_tab_count],
244253
spec->partition_list.first);
245254
join_tab[top_join_tab_count].used_for_window_func= true;
246255

247256
create_sort_index(this->thd, this, &join_tab[top_join_tab_count]);
257+
/* Disconnect order_list from partition_list */
248258
*(spec->partition_list.next)= NULL;
249-
//join_tab[top_join_tab_count] has the temp. table that we need.
250-
//bool JOIN::add_sorting_to_table(JOIN_TAB *tab, ORDER *order)
251-
252-
// spec->partition_list.first
253-
#if 0
254-
ha_rows examined_rows = 0;
255-
ha_rows found_rows = 0;
256-
ha_rows filesort_retval;
257-
/*
258-
psergey: Igor suggests to use create_sort_index() here, but I think
259-
it doesn't make sense: create_sort_index() assumes that it operates
260-
on a base table in the join.
261-
It calls test_if_skip_sort_order, checks for quick_select and what
262-
not.
263-
It also assumes that ordering comes either from ORDER BY or GROUP BY.
264-
todo: check this again.
265-
*/
266-
uint total_size= spec->partition_list.elements +
267-
spec->order_list.elements;
268-
SORT_FIELD *s_order=
269-
(SORT_FIELD *) my_malloc(sizeof(SORT_FIELD) * (total_size+1),
270-
MYF(MY_WME | MY_ZEROFILL | MY_THREAD_SPECIFIC));
271-
size_t pos= 0;
272-
for (ORDER* curr = spec->partition_list.first; curr; curr=curr->next, pos++)
273-
s_order[pos].item = *curr->item;
274-
275-
for (ORDER* curr = spec->order_list.first; curr; curr=curr->next, pos++)
276-
s_order[pos].item = *curr->item;
277-
278-
/* This is free'd by free_io_cache call below. */
279-
table[0]->sort.io_cache=(IO_CACHE*) my_malloc(sizeof(IO_CACHE),
280-
MYF(MY_WME | MY_ZEROFILL|
281-
MY_THREAD_SPECIFIC));
282-
283-
Filesort_tracker dummy_tracker(false);
284-
filesort_retval= filesort(thd, table[0], s_order,
285-
total_size,
286-
this->select, HA_POS_ERROR, FALSE,
287-
&examined_rows, &found_rows,
288-
&dummy_tracker);
289-
table[0]->sort.found_records= filesort_retval;
290-
291-
join_tab->read_first_record = join_init_read_record;
292-
join_tab->records= found_rows;
293-
294-
my_free(s_order);
295-
#endif
259+
296260
/*
297261
Go through the sorted array and compute the window function
298262
*/
299263
READ_RECORD info;
300-
//TABLE *tbl= *table;
301264
TABLE *tbl= join_tab[top_join_tab_count].table;
302265
if (init_read_record(&info, thd, tbl, select, 0, 1, FALSE))
303266
return true;
@@ -326,11 +289,12 @@ bool JOIN::process_window_functions(List<Item> *curr_fields_list)
326289
return true;
327290
}
328291
item_win->set_read_value_from_result_field();
292+
/* This calls filesort_free_buffers(): */
329293
end_read_record(&info);
330-
#if 0
331-
filesort_free_buffers(table[0], true);
332-
free_io_cache(table[0]);
333-
#endif
294+
295+
delete join_tab[top_join_tab_count].filesort;
296+
join_tab[top_join_tab_count].filesort= NULL;
297+
free_io_cache(tbl);
334298
}
335299
}
336300
}

0 commit comments

Comments
 (0)