[MDEV-9634] Window function produces incorrect value - Jira

Details

Type: Task
Status: Closed (View Workflow)
Priority: Major
Resolution: Fixed
Fix Version/s: N/A
Component/s: Optimizer
Labels:
None

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.

Attachments

Issue Links

is part of

MDEV-9526 Compute Aggregate functions as window functions

Closed

Activity

Ascending order - Click to sort in descending order

Sergei Petrunia added a comment - 2016-02-25 15:41

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.

Sergei Petrunia added a comment - 2016-02-25 15:41 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.

Sergei Petrunia added a comment - 2016-02-25 15:55

  #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}

Sergei Petrunia added a comment - 2016-02-25 15:55 #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}

Sergei Petrunia added a comment - 2016-02-25 16:02

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.

Sergei Petrunia added a comment - 2016-02-25 16:02 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.

Sergei Petrunia added a comment - 2016-02-25 16:44

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.

Sergei Petrunia added a comment - 2016-02-25 16:44 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.

Sergei Petrunia added a comment - 2016-02-25 17:04

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.

Sergei Petrunia added a comment - 2016-02-25 17:04 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.

Sergei Petrunia added a comment - 2016-02-26 01:11

Pushed into bb-10.2-mdev9543 branch.

Sergei Petrunia added a comment - 2016-02-26 01:11 Pushed into bb-10.2-mdev9543 branch.

People

Assignee:: Sergei Petrunia

Reporter:: Sergei Petrunia

Votes:: 0 Vote for this issue

Watchers:: 1 Start watching this issue

Dates

Created:: 2016-02-25 15:39

Updated:: 2016-02-26 01:11

Resolved:: 2016-02-26 01:11

Git Integration

Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.

MariaDB Server