Skip to content

Commit

Permalink
MDEV-9634: Window function produces incorrect value
Browse files Browse the repository at this point in the history
- Item_sum_count::remove() should check if the argument's value is NULL.
- Window Function item must have its Item_window_func::split_sum_func
  called,
- and it must call split_sum_func for aggregate's arguments (see the
  comment near Item_window_func::split_sum_func for explanation why)
  • Loading branch information
spetrunia committed Feb 25, 2016
1 parent 0c6d753 commit d290a66
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 3 deletions.
37 changes: 37 additions & 0 deletions mysql-test/r/win.result
Expand Up @@ -423,3 +423,40 @@ from t1
window w1 as (partition by c), w2 as (w1 order by pk);
ERROR HY000: Unacceptable combination of window frame bound specifications
drop table t0,t1;
#
# MDEV-9634: Window function produces incorrect value
#
create table t0 (a int);
insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
create table t2 (part_id int, pk int, a int);
insert into t2 select
if(a<5, 0, 1), a, if(a<5, NULL, 1) from t0;
select * from t2;
part_id pk a
0 0 NULL
0 1 NULL
0 2 NULL
0 3 NULL
0 4 NULL
1 5 1
1 6 1
1 7 1
1 8 1
1 9 1
select
part_id, pk, a,
count(a) over (partition by part_id order by pk
rows between 1 preceding and 1 following) as CNT
from t2;
part_id pk a CNT
0 0 NULL 0
0 1 NULL 0
0 2 NULL 0
0 3 NULL 0
0 4 NULL 0
1 5 1 2
1 6 1 3
1 7 1 3
1 8 1 3
1 9 1 2
drop table t0, t2;
15 changes: 15 additions & 0 deletions mysql-test/t/win.test
Expand Up @@ -284,7 +284,22 @@ window w1 as (partition by c), w2 as (w1 order by pk);

drop table t0,t1;

--echo #
--echo # MDEV-9634: Window function produces incorrect value
--echo #

create table t0 (a int);
insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
create table t2 (part_id int, pk int, a int);
insert into t2 select
if(a<5, 0, 1), a, if(a<5, NULL, 1) from t0;
select * from t2;

select
part_id, pk, a,
count(a) over (partition by part_id order by pk
rows between 1 preceding and 1 following) as CNT
from t2;

drop table t0, t2;

3 changes: 2 additions & 1 deletion sql/item_sum.cc
Expand Up @@ -1538,8 +1538,9 @@ bool Item_sum_count::add()

void Item_sum_count::remove()
{
/* TODO: Handle DECIMAL type */
DBUG_ASSERT(aggr->Aggrtype() == Aggregator::SIMPLE_AGGREGATOR);
if (aggr->arg_is_null(false))
return;
count--;
}

Expand Down
36 changes: 36 additions & 0 deletions sql/item_windowfunc.cc
Expand Up @@ -65,6 +65,42 @@ Item_window_func::fix_fields(THD *thd, Item **ref)
}


/*
@detail
Window function evaluates its arguments when it is scanning the temporary
table in partition/order-by order. That is, arguments should be read from
the temporary table, not from the original base columns.
In order for this to work, we need to call "split_sum_func" for each
argument. The effect of the call is:
1. the argument is added into ref_pointer_array. This will cause the
argument to be saved in the temp.table
2. argument item is replaced with an Item_ref object. this object refers
the argument through the ref_pointer_array.
then, change_to_use_tmp_fields() will replace ref_pointer_array with an
array that points to the temp.table fields.
This way, when window_func attempts to evaluate its arguments, it will use
Item_ref objects which will read data from the temp.table.
Note: Before window functions, aggregate functions never needed to do such
transformations on their arguments. This is because grouping operation
does not need to read from the temp.table.
(Q: what happens when we first sort and then do grouping in a
group-after-group mode? dont group by items read from temp.table, then?)
*/

void Item_window_func::split_sum_func(THD *thd, Ref_ptr_array ref_pointer_array,
List<Item> &fields, uint flags)
{
for (uint i=0; i < window_func->argument_count(); i++)
{
Item **p_item= &window_func->arguments()[i];
(*p_item)->split_sum_func2(thd, ref_pointer_array, fields, p_item, flags);
}
}


/*
This must be called before advance_window() can be called.
Expand Down
2 changes: 2 additions & 0 deletions sql/item_windowfunc.h
Expand Up @@ -378,6 +378,8 @@ class Item_window_func : public Item_result_field
window_func->val_decimal(dec);
}

void split_sum_func(THD *thd, Ref_ptr_array ref_pointer_array,
List<Item> &fields, uint flags);
void fix_length_and_dec()
{
window_func->fix_length_and_dec();
Expand Down
8 changes: 6 additions & 2 deletions sql/sql_base.cc
Expand Up @@ -7966,8 +7966,12 @@ bool setup_fields(THD *thd, Ref_ptr_array ref_pointer_array,
ref[0]= item;
ref.pop_front();
}
if (item->with_sum_func && item->type() != Item::SUM_FUNC_ITEM &&
sum_func_list)
/*
split_sum_func() must be called for Window Function items, see
Item_window_func::split_sum_func.
*/
if ((item->with_sum_func && item->type() != Item::SUM_FUNC_ITEM &&
sum_func_list) || item->type() == Item::WINDOW_FUNC_ITEM)
item->split_sum_func(thd, ref_pointer_array, *sum_func_list,
SPLIT_SUM_SELECT);
thd->lex->current_select->select_list_tables|= item->used_tables();
Expand Down

0 comments on commit d290a66

Please sign in to comment.