From 110ffba2a1d5f461b0c2a5ad36b6b830256f4ece Mon Sep 17 00:00:00 2001 From: Piotr Rzysko Date: Sat, 6 Sep 2025 05:36:03 +0200 Subject: [PATCH] Fix accumulator reset when arguments outnumber aggregates Previously, while resetting accumulator registers, we would also reset subsequent registers. This happened because the number of registers to reset was computed as the sum of arguments rather than the number of aggregate functions. --- core/translate/group_by.rs | 2 +- testing/groupby.test | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/core/translate/group_by.rs b/core/translate/group_by.rs index 37c05cc45..bb3f5b70b 100644 --- a/core/translate/group_by.rs +++ b/core/translate/group_by.rs @@ -762,7 +762,7 @@ pub fn group_by_emit_row_phase<'a>( program.emit_insn(Insn::Null { dest: start_reg, dest_end: Some( - start_reg + t_ctx.non_aggregate_expressions.len() + plan.agg_args_count() - 1, + start_reg + t_ctx.non_aggregate_expressions.len() + plan.aggregates.len() - 1, ), }); diff --git a/testing/groupby.test b/testing/groupby.test index dc5418110..5af77e952 100755 --- a/testing/groupby.test +++ b/testing/groupby.test @@ -308,3 +308,21 @@ shirt shorts sweater sweatshirt} + +# There was a bug where, while resetting accumulator registers, subsequent registers were also reset. +# This happened when there were more arguments than aggregate functions — the number of registers to reset +# was calculated as the sum of the arguments, not the number of aggregates. +# The issue affected cases where rows were pre-sorted, hence the 'GROUP BY id' test. +do_execsql_test more_args_than_aggregates { + SELECT group_concat(name, ','), group_concat(name, ';'), group_concat(name, '.') FROM products GROUP BY id; +} {hat|hat|hat +cap|cap|cap +shirt|shirt|shirt +sweater|sweater|sweater +sweatshirt|sweatshirt|sweatshirt +shorts|shorts|shorts +jeans|jeans|jeans +sneakers|sneakers|sneakers +boots|boots|boots +coat|coat|coat +accessories|accessories|accessories}