MDEV-19398: Assertion `item1->type() == Item::FIELD_ITEM ...

Window Functions code tries to minimize the number of times it
needs to sort the select's resultset by finding "compatible"
OVER (PARTITION BY ... ORDER BY ...) clauses.

This employs compare_order_elements(). That function assumed that
the order expressions are Item_field-derived objects (that refer
to a temp.table). But this is not always the case: one can
construct queries order expressions are arbitrary item expressions.

Add handling for such expressions: sort them according to the window
specification they appeared in.
This means we cannot detect that two compatible PARTITION BY clauses
that use expressions can share the sorting step.
But at least we won't crash.

Author: spetrunia

mysql-test/r/win.result

@@ -4238,5 +4238,47 @@ SELECT 1 UNION SELECT a FROM t1 ORDER BY (row_number() over ());
 ERROR HY000: Expression #1 of ORDER BY contains aggregate function and applies to a UNION
 DROP TABLE t1;
 #
+#  MDEV-19398: Assertion `item1->type() == Item::FIELD_ITEM &&
+#    item2->type() == Item::FIELD_ITEM' failed in compare_order_elements
+#
+CREATE TABLE t1 ( id varchar(10));
+INSERT INTO t1 values (1),(2),(3);
+SELECT
+dense_rank() over (ORDER BY avg(1)+3),
+rank()       over (ORDER BY avg(1))
+FROM t1
+GROUP BY nullif(id, 15532);
+dense_rank() over (ORDER BY avg(1)+3)	rank()       over (ORDER BY avg(1))
+1	1
+1	1
+1	1
+SELECT
+dense_rank() over (ORDER BY avg(1)),
+rank()       over (ORDER BY avg(1))
+FROM t1
+GROUP BY nullif(id, 15532);
+dense_rank() over (ORDER BY avg(1))	rank()       over (ORDER BY avg(1))
+1	1
+1	1
+1	1
+drop table t1;
+CREATE TABLE t1 ( a char(25), b text);
+INSERT INTO t1 VALUES ('foo','bar');
+SELECT
+SUM(b)       OVER (PARTITION BY a),
+ROW_NUMBER() OVER (PARTITION BY b)
+FROM t1
+GROUP BY
+LEFT((SYSDATE()), 'foo')
+WITH ROLLUP;
+SUM(b)       OVER (PARTITION BY a)	ROW_NUMBER() OVER (PARTITION BY b)
+NULL	1
+NULL	1
+Warnings:
+Warning	1292	Truncated incorrect INTEGER value: 'foo'
+Warning	1292	Truncated incorrect INTEGER value: 'foo'
+drop table t1;
+#
+#
 # End of 10.2 tests
 #

mysql-test/t/win.test

@@ -2740,6 +2740,39 @@ INSERT INTO t1 VALUES (1),(1),(1),(1),(1),(2),(2),(2),(2),(2),(2);
 SELECT 1 UNION SELECT a FROM t1 ORDER BY (row_number() over ());
 DROP TABLE t1;
 
+--echo #
+--echo #  MDEV-19398: Assertion `item1->type() == Item::FIELD_ITEM &&
+--echo #    item2->type() == Item::FIELD_ITEM' failed in compare_order_elements
+--echo #
+CREATE TABLE t1 ( id varchar(10));
+INSERT INTO t1 values (1),(2),(3);
+
+SELECT
+  dense_rank() over (ORDER BY avg(1)+3),
+  rank()       over (ORDER BY avg(1))
+FROM t1
+GROUP BY nullif(id, 15532);
+
+SELECT
+  dense_rank() over (ORDER BY avg(1)),
+  rank()       over (ORDER BY avg(1))
+FROM t1
+GROUP BY nullif(id, 15532);
+drop table t1;
+
+CREATE TABLE t1 ( a char(25), b text);
+INSERT INTO t1 VALUES ('foo','bar');
+
+SELECT
+  SUM(b)       OVER (PARTITION BY a),
+  ROW_NUMBER() OVER (PARTITION BY b)
+FROM t1
+GROUP BY
+  LEFT((SYSDATE()), 'foo')
+WITH ROLLUP;
+drop table t1;
+
+--echo #
 --echo #
 --echo # End of 10.2 tests
 --echo #

sql/sql_parse.cc

@@ -8624,6 +8624,7 @@ bool st_select_lex::add_window_def(THD *thd,
     fields_in_window_functions+= win_part_list_ptr->elements +
                                  win_order_list_ptr->elements;
   }
+  win_def->win_spec_number= window_specs.elements;
   return (win_def == NULL || window_specs.push_back(win_def));
 }
 
@@ -8651,6 +8652,7 @@ bool st_select_lex::add_window_spec(THD *thd,
                                  win_order_list_ptr->elements;
   }
   thd->lex->win_spec= win_spec;
+  win_spec->win_spec_number= window_specs.elements;
   return (win_spec == NULL || window_specs.push_back(win_spec));
 }
 

sql/sql_window.cc

@@ -312,15 +312,49 @@ setup_windows(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables,
 #define CMP_GT         2    // Greater then
 
 static
-int compare_order_elements(ORDER *ord1, ORDER *ord2)
+int compare_order_elements(ORDER *ord1, int weight1,
+                           ORDER *ord2, int weight2)
 {
   if (*ord1->item == *ord2->item && ord1->direction == ord2->direction)
     return CMP_EQ;
   Item *item1= (*ord1->item)->real_item();
   Item *item2= (*ord2->item)->real_item();
-  DBUG_ASSERT(item1->type() == Item::FIELD_ITEM &&
-              item2->type() == Item::FIELD_ITEM); 
-  ptrdiff_t cmp= ((Item_field *) item1)->field - ((Item_field *) item2)->field;
+
+  bool item1_field= (item1->type() == Item::FIELD_ITEM);
+  bool item2_field= (item2->type() == Item::FIELD_ITEM);
+
+  ptrdiff_t cmp;
+  if (item1_field && item2_field)
+  {
+    DBUG_ASSERT(((Item_field *) item1)->field->table ==
+                ((Item_field *) item2)->field->table);
+    cmp= ((Item_field *) item1)->field->field_index -
+         ((Item_field *) item2)->field->field_index;
+  }
+  else if (item1_field && !item2_field)
+    return CMP_LT;
+  else if (!item1_field && item2_field)
+    return CMP_LT;
+  else
+  {
+    /*
+      Ok, item1_field==NULL and item2_field==NULL.
+      We're not able to compare Item expressions. Order them according to
+      their passed "weight" (which comes from Window_spec::win_spec_number):
+    */
+    if (weight1 != weight2)
+      cmp= weight1 - weight2;
+    else
+    {
+      /*
+        The weight is the same. That is, the elements come from the same
+        window specification... This shouldn't happen.
+      */
+      DBUG_ASSERT(0);
+      cmp= item1 - item2;
+    }
+  }
+
   if (cmp == 0)
   {
     if (ord1->direction == ord2->direction)
@@ -333,7 +367,9 @@ int compare_order_elements(ORDER *ord1, ORDER *ord2)
 
 static
 int compare_order_lists(SQL_I_List<ORDER> *part_list1,
-                        SQL_I_List<ORDER> *part_list2)
+                        int spec_number1,
+                        SQL_I_List<ORDER> *part_list2,
+                        int spec_number2)
 {
   if (part_list1 == part_list2)
     return CMP_EQ;
@@ -358,7 +394,8 @@ int compare_order_lists(SQL_I_List<ORDER> *part_list1,
     if (!elem1 || !elem2)
       break;
 
-    if ((cmp= compare_order_elements(elem1, elem2)))
+    if ((cmp= compare_order_elements(elem1, spec_number1,
+                                     elem2, spec_number2)))
       return cmp;
   }
   if (elem1)
@@ -453,7 +490,9 @@ int compare_window_spec_joined_lists(Window_spec *win_spec1,
   win_spec1->join_partition_and_order_lists();
   win_spec2->join_partition_and_order_lists();
   int cmp= compare_order_lists(win_spec1->partition_list, 
-                               win_spec2->partition_list);
+                               win_spec1->win_spec_number,
+                               win_spec2->partition_list,
+                               win_spec2->win_spec_number);
   win_spec1->disjoin_partition_and_order_lists();
   win_spec2->disjoin_partition_and_order_lists();
   return cmp;
@@ -471,7 +510,9 @@ int compare_window_funcs_by_window_specs(Item_window_func *win_func1,
   if (win_spec1 == win_spec2)
     return CMP_EQ;
   cmp= compare_order_lists(win_spec1->partition_list, 
-                           win_spec2->partition_list);
+                           win_spec1->win_spec_number,
+                           win_spec2->partition_list,
+                           win_spec2->win_spec_number);
   if (cmp == CMP_EQ)
   {
     /* 
@@ -490,7 +531,9 @@ int compare_window_funcs_by_window_specs(Item_window_func *win_func1,
     }
 
     cmp= compare_order_lists(win_spec1->order_list,
-                             win_spec2->order_list);
+                             win_spec1->win_spec_number,
+                             win_spec2->order_list,
+                             win_spec2->win_spec_number);
 
     if (cmp != CMP_EQ)
       return cmp;
@@ -587,7 +630,9 @@ void order_window_funcs_by_window_specs(List<Item_window_func> *win_func_list)
       int cmp;
       if (win_spec_prev->partition_list == win_spec_curr->partition_list)
         cmp= compare_order_lists(win_spec_prev->order_list,
-                                 win_spec_curr->order_list);
+                                 win_spec_prev->win_spec_number,
+                                 win_spec_curr->order_list,
+                                 win_spec_curr->win_spec_number);
       else
         cmp= compare_window_spec_joined_lists(win_spec_prev, win_spec_curr);
       if (!(CMP_LT_C <= cmp && cmp <= CMP_GT_C))

sql/sql_window.h

@@ -108,6 +108,13 @@ class Window_spec : public Sql_alloc
 
   Window_spec *referenced_win_spec;
 
+  /*
+    Window_spec objects are numbered by the number of their appearance in the
+    query. This is used by compare_order_elements() to provide a predictable
+    ordering of PARTITION/ORDER BY clauses.
+  */
+  int win_spec_number;
+
   Window_spec(LEX_STRING *win_ref, 
               SQL_I_List<ORDER> *part_list,
               SQL_I_List<ORDER> *ord_list,