diff --git a/mysql-test/r/win.result b/mysql-test/r/win.result index 9dbee937fd2d3..a6b43788ffe42 100644 --- a/mysql-test/r/win.result +++ b/mysql-test/r/win.result @@ -1,4 +1,5 @@ drop table if exists t1,t2; +drop view if exists v1; # ######################################################################## # # Parser tests # ######################################################################## @@ -1916,3 +1917,45 @@ EXPLAIN } } drop table t1; +# +# MDEV-9893: Window functions with different ORDER BY lists, +# one of these lists containing an expression +# +create table t1 (s1 int, s2 char(5)); +insert into t1 values (1,'a'); +insert into t1 values (null,null); +insert into t1 values (3,null); +insert into t1 values (4,'a'); +insert into t1 values (2,'b'); +insert into t1 values (-1,''); +select +*, +ROW_NUMBER() OVER (order by s1), +CUME_DIST() OVER (order by -s1) +from t1; +s1 s2 ROW_NUMBER() OVER (order by s1) CUME_DIST() OVER (order by -s1) +1 a 3 0.8333333333 +NULL NULL 1 0.1666666667 +3 NULL 5 0.5000000000 +4 a 6 0.3333333333 +2 b 4 0.6666666667 +-1 2 1.0000000000 +drop table t1; +# +# MDEV-9925: Wrong result with aggregate function as a window function +# +create table t1 (i int); +insert into t1 values (1),(2); +select i, sum(i) over (partition by i) from t1; +i sum(i) over (partition by i) +1 1 +2 2 +drop table t1; +# +# MDEV-9922: Assertion `!join->only_const_tables() && fsort' failed in int create_sort_index +# +create view v1 as select 1 as i; +select rank() over (order by i) from v1; +rank() over (order by i) +1 +drop view v1; diff --git a/mysql-test/r/win_rank.result b/mysql-test/r/win_rank.result index 54f70147b1a1a..725683d38699f 100644 --- a/mysql-test/r/win_rank.result +++ b/mysql-test/r/win_rank.result @@ -40,11 +40,11 @@ pk a b rank dense_rank 3 1 10 3 2 4 1 10 3 2 8 2 10 5 3 -5 2 20 1 0 -6 2 20 1 0 -7 2 20 1 0 -9 4 20 4 1 -10 4 20 4 1 +5 2 20 1 1 +6 2 20 1 1 +7 2 20 1 1 +9 4 20 4 2 +10 4 20 4 2 drop table t1; # # Test with null values in the table. diff --git a/mysql-test/t/win.test b/mysql-test/t/win.test index dffeb39ae8c6e..09ddf41b4f05c 100644 --- a/mysql-test/t/win.test +++ b/mysql-test/t/win.test @@ -4,6 +4,7 @@ --disable_warnings drop table if exists t1,t2; +drop view if exists v1; --enable_warnings --echo # ######################################################################## @@ -1163,3 +1164,40 @@ select distinct rank() over (partition by part_id order by a) from t1; drop table t1; +--echo # +--echo # MDEV-9893: Window functions with different ORDER BY lists, +--echo # one of these lists containing an expression +--echo # + +create table t1 (s1 int, s2 char(5)); +insert into t1 values (1,'a'); +insert into t1 values (null,null); +insert into t1 values (3,null); +insert into t1 values (4,'a'); +insert into t1 values (2,'b'); +insert into t1 values (-1,''); + +select + *, + ROW_NUMBER() OVER (order by s1), + CUME_DIST() OVER (order by -s1) +from t1; + +drop table t1; + + +--echo # +--echo # MDEV-9925: Wrong result with aggregate function as a window function +--echo # +create table t1 (i int); +insert into t1 values (1),(2); +select i, sum(i) over (partition by i) from t1; +drop table t1; + +--echo # +--echo # MDEV-9922: Assertion `!join->only_const_tables() && fsort' failed in int create_sort_index +--echo # +create view v1 as select 1 as i; +select rank() over (order by i) from v1; +drop view v1; + diff --git a/sql/item_windowfunc.cc b/sql/item_windowfunc.cc index c8f398577b5a5..d157d545dadb5 100644 --- a/sql/item_windowfunc.cc +++ b/sql/item_windowfunc.cc @@ -185,8 +185,11 @@ void Item_sum_dense_rank::setup_window_func(THD *thd, Window_spec *window_spec) bool Item_sum_dense_rank::add() { - if (peer_tracker.check_if_next_group()) + if (peer_tracker.check_if_next_group() || first_add) + { + first_add= false; dense_rank++; + } return false; } diff --git a/sql/item_windowfunc.h b/sql/item_windowfunc.h index 35fff511e5ab9..40f48cc7dc5a0 100644 --- a/sql/item_windowfunc.h +++ b/sql/item_windowfunc.h @@ -213,7 +213,9 @@ class Item_sum_rank: public Item_sum_int class Item_sum_dense_rank: public Item_sum_int { longlong dense_rank; + bool first_add; Group_bound_tracker peer_tracker; + public: /* XXX(cvicentiu) This class could potentially be implemented in the rank class, with a switch for the DENSE case. @@ -221,6 +223,7 @@ class Item_sum_dense_rank: public Item_sum_int void clear() { dense_rank= 0; + first_add= true; } bool add(); void update_field() {} @@ -229,9 +232,8 @@ class Item_sum_dense_rank: public Item_sum_int return dense_rank; } - public: Item_sum_dense_rank(THD *thd) - : Item_sum_int(thd), dense_rank(0) {} + : Item_sum_int(thd), dense_rank(0), first_add(true) {} enum Sumfunctype sum_func () const { return DENSE_RANK_FUNC; @@ -427,7 +429,6 @@ class Item_sum_ntile : public Item_sum_window_with_row_count public: Item_sum_ntile(THD* thd, Item* num_quantiles_expr) : Item_sum_window_with_row_count(thd, num_quantiles_expr), - num_quantiles_expr_(num_quantiles_expr), current_row_count_(0) {}; double val_real() @@ -443,7 +444,7 @@ class Item_sum_ntile : public Item_sum_window_with_row_count return 0; } - longlong num_quantiles= num_quantiles_expr_->val_int(); + longlong num_quantiles= get_num_quantiles(); if (num_quantiles <= 0) { my_error(ER_INVALID_NTILE_ARGUMENT, MYF(0)); @@ -487,19 +488,8 @@ class Item_sum_ntile : public Item_sum_window_with_row_count enum Item_result result_type () const { return INT_RESULT; } enum_field_types field_type() const { return MYSQL_TYPE_LONGLONG; } - bool fix_fields(THD *thd, Item **ref) - { - if (Item_sum_window_with_row_count::fix_fields(thd, ref)) - return true; - // TODO-cvicentiu is ref as a parameter here ok? - if (!num_quantiles_expr_->fixed) - num_quantiles_expr_->fix_fields(thd, ref); - - return false; - } - private: - Item* num_quantiles_expr_; + longlong get_num_quantiles() { return args[0]->val_int(); } ulong current_row_count_; }; diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 009a642a790da..6cc78a4c29468 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -6824,6 +6824,8 @@ find_field_in_tables(THD *thd, Item_ident *item, or as a field name without alias, or as a field hidden by alias, or ignoring alias) + limit How many items in the list to check + (if limit==0 then all items are to be checked) RETURN VALUES 0 Item is not found or item is not unique, @@ -6841,9 +6843,10 @@ Item **not_found_item= (Item**) 0x1; Item ** find_item_in_list(Item *find, List &items, uint *counter, find_item_error_report_type report_error, - enum_resolution_type *resolution) + enum_resolution_type *resolution, uint limit) { List_iterator li(items); + uint n_items= limit == 0 ? items.elements : limit; Item **found=0, **found_unaliased= 0, *item; const char *db_name=0; const char *field_name=0; @@ -6867,8 +6870,9 @@ find_item_in_list(Item *find, List &items, uint *counter, db_name= ((Item_ident*) find)->db_name; } - for (uint i= 0; (item=li++); i++) + for (uint i= 0; i < n_items; i++) { + item= li++; if (field_name && (item->real_item()->type() == Item::FIELD_ITEM || ((item->type() == Item::REF_ITEM) && diff --git a/sql/sql_base.h b/sql/sql_base.h index 24a7c7a5a2e96..ecadd31d3f982 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -197,7 +197,7 @@ Field * find_field_in_table_sef(TABLE *table, const char *name); Item ** find_item_in_list(Item *item, List &items, uint *counter, find_item_error_report_type report_error, - enum_resolution_type *resolution); + enum_resolution_type *resolution, uint limit= 0); bool setup_tables(THD *thd, Name_resolution_context *context, List *from_clause, TABLE_LIST *tables, List &leaves, bool select_insert, diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 6220ff0e9edbf..52e035d6e7513 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -21221,8 +21221,6 @@ create_sort_index(THD *thd, JOIN *join, JOIN_TAB *tab, Filesort *fsort) if (fsort == NULL) fsort= tab->filesort; - // One row, no need to sort. make_tmp_tables_info should already handle this. - DBUG_ASSERT(!join->only_const_tables() && fsort); table= tab->table; select= fsort->select; @@ -21734,6 +21732,7 @@ cp_buffer_from_ref(THD *thd, TABLE *table, TABLE_REF *ref) @param[in,out] all_fields All select, group and order by fields @param[in] is_group_field True if order is a GROUP field, false if ORDER by field + @param[in] search_in_all_fields If true then search in all_fields @retval FALSE if OK @@ -21744,7 +21743,7 @@ cp_buffer_from_ref(THD *thd, TABLE *table, TABLE_REF *ref) static bool find_order_in_list(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables, ORDER *order, List &fields, List &all_fields, - bool is_group_field) + bool is_group_field, bool search_in_all_fields) { Item *order_item= *order->item; /* The item from the GROUP/ORDER caluse. */ Item::Type order_item_type; @@ -21849,6 +21848,18 @@ find_order_in_list(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables thd->where); } } + else if (search_in_all_fields) + { + Item **found_item= find_item_in_list(order_item, all_fields, &counter, + REPORT_EXCEPT_NOT_FOUND, &resolution, + all_fields.elements - fields.elements); + if (found_item != not_found_item) + { + order->item= &ref_pointer_array[all_fields.elements-1-counter]; + order->in_field_list= 0; + return FALSE; + } + } order->in_field_list=0; /* @@ -21896,14 +21907,15 @@ find_order_in_list(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables */ int setup_order(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables, - List &fields, List &all_fields, ORDER *order) + List &fields, List &all_fields, ORDER *order, + bool search_in_all_fields) { enum_parsing_place parsing_place= thd->lex->current_select->parsing_place; thd->where="order clause"; for (; order; order=order->next) { if (find_order_in_list(thd, ref_pointer_array, tables, order, fields, - all_fields, FALSE)) + all_fields, FALSE, search_in_all_fields)) return 1; if ((*order->item)->with_window_func && parsing_place != IN_ORDER_BY) { @@ -21918,18 +21930,19 @@ int setup_order(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables, /** Intitialize the GROUP BY list. - @param thd Thread handler - @param ref_pointer_array We store references to all fields that was + @param thd Thread handler + @param ref_pointer_array We store references to all fields that was not in 'fields' here. - @param fields All fields in the select part. Any item in + @param fields All fields in the select part. Any item in 'order' that is part of these list is replaced by a pointer to this fields. - @param all_fields Total list of all unique fields used by the + @param all_fields Total list of all unique fields used by the select. All items in 'order' that was not part of fields will be added first to this list. - @param order The fields we should do GROUP BY on. - @param hidden_group_fields Pointer to flag that is set to 1 if we added + @param order The fields we should do GROUP/PARTITION BY on + @param hidden_group_fields Pointer to flag that is set to 1 if we added any fields to all_fields. + @param search_in_all_fields If true then search in all_fields @todo change ER_WRONG_FIELD_WITH_GROUP to more detailed @@ -21944,7 +21957,7 @@ int setup_order(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables, int setup_group(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables, List &fields, List &all_fields, ORDER *order, - bool *hidden_group_fields) + bool *hidden_group_fields, bool search_in_all_fields) { enum_parsing_place parsing_place= thd->lex->current_select->parsing_place; *hidden_group_fields=0; @@ -21959,7 +21972,7 @@ setup_group(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables, for (ord= order; ord; ord= ord->next) { if (find_order_in_list(thd, ref_pointer_array, tables, ord, fields, - all_fields, TRUE)) + all_fields, TRUE, search_in_all_fields)) return 1; (*ord->item)->marker= UNDEF_POS; /* Mark found */ if ((*ord->item)->with_sum_func && parsing_place == IN_GROUP_BY) diff --git a/sql/sql_select.h b/sql/sql_select.h index 1718a4676f16f..d4dc691c547cc 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -1942,10 +1942,11 @@ int get_quick_record(SQL_SELECT *select); SORT_FIELD * make_unireg_sortorder(THD *thd, ORDER *order, uint *length, SORT_FIELD *sortorder); int setup_order(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables, - List &fields, List &all_fields, ORDER *order); + List &fields, List &all_fields, ORDER *order, + bool search_in_all_fields= true); int setup_group(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables, List &fields, List &all_fields, ORDER *order, - bool *hidden_group_fields); + bool *hidden_group_fields, bool search_in_all_fields= true); bool fix_inner_refs(THD *thd, List &all_fields, SELECT_LEX *select, Ref_ptr_array ref_pointer_array); int join_read_key2(THD *thd, struct st_join_table *tab, TABLE *table, diff --git a/sql/sql_window.cc b/sql/sql_window.cc index cd7d8df0c9372..d8aa79130a42d 100644 --- a/sql/sql_window.cc +++ b/sql/sql_window.cc @@ -122,9 +122,10 @@ setup_windows(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables, bool hidden_group_fields; if (win_spec->check_window_names(itp) || setup_group(thd, ref_pointer_array, tables, fields, all_fields, - win_spec->partition_list->first, &hidden_group_fields) || + win_spec->partition_list->first, &hidden_group_fields, + true) || setup_order(thd, ref_pointer_array, tables, fields, all_fields, - win_spec->order_list->first) || + win_spec->order_list->first, true) || (win_spec->window_frame && win_spec->window_frame->check_frame_bounds())) { @@ -236,21 +237,21 @@ setup_windows(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables, static int compare_order_elements(ORDER *ord1, ORDER *ord2) { + if (*ord1->item == *ord2->item) + return CMP_EQ; Item *item1= (*ord1->item)->real_item(); Item *item2= (*ord2->item)->real_item(); - if (item1->used_tables() == item2->used_tables()) + DBUG_ASSERT(item1->type() == Item::FIELD_ITEM && + item2->type() == Item::FIELD_ITEM); + int cmp= ((Item_field *) item1)->field - ((Item_field *) item2)->field; + if (cmp == 0) { - int cmp= strcmp(item1->name, item2->name); - if (cmp == 0) - { - if (ord1->direction == ord2->direction) - return CMP_EQ; - return ord1->direction > ord2->direction ? CMP_GT : CMP_LT; - } - else - return cmp > 0 ? CMP_GT : CMP_LT; + if (ord1->direction == ord2->direction) + return CMP_EQ; + return ord1->direction > ord2->direction ? CMP_GT : CMP_LT; } - return item1->used_tables() > item2->used_tables() ? CMP_GT : CMP_LT; + else + return cmp > 0 ? CMP_GT : CMP_LT; } static @@ -667,11 +668,68 @@ class Table_read_cursor : public Rowid_seq_cursor // todo: should move_to() also read row here? }; + /* - TODO: We should also have a cursor that reads table rows and - stays within the current partition. + A cursor which only moves within a partition. The scan stops at the partition + end, and it needs an explicit command to move to the next partition. */ +class Partition_read_cursor +{ + Table_read_cursor tbl_cursor; + Group_bound_tracker bound_tracker; + bool end_of_partition; +public: + void init(THD *thd, READ_RECORD *info, SQL_I_List *partition_list) + { + tbl_cursor.init(info); + bound_tracker.init(thd, partition_list); + end_of_partition= false; + } + + /* + Informs the cursor that we need to move into the next partition. + The next partition is provided in two ways: + - in table->record[0].. + - rownum parameter has the row number. + */ + void on_next_partition(int rownum) + { + /* Remember the sort key value from the new partition */ + bound_tracker.check_if_next_group(); + end_of_partition= false; + } + + /* + Moves to a new row. The row is assumed to be within the current partition + */ + void move_to(int rownum) { tbl_cursor.move_to(rownum); } + + /* + This returns -1 when end of partition was reached. + */ + int get_next() + { + int res; + if (end_of_partition) + return -1; + if ((res= tbl_cursor.get_next())) + return res; + + if (bound_tracker.compare_with_cache()) + { + end_of_partition= true; + return -1; + } + return 0; + } + + bool restore_last_row() + { + return tbl_cursor.restore_last_row(); + } +}; + ///////////////////////////////////////////////////////////////////////////// @@ -685,6 +743,27 @@ class Table_read_cursor : public Rowid_seq_cursor The cursor also assumes that the current row moves forward through the partition and will move to the next adjacent partition after this one. + List of all cursor classes: + Frame_cursor + Frame_range_n_top + Frame_range_n_bottom + + Frame_range_current_row_top + Frame_range_current_row_bottom + + Frame_n_rows_preceding + Frame_n_rows_following + + Frame_rows_current_row_top = Frame_n_rows_preceding(0) + Frame_rows_current_row_bottom + + // These handle both RANGE and ROWS-type bounds + Frame_unbounded_preceding + Frame_unbounded_following + + // This is not used as a frame bound, it counts rows in the partition: + Frame_unbounded_following_set_count : public Frame_unbounded_following + @todo - if we want to allocate this on the MEM_ROOT we should make sure it is not re-allocated for every subquery execution. @@ -851,7 +930,7 @@ class Frame_range_n_top : public Frame_cursor class Frame_range_n_bottom: public Frame_cursor { - Table_read_cursor cursor; + Partition_read_cursor cursor; Cached_item_item *range_expr; @@ -860,7 +939,6 @@ class Frame_range_n_bottom: public Frame_cursor const bool is_preceding; - Group_bound_tracker bound_tracker; bool end_of_partition; /* @@ -877,7 +955,7 @@ class Frame_range_n_bottom: public Frame_cursor SQL_I_List *partition_list, SQL_I_List *order_list) { - cursor.init(info); + cursor.init(thd, info, partition_list); DBUG_ASSERT(order_list->elements == 1); Item *src_expr= order_list->first->item[0]; @@ -899,8 +977,6 @@ class Frame_range_n_bottom: public Frame_cursor item_add= new (thd->mem_root) Item_func_plus(thd, src_expr, n_val); item_add->fix_fields(thd, &item_add); - - bound_tracker.init(thd, partition_list); } void pre_next_partition(longlong rownum, Item_sum* item) @@ -908,7 +984,7 @@ class Frame_range_n_bottom: public Frame_cursor // Save the value of FUNC(current_row) range_expr->fetch_value_from(item_add); - bound_tracker.check_if_next_group(); + cursor.on_next_partition(rownum); end_of_partition= false; } @@ -949,11 +1025,6 @@ class Frame_range_n_bottom: public Frame_cursor int res; while (!(res= cursor.get_next())) { - if (bound_tracker.check_if_next_group()) - { - end_of_partition= true; - break; - } if (order_direction * range_expr->cmp_read_only() < 0) break; item->add(); @@ -980,7 +1051,8 @@ class Frame_range_n_bottom: public Frame_cursor class Frame_range_current_row_bottom: public Frame_cursor { - Table_read_cursor cursor; + Partition_read_cursor cursor; + Group_bound_tracker peer_tracker; bool dont_move; @@ -989,7 +1061,7 @@ class Frame_range_current_row_bottom: public Frame_cursor SQL_I_List *partition_list, SQL_I_List *order_list) { - cursor.init(info); + cursor.init(thd, info, partition_list); peer_tracker.init(thd, order_list); } @@ -997,6 +1069,7 @@ class Frame_range_current_row_bottom: public Frame_cursor { // Save the value of the current_row peer_tracker.check_if_next_group(); + cursor.on_next_partition(rownum); if (rownum != 0) { // Add the current row now because our cursor has already seen it @@ -1159,17 +1232,19 @@ class Frame_unbounded_preceding : public Frame_cursor class Frame_unbounded_following : public Frame_cursor { - protected: - Table_read_cursor cursor; - Group_bound_tracker bound_tracker; + Partition_read_cursor cursor; public: void init(THD *thd, READ_RECORD *info, SQL_I_List *partition_list, SQL_I_List *order_list) { - cursor.init(info); - bound_tracker.init(thd, partition_list); + cursor.init(thd, info, partition_list); + } + + void pre_next_partition(longlong rownum, Item_sum* item) + { + cursor.on_next_partition(rownum); } void next_partition(longlong rownum, Item_sum* item) @@ -1180,15 +1255,11 @@ class Frame_unbounded_following : public Frame_cursor if (cursor.get_next()) return; } - /* Remember which partition we are in */ - bound_tracker.check_if_next_group(); item->add(); /* Walk to the end of the partition, updating the SUM function */ while (!cursor.get_next()) { - if (bound_tracker.check_if_next_group()) - break; item->add(); } } @@ -1202,6 +1273,9 @@ class Frame_unbounded_following : public Frame_cursor class Frame_unbounded_following_set_count : public Frame_unbounded_following { +public: + // pre_next_partition is inherited + void next_partition(longlong rownum, Item_sum* item) { ulonglong num_rows_in_partition= 0; @@ -1213,13 +1287,9 @@ class Frame_unbounded_following_set_count : public Frame_unbounded_following } num_rows_in_partition++; - /* Remember which partition we are in */ - bound_tracker.check_if_next_group(); /* Walk to the end of the partition, find how many rows there are. */ while (!cursor.get_next()) { - if (bound_tracker.check_if_next_group()) - break; num_rows_in_partition++; } @@ -1367,14 +1437,8 @@ class Frame_n_rows_following : public Frame_cursor const bool is_top_bound; const ha_rows n_rows; - Table_read_cursor cursor; + Partition_read_cursor cursor; bool at_partition_end; - - /* - This cursor reaches partition end before the main cursor has reached it. - bound_tracker is used to detect partition end. - */ - Group_bound_tracker bound_tracker; public: Frame_n_rows_following(bool is_top_bound_arg, ha_rows n_rows_arg) : is_top_bound(is_top_bound_arg), n_rows(n_rows_arg) @@ -1385,17 +1449,15 @@ class Frame_n_rows_following : public Frame_cursor void init(THD *thd, READ_RECORD *info, SQL_I_List *partition_list, SQL_I_List *order_list) { - cursor.init(info); + cursor.init(thd, info, partition_list); at_partition_end= false; - bound_tracker.init(thd, partition_list); } void pre_next_partition(longlong rownum, Item_sum* item) { at_partition_end= false; - // Fetch current partition value - bound_tracker.check_if_next_group(); + cursor.on_next_partition(rownum); if (rownum != 0) { @@ -1433,15 +1495,10 @@ class Frame_n_rows_following : public Frame_cursor { if (!cursor.get_next()) { - if (bound_tracker.check_if_next_group()) - at_partition_end= true; + if (is_top_bound) // this is frame start endpoint + item->remove(); else - { - if (is_top_bound) // this is frame start endpoint - item->remove(); - else - item->add(); - } + item->add(); } else at_partition_end= true;