[MDEV-9634] Window function produces incorrect value Created: 2016-02-25  Updated: 2016-02-26  Resolved: 2016-02-26

Status: Closed
Project: MariaDB Server
Component/s: Optimizer
Fix Version/s: N/A

Type: Task Priority: Major
Reporter: Sergei Petrunia Assignee: Sergei Petrunia
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
PartOf
is part of MDEV-9526 Compute Aggregate functions as window... Closed

 Description   

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 |   0 |
|       1 |    6 |    1 |   0 |
|       1 |    7 |    1 |   0 |
|       1 |    8 |    1 |   0 |
|       1 |    9 |    1 |   0 |
+---------+------+------+-----+

In other settings the value of window can be different.



 Comments   
Comment by Sergei Petrunia [ 2016-02-25 ]

An apparent fix is to make Item_sum_count::remove to take NULLs into account:

diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index 91abea8..d9a92c7 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -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--;
 }

and it doesn't help.

Comment by Sergei Petrunia [ 2016-02-25 ]

  #0  Item_field::is_null (this=0x7fff540078c8) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/item.h:2457
  #1  0x0000555555d699d9 in Aggregator_simple::arg_is_null (this=0x7fff5400b340, use_null_value=false) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/item_sum.cc:1478
  #2  0x0000555555d69cb2 in Item_sum_count::add (this=0x7fff540079b8) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/item_sum.cc:1528
  #3  0x0000555555c1bfa3 in Frame_n_rows::next_row_intern (this=0x7fff5400b448, item=0x7fff540079b8) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/sql_window.cc:419
  #4  0x0000555555c1be5d in Frame_n_rows::next_partition (this=0x7fff5400b448, first=true, item=0x7fff540079b8) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/sql_window.cc:388
  #5  0x0000555555c1b1e7 in compute_window_func_with_frames (item_win=0x7fff54007f40, tbl=0x7fff540182c8, info=0x7ffff42ffb90) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/sql_window.cc:536
  #6  0x0000555555c1b72d in JOIN::process_window_functions (this=0x7fff54008728, curr_fields_list=0x55555ab28840) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/sql_window.cc:803
  #7  0x0000555555af00bb in AGGR_OP::end_send (this=0x7fff5400a720) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/sql_select.cc:25803
  #8  0x0000555555adc54b in sub_select_postjoin_aggr (join=0x7fff54008728, join_tab=0x7fff54009d40, end_of_records=true) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/sql_select.cc:17869
  #9  0x0000555555adc83e in sub_select (join=0x7fff54008728, join_tab=0x7fff540099b0, end_of_records=true) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/sql_select.cc:18105
  #10 0x0000555555adc105 in do_select (join=0x7fff54008728, procedure=0x0) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/sql_select.cc:17699
  #11 0x0000555555ab916a in JOIN::exec_inner (this=0x7fff54008728) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/sql_select.cc:3243
  #12 0x0000555555ab86d1 in JOIN::exec (this=0x7fff54008728) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/sql_select.cc:3052
  #13 0x0000555555ab9806 in mysql_select (thd=0x55555ab24630, tables=0x7fff54008050, wild_num=0, fields=..., conds=0x0, og_num=0, order=0x0, group=0x0, having=0x0, proc_param=0x0, select_options=2147748608, result=0x7fff54008708, unit=0x55555ab28038, select_lex=0x55555ab28728) at /home/psergey/dev-git/10.2-window-simple-merge-try3/sql/sql_select.cc:3438

(gdb) p field->table->file
  $15 = (ha_innobase *) 0x7fff5400fe18
(gdb) p field->table->s->table_name
  $16 = {str = 0x7fff54006deb "t2", length = 2}

Comment by Sergei Petrunia [ 2016-02-25 ]

So, the parameters of window functions are not aware that they should be looking into the buffer tmp. table. They are looking at the field in the original table instead, which has some value that has nothing to do with the row pointed by the frame bound cursor.

Comment by Sergei Petrunia [ 2016-02-25 ]

Checking how this is done for other kinds of queries.

We need to read from temp. table
1. when using SQL_BUFFER_RESULT
2. when doing sorting after we've done grouping

#1 is achieved by setting JOIN::fields to point to tmp_fields_list1.
This is done in :make_aggr_tables_info. Then, join->result->send_row(fields)
reads data from tmp_fields_list1 which has Item_temtable_field objects.

#2 also explicity reads from the temp.table
The key is produced by make_sortkey,find_all_keys,filesort,create_sort_index.
and there, sort_field structure has Field object that points into the temp.table.

Comment by Sergei Petrunia [ 2016-02-25 ]

On the other hand, when process_window_functions() invokes filesort, it manages to sort the data in the temp.table.

Looking how it does that. In process_window_functions() window's PARTITION/ORDER point into the temp.table already:

(gdb) p spec->partition_list.first->item[0]
  $104 = (Item_temptable_field *) 0x7fff5c00a730

This is caused by

  • ORDER structures in window's PARTITION/ORDER lists keep Item**.
  • fix_fields, setup_group – here Item** is set to point to join's ref_pointer_array.
  • JOIN::copy_ref_ptr_array, JOIN::set_items_ref_array, JOIN::make_aggr_tables_info – here ref_pointer_array is switched to point to the temp.table fields.
Comment by Sergei Petrunia [ 2016-02-26 ]

Pushed into bb-10.2-mdev9543 branch.

Generated at Thu Feb 08 07:36:09 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.