|
Thanks a lot!
I repeated on 10.4, results are not deterministic.
10.2, 10.3 returned the stable result (and correct)
create table t1 (a int, b int);
|
insert into t1 values (1, 1), (2, 1), (3, 2), (4, 2);
|
|
MariaDB [test]> select sql_no_cache sum(a) over (), sum(a) over (partition by b) c, sum(a) over (order by a) d from t1 order by a;
|
+----------------+------+------+
|
| sum(a) over () | c | d |
|
+----------------+------+------+
|
| 10 | 3 | 3 |
|
| 10 | 3 | 2 |
|
| 10 | 7 | 6 |
|
| 10 | 7 | 10 |
|
+----------------+------+------+
|
4 rows in set (0.000 sec)
|
|
MariaDB [test]> select sql_no_cache sum(a) over (), sum(a) over (partition by b) c, sum(a) over (order by a) d from t1 order by a;
|
+----------------+------+------+
|
| sum(a) over () | c | d |
|
+----------------+------+------+
|
| 10 | 3 | 1 |
|
| 10 | 3 | 3 |
|
| 10 | 7 | 10 |
|
| 10 | 7 | 7 |
|
+----------------+------+------+
|
4 rows in set (0.000 sec)
|
|
MariaDB [test]> select sql_no_cache sum(a) over (), sum(a) over (partition by b) c, sum(a) over (order by a) d from t1 order by a;
|
+----------------+------+------+
|
| sum(a) over () | c | d |
|
+----------------+------+------+
|
| 10 | 3 | 1 |
|
| 10 | 3 | 3 |
|
| 10 | 7 | 6 |
|
| 10 | 7 | 10 |
|
+----------------+------+------+
|
4 rows in set (0.000 sec)
|
|
|
alice I can't reproduce this now on 10.4, not on 10.4.6 too
MariaDB [test]> select sql_no_cache sum(a) over (), sum(a) over (partition by b) c, sum(a) over (order by a) d from t1 order by a;
|
+----------------+------+------+
|
| sum(a) over () | c | d |
|
+----------------+------+------+
|
| 10 | 3 | 1 |
|
| 10 | 3 | 3 |
|
| 10 | 7 | 6 |
|
| 10 | 7 | 10 |
|
+----------------+------+------+
|
4 rows in set (0.005 sec)
|
|
MariaDB [test]> select sql_no_cache sum(a) over (), sum(a) over (partition by b) c, sum(a) over (order by a) d from t1 order by a;
|
+----------------+------+------+
|
| sum(a) over () | c | d |
|
+----------------+------+------+
|
| 10 | 3 | 1 |
|
| 10 | 3 | 3 |
|
| 10 | 7 | 6 |
|
| 10 | 7 | 10 |
|
+----------------+------+------+
|
4 rows in set (0.002 sec)
|
|
|
|
OK i see this different results, here
MariaDB [test]> select version();
|
+-----------------------+
|
| version() |
|
+-----------------------+
|
| 10.2.29-MariaDB-debug |
|
+-----------------------+
|
1 row in set (0.00 sec)
|
|
MariaDB [test]> select sum(a) over (), sum(a) over (partition by b) c, sum(a) over (order by a) d from t1;
|
+----------------+------+------+
|
| sum(a) over () | c | d |
|
+----------------+------+------+
|
| 10 | 3 | 1 |
|
| 10 | 3 | 3 |
|
| 10 | 7 | 7 |
|
| 10 | 7 | 10 |
|
+----------------+------+------+
|
4 rows in set (0.01 sec)
|
|
|
MariaDB [test]> select sum(a) over (), sum(a) over (partition by b) c, sum(a) over (order by a) d from t1 order by a;
|
+----------------+------+------+
|
| sum(a) over () | c | d |
|
+----------------+------+------+
|
| 10 | 3 | 1 |
|
| 10 | 3 | 3 |
|
| 10 | 7 | 6 |
|
| 10 | 7 | 10 |
|
+----------------+------+------+
|
4 rows in set (0.00 sec)
|
|
After adding the ORDERBY clause, the results are different which makes no sense as the window function computation is done before the ORDER BY clause is applied.
|
|
Explanation
QUERY:
SELECT
|
sum(a) OVER () AS x,
|
sum(a) OVER (PARTITION BY b) AS y,
|
sum(a) OVER (ORDER BY a) AS z
|
FROM t1;
|
So when there are multiple window function, we try to find out the common sort
criteria which would help us in reducing the number of times we need to sort
the temporary table to which window functions are attached.
For the above query we need to call filesort twice:
- PARTITION BY b, this will allow calculation for both sum(a) OVER() and sum(a) OVER (PARTITION BY b)
- ORDER BY a, this will allow calculation for sum(a) OVER (ORDER BY a)
Currently we do only 1 sorting which is incorrect.
So there is a bug in the function which compares window functions.
|
|
Patch
http://lists.askmonty.org/pipermail/commits/2019-December/014094.html
|
|
Here's my analysis:
For the above test case:
In the function order_window_funcs_by_window_specs() after the call
bubble_sort<Item_window_func>(win_func_list,
|
compare_window_funcs_by_window_specs,
|
NULL);
|
we have the sequence of Item_window_func elements in win_func_list that refer to the following window spec:
() (ORDER BY a),
() ()
(PARTITION BY b) ()
This sequence is ok is it allows to minimize the number of sortings.
The problem I see in the code after the mentioned call of bubble_sort<Item_window_func>() that does not not set PARTITION_CHANGE_FLAG for the last element.
|
|
Varun has noticed these two queries
SELECT
|
SUM(a) OVER (PARTITION BY a,b,c) AS y,
|
SUM(a) OVER (PARTITION BY a,b) AS x
|
FROM t1;
|
|
SELECT
|
SUM(a) OVER (PARTITION BY a,b ORDER BY c) AS y,
|
SUM(a) OVER (PARTITION BY a,b) AS x
|
FROM t1;
|
that are supposed to have the same order of window functions in the win_func_list list after the call of bubble_sort actually produce two different orders. The expected order is: the first window function, then the second.
My analyses of this problem showed that it happened due to a bug in the code of compare_order_lists().
This change:
--- a/sql/sql_window.cc
|
+++ b/sql/sql_window.cc
|
@@ -475,9 +475,9 @@ int compare_order_lists(SQL_I_List<ORDER> *part_list1,
|
return cmp;
|
}
|
if (elem1)
|
- return CMP_GT_C;
|
- if (elem2)
|
return CMP_LT_C;
|
+ if (elem2)
|
+ return CMP_GT_C;
|
return CMP_EQ;
|
}
|
fixes the problem.
|
|
I see a problem with the function that compares two window specification compare_window_funcs_by_window_specs(). This function is used a parameter for a regular sorting procedure. It means that this function must hold the transitivity property:
ws1 < ws2 && ws2 < ws3 => ws1 < ws3.
However according to the corrected code we have:
(bcd) () < (bc) ()
(bc) < (b) (ca)
yet ! ((bcd) () < (b) (ca))
|
|
Actually we don't need to sort window function by window spec. We have to partition them in such a way that each partition contains the functions whose concatenations of partition lists and order lists are compatible for each pair of elements in the partition. For this we don't need any sorting procedure. Partitions are built one by one in such a way that each newly built partition must include as many elements as possible. The elements of already built partitions are placed at the beginning of the list. The first of the remaining elements is included in the next partition to be built.
Now: Now we ignore possible optimizations when a prefix of the order list coincides with a suffix of the partition list.
Optimization of partition usage should be done separately within each group with the same sort order. This optimization is done with sub-partitioning that is done by compatible partition lists.
|
|
Some observations:
compare_window_funcs_by_window_specs attempts to create an ordering that
groups window functions not only by the PARTITION BY/ORDER BY sorting, but
also by
- PARITION definition (with the intent to reuse the partition bound checker)
- frame definition (see compare_window_frames) (with the intent to share the
frame cursors)
sharing the sorting seems to be preferred to sharing the PARTITION definition.
The sorted sequence then has these flags for some elements:
#define SORTORDER_CHANGE_FLAG 1
|
#define PARTITION_CHANGE_FLAG 2
|
#define FRAME_CHANGE_FLAG 4
|
But the execution code only uses SORTORDER_CHANGE_FLAG.
|
|
Patch: http://lists.askmonty.org/pipermail/commits/2019-December/014112.html . igor, please review.
|
|
Igor, please review.
|
|
Igor's input from the optimizer call discussion:
- Although functions that compare the PARTITION BY clause and frame bounds are not used, they should not be removed (with the rationale that they might be useful in the future when/if we need to implement partition bound reuse or frame bound reuse)
- Leave everything in one List object with group bounds being delimited by SORTORDER_CHANGE_FLAG, PARTITION_CHANGE_FLAG and FRAME_CHANGE_FLAG (I'm not sure about the reasoning for this one).
|