diff --git a/mysql-test/main/func_gconcat.result b/mysql-test/main/func_gconcat.result index 0e6c5295f3b67..a1eabfec0e5f1 100644 --- a/mysql-test/main/func_gconcat.result +++ b/mysql-test/main/func_gconcat.result @@ -863,7 +863,7 @@ group_concat(distinct a, c) 00,01,10,11,31 select group_concat(distinct a, c order by a) from t1; group_concat(distinct a, c order by a) -00,01,11,10,31 +01,00,11,10,31 select group_concat(distinct a, c) from t1; group_concat(distinct a, c) 00,01,10,11,31 diff --git a/mysql-test/main/gconcat_distinct_spill.result b/mysql-test/main/gconcat_distinct_spill.result new file mode 100644 index 0000000000000..9408e12204c03 --- /dev/null +++ b/mysql-test/main/gconcat_distinct_spill.result @@ -0,0 +1,105 @@ +# +# Each block records the answer computed with memory to spare, then +# recomputes it with the duplicate filter starved, and compares. +# +CREATE TABLE t1 (pk INT AUTO_INCREMENT PRIMARY KEY, a VARCHAR(100) NOT NULL); +INSERT INTO t1 (a) SELECT LPAD(seq, 4, '0') FROM seq_1_to_50; +INSERT INTO t1 (a) SELECT a FROM t1 ORDER BY pk; +SELECT COUNT(*) AS rows_in_table, COUNT(DISTINCT a) AS distinct_values FROM t1; +rows_in_table distinct_values +100 50 +SELECT GROUP_CONCAT(DISTINCT a) INTO @gc FROM t1; +SELECT GROUP_CONCAT(DISTINCT a ORDER BY a) INTO @gc_order FROM t1; +SELECT JSON_ARRAYAGG(DISTINCT a) INTO @ja FROM t1; +SELECT JSON_ARRAYAGG(DISTINCT a ORDER BY a) INTO @ja_order FROM t1; +SET @@tmp_memory_table_size=0; +SELECT GROUP_CONCAT(DISTINCT a) = @gc AS gc_unchanged FROM t1; +gc_unchanged +1 +SELECT GROUP_CONCAT(DISTINCT a ORDER BY a) = @gc_order AS gc_order_unchanged FROM t1; +gc_order_unchanged +1 +SELECT JSON_ARRAYAGG(DISTINCT a) = @ja AS ja_unchanged FROM t1; +ja_unchanged +1 +SELECT JSON_ARRAYAGG(DISTINCT a ORDER BY a) = @ja_order AS ja_order_unchanged FROM t1; +ja_order_unchanged +1 +SET @@tmp_memory_table_size=DEFAULT; +DROP TABLE t1; +# +# Values wide enough that the filter flushes on nearly every row. +# Here the ORDER BY case used to return a single value out of 30. +# +CREATE TABLE t2 (a VARCHAR(2000)) AS +SELECT CONCAT(seq, REPEAT('.', 1990)) AS a FROM seq_1_to_30; +SELECT COUNT(*) AS rows_in_table, COUNT(DISTINCT a) AS distinct_values FROM t2; +rows_in_table distinct_values +30 30 +SELECT GROUP_CONCAT(DISTINCT a) INTO @gc FROM t2; +SELECT GROUP_CONCAT(DISTINCT a ORDER BY a) INTO @gc_order FROM t2; +SET @@tmp_memory_table_size=1000, @@max_heap_table_size=1000; +Warnings: +Warning 1292 Truncated incorrect tmp_memory_table_size value: '1000' +Warning 1292 Truncated incorrect max_heap_table_size value: '1000' +SELECT GROUP_CONCAT(DISTINCT a) = @gc AS gc_unchanged FROM t2; +gc_unchanged +1 +SELECT GROUP_CONCAT(DISTINCT a ORDER BY a) = @gc_order AS gc_order_unchanged FROM t2; +gc_order_unchanged +1 +SET @@tmp_memory_table_size=DEFAULT, @@max_heap_table_size=DEFAULT; +DROP TABLE t2; +# +# ORDER BY does not order the rows that tie on the ordering +# expression. Which of them comes first is not specified, but it +# must not depend on the memory available or on the order the rows +# are read in. +# +CREATE TABLE t3 (a BIT(2), b VARCHAR(10), c BIT); +INSERT INTO t3 VALUES (1, 'a', 0), (0, 'b', 1), (0, 'c', 0), (3, 'd', 1), +(1, 'e', 1), (3, 'f', 1), (0, 'g', 1); +SELECT GROUP_CONCAT(DISTINCT a, c ORDER BY a) AS at_default FROM t3; +at_default +01,00,11,10,31 +SET @@tmp_memory_table_size=0; +SELECT GROUP_CONCAT(DISTINCT a, c ORDER BY a) AS at_zero FROM t3; +at_zero +01,00,11,10,31 +SET @@tmp_memory_table_size=DEFAULT; +DELETE FROM t3; +INSERT INTO t3 VALUES (0, 'c', 0), (0, 'b', 1), (1, 'a', 0), (3, 'd', 1), +(1, 'e', 1), (3, 'f', 1), (0, 'g', 1); +SELECT GROUP_CONCAT(DISTINCT a, c ORDER BY a) AS reversed_scan_order FROM t3; +reversed_scan_order +01,00,11,10,31 +DROP TABLE t3; +# +# The sort tree can overflow too. Starving it makes repack_tree() +# cut rows out of the group, which is expected, but the rows that +# do come back must still be deduplicated and still be in order. +# +CREATE TABLE t4 (pk INT AUTO_INCREMENT PRIMARY KEY, a VARCHAR(20)); +INSERT INTO t4 (a) SELECT LPAD(seq, 6, '0') FROM seq_1_to_200; +INSERT INTO t4 (a) SELECT a FROM t4 ORDER BY pk; +SET @@tmp_memory_table_size=0, @@group_concat_max_len=4000; +SELECT GROUP_CONCAT(DISTINCT a ORDER BY a) INTO @gc FROM t4; +Warnings: +Warning 1260 Row 32 was cut by group_concat() +SELECT JSON_ARRAYAGG(DISTINCT a ORDER BY a) INTO @ja FROM t4; +SET @@tmp_memory_table_size=DEFAULT, @@group_concat_max_len=DEFAULT; +SELECT COUNT(*) = COUNT(DISTINCT val) AS gc_no_duplicates, +GROUP_CONCAT(val ORDER BY val) = @gc AS gc_ascending +FROM (SELECT SUBSTRING_INDEX(SUBSTRING_INDEX(@gc, ',', seq), ',', -1) AS val +FROM seq_1_to_500 +WHERE seq <= 1 + LENGTH(@gc) - LENGTH(REPLACE(@gc, ',', ''))) split; +gc_no_duplicates gc_ascending +1 1 +SELECT JSON_VALID(@ja) AS ja_valid, +COUNT(*) = COUNT(DISTINCT val) AS ja_no_duplicates, +JSON_ARRAYAGG(val ORDER BY val) = @ja AS ja_ascending +FROM (SELECT JSON_UNQUOTE(JSON_EXTRACT(@ja, CONCAT('$[', seq - 1, ']'))) AS val +FROM seq_1_to_500 WHERE seq <= JSON_LENGTH(@ja)) split; +ja_valid ja_no_duplicates ja_ascending +1 1 1 +DROP TABLE t4; diff --git a/mysql-test/main/gconcat_distinct_spill.test b/mysql-test/main/gconcat_distinct_spill.test new file mode 100644 index 0000000000000..ba7b1f59a94c3 --- /dev/null +++ b/mysql-test/main/gconcat_distinct_spill.test @@ -0,0 +1,93 @@ +# +# GROUP_CONCAT(DISTINCT ...) and JSON_ARRAYAGG(DISTINCT ...) filter +# duplicates with a Unique object, which flushes to disk when it runs out +# of memory. The answer must not depend on whether that flush happened. +# +--source include/have_sequence.inc + +--echo # +--echo # Each block records the answer computed with memory to spare, then +--echo # recomputes it with the duplicate filter starved, and compares. +--echo # + +CREATE TABLE t1 (pk INT AUTO_INCREMENT PRIMARY KEY, a VARCHAR(100) NOT NULL); +INSERT INTO t1 (a) SELECT LPAD(seq, 4, '0') FROM seq_1_to_50; +INSERT INTO t1 (a) SELECT a FROM t1 ORDER BY pk; +SELECT COUNT(*) AS rows_in_table, COUNT(DISTINCT a) AS distinct_values FROM t1; + +SELECT GROUP_CONCAT(DISTINCT a) INTO @gc FROM t1; +SELECT GROUP_CONCAT(DISTINCT a ORDER BY a) INTO @gc_order FROM t1; +SELECT JSON_ARRAYAGG(DISTINCT a) INTO @ja FROM t1; +SELECT JSON_ARRAYAGG(DISTINCT a ORDER BY a) INTO @ja_order FROM t1; + +SET @@tmp_memory_table_size=0; +SELECT GROUP_CONCAT(DISTINCT a) = @gc AS gc_unchanged FROM t1; +SELECT GROUP_CONCAT(DISTINCT a ORDER BY a) = @gc_order AS gc_order_unchanged FROM t1; +SELECT JSON_ARRAYAGG(DISTINCT a) = @ja AS ja_unchanged FROM t1; +SELECT JSON_ARRAYAGG(DISTINCT a ORDER BY a) = @ja_order AS ja_order_unchanged FROM t1; +SET @@tmp_memory_table_size=DEFAULT; +DROP TABLE t1; + +--echo # +--echo # Values wide enough that the filter flushes on nearly every row. +--echo # Here the ORDER BY case used to return a single value out of 30. +--echo # +CREATE TABLE t2 (a VARCHAR(2000)) AS + SELECT CONCAT(seq, REPEAT('.', 1990)) AS a FROM seq_1_to_30; +SELECT COUNT(*) AS rows_in_table, COUNT(DISTINCT a) AS distinct_values FROM t2; + +SELECT GROUP_CONCAT(DISTINCT a) INTO @gc FROM t2; +SELECT GROUP_CONCAT(DISTINCT a ORDER BY a) INTO @gc_order FROM t2; + +SET @@tmp_memory_table_size=1000, @@max_heap_table_size=1000; +SELECT GROUP_CONCAT(DISTINCT a) = @gc AS gc_unchanged FROM t2; +SELECT GROUP_CONCAT(DISTINCT a ORDER BY a) = @gc_order AS gc_order_unchanged FROM t2; +SET @@tmp_memory_table_size=DEFAULT, @@max_heap_table_size=DEFAULT; +DROP TABLE t2; + +--echo # +--echo # ORDER BY does not order the rows that tie on the ordering +--echo # expression. Which of them comes first is not specified, but it +--echo # must not depend on the memory available or on the order the rows +--echo # are read in. +--echo # +CREATE TABLE t3 (a BIT(2), b VARCHAR(10), c BIT); +INSERT INTO t3 VALUES (1, 'a', 0), (0, 'b', 1), (0, 'c', 0), (3, 'd', 1), +(1, 'e', 1), (3, 'f', 1), (0, 'g', 1); +SELECT GROUP_CONCAT(DISTINCT a, c ORDER BY a) AS at_default FROM t3; +SET @@tmp_memory_table_size=0; +SELECT GROUP_CONCAT(DISTINCT a, c ORDER BY a) AS at_zero FROM t3; +SET @@tmp_memory_table_size=DEFAULT; + +DELETE FROM t3; +INSERT INTO t3 VALUES (0, 'c', 0), (0, 'b', 1), (1, 'a', 0), (3, 'd', 1), +(1, 'e', 1), (3, 'f', 1), (0, 'g', 1); +SELECT GROUP_CONCAT(DISTINCT a, c ORDER BY a) AS reversed_scan_order FROM t3; +DROP TABLE t3; + +--echo # +--echo # The sort tree can overflow too. Starving it makes repack_tree() +--echo # cut rows out of the group, which is expected, but the rows that +--echo # do come back must still be deduplicated and still be in order. +--echo # +CREATE TABLE t4 (pk INT AUTO_INCREMENT PRIMARY KEY, a VARCHAR(20)); +INSERT INTO t4 (a) SELECT LPAD(seq, 6, '0') FROM seq_1_to_200; +INSERT INTO t4 (a) SELECT a FROM t4 ORDER BY pk; + +SET @@tmp_memory_table_size=0, @@group_concat_max_len=4000; +SELECT GROUP_CONCAT(DISTINCT a ORDER BY a) INTO @gc FROM t4; +SELECT JSON_ARRAYAGG(DISTINCT a ORDER BY a) INTO @ja FROM t4; +SET @@tmp_memory_table_size=DEFAULT, @@group_concat_max_len=DEFAULT; + +SELECT COUNT(*) = COUNT(DISTINCT val) AS gc_no_duplicates, + GROUP_CONCAT(val ORDER BY val) = @gc AS gc_ascending +FROM (SELECT SUBSTRING_INDEX(SUBSTRING_INDEX(@gc, ',', seq), ',', -1) AS val + FROM seq_1_to_500 + WHERE seq <= 1 + LENGTH(@gc) - LENGTH(REPLACE(@gc, ',', ''))) split; + +SELECT JSON_VALID(@ja) AS ja_valid, + COUNT(*) = COUNT(DISTINCT val) AS ja_no_duplicates, + JSON_ARRAYAGG(val ORDER BY val) = @ja AS ja_ascending +FROM (SELECT JSON_UNQUOTE(JSON_EXTRACT(@ja, CONCAT('$[', seq - 1, ']'))) AS val + FROM seq_1_to_500 WHERE seq <= JSON_LENGTH(@ja)) split; +DROP TABLE t4; diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 2cc115769d691..79f8e02595883 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -4296,6 +4296,60 @@ bool Item_func_group_concat::repack_tree(THD *thd) } +/* + Insert one row into the ORDER BY tree, repacking it first if it has + grown past the memory we are allowed to use. + + 'key' is a temporary table record in the format produced by + get_record_pointer(). table->field[0] of that record holds the length + the row adds to the result, which is what repack_tree() adds up to + decide where to cut. + + @return FALSE row inserted + @return TRUE out of memory +*/ + +bool Item_func_group_concat::insert_to_order_tree(uchar *key) +{ + DBUG_ASSERT(tree); + THD *thd= table->in_use; + /* + Repack when GCONCAT_TREE_REPACK_PARTS of the memory we may use is + gone. The rest is needed for the new tree that repack_tree() + allocates while the old one is still around. + */ + if (tree->allocated > + max_tree_size / GCONCAT_TREE_PARTS * GCONCAT_TREE_REPACK_PARTS && + tree->elements_in_tree > 1) + if (repack_tree(thd)) + return TRUE; + /* check if there was enough memory to insert the row */ + return !tree_insert(tree, key, 0, tree->custom_arg); +} + + +/* + Insert one deduplicated row into the ORDER BY tree. + + Callback for Unique::walk(). The walk merges what unique_filter spilled + to disk back with what it still holds in memory, so it is the first + point at which the whole set of distinct rows is known. Every row it + visits belongs in the result. + + @return 0 row inserted + @return 1 out of memory, which stops the walk +*/ + +int Item_func_group_concat::dump_leaf_key_to_tree(void *key_arg, + element_count count + __attribute__((unused)), + void *item_arg) +{ + auto item= static_cast(item_arg); + return item->insert_to_order_tree(static_cast(key_arg)); +} + + bool Item_func_group_concat::add(bool exclude_nulls) { if (always_null && exclude_nulls) @@ -4337,7 +4391,15 @@ bool Item_func_group_concat::add(bool exclude_nulls) null_value= FALSE; bool row_eligible= TRUE; - if (distinct) + /* + Store how much this row adds to the result in the record itself. With + DISTINCT the row does not reach the ORDER BY tree until val_str(), and + the record kept by unique_filter is all that is left of it by then. + */ + if (tree) + table->field[0]->store(row_str_len, FALSE); + + if (distinct) { /* Filter out duplicate rows. */ uint count= unique_filter->elements_in_tree(); @@ -4346,26 +4408,16 @@ bool Item_func_group_concat::add(bool exclude_nulls) row_eligible= FALSE; } - TREE_ELEMENT *el= 0; // Only for safety - if (row_eligible && tree) - { - THD *thd= table->in_use; - table->field[0]->store(row_str_len, FALSE); - /* - Repack when GCONCAT_TREE_REPACK_PARTS of the memory we may use is - gone. The rest is needed for the new tree that repack_tree() - allocates while the old one is still around. - */ - if (tree->allocated > - max_tree_size / GCONCAT_TREE_PARTS * GCONCAT_TREE_REPACK_PARTS && - tree->elements_in_tree > 1) - if (repack_tree(thd)) - return 1; - el= tree_insert(tree, get_record_pointer(), 0, tree->custom_arg); - /* check if there was enough memory to insert the row */ - if (!el) - return 1; - } + /* + With DISTINCT the ORDER BY tree is not filled here. row_eligible only + reflects the part of unique_filter that is currently in memory, so as + soon as unique_filter starts flushing to disk it stops telling us + whether a row is a duplicate. val_str() fills the tree instead, from + the unique_filter walk that merges everything back together. + */ + if (!distinct && row_eligible && tree && + insert_to_order_tree(get_record_pointer())) + return 1; /* In case of GROUP_CONCAT with DISTINCT or ORDER BY (or both) don't dump the @@ -4595,7 +4647,25 @@ String* Item_func_group_concat::val_str(String* str) if (!result_finalized) // Result yet to be written. { - if (tree != NULL) // order by + if (tree && distinct) // distinct and order by + { + /* + Sort the distinct rows now. add() could not do it, as a row is + only known not to be a duplicate once unique_filter has merged + everything it flushed to disk, which the walk below does. + */ + if (unique_filter->walk(table, &dump_leaf_key_to_tree, this)) + { + /* + Out of memory; mysys has reported it. The tree holds only part + of the group, so tell the user that the result was cut instead + of returning a short one silently. + */ + result_cut= TRUE; + } + tree_walk(tree, &dump_leaf_key, this, left_root_right); + } + else if (tree != NULL) // order by tree_walk(tree, &dump_leaf_key, this, left_root_right); else if (distinct) // distinct (and no order by). unique_filter->walk(table, &dump_leaf_key, this); diff --git a/sql/item_sum.h b/sql/item_sum.h index 9a60a368dd1f7..c16f9203c4cf0 100644 --- a/sql/item_sum.h +++ b/sql/item_sum.h @@ -2199,6 +2199,9 @@ class Item_func_group_concat : public Item_sum_str qsort_cmp2 get_comparator_function_for_order_by(); uchar* get_record_pointer(); uint get_null_bytes(); + bool insert_to_order_tree(uchar *key); + static int dump_leaf_key_to_tree(void *key_arg, element_count count, + void *item_arg); protected: Item *shallow_copy(THD *thd) const override