From 64b83a45e89a7fe91f94c093d03d2d86ebe219f1 Mon Sep 17 00:00:00 2001 From: Piotr Rzysko Date: Sat, 21 Jun 2025 10:03:10 +0200 Subject: [PATCH] Fix infinite aggregation loop when sorting is not required MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, with the `index_experimental` feature enabled, the query in the added test would enter an infinite loop. This happened because `label_grouping_agg_step` pointed to a constant argument that was moved to the end of the program. As a result, the aggregation loop would jump to the constant, then return to the start of the main loop, rewind the index, and re-enter the aggregation loop—causing it to repeat indefinitely. --- core/translate/group_by.rs | 2 +- testing/groupby.test | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/core/translate/group_by.rs b/core/translate/group_by.rs index cb0b56076..0c8fd3e23 100644 --- a/core/translate/group_by.rs +++ b/core/translate/group_by.rs @@ -605,7 +605,7 @@ pub fn group_by_process_single_group<'a>( }); // Process each aggregate function for the current row - program.resolve_label(labels.label_grouping_agg_step, program.offset()); + program.preassign_label_to_next_insn(labels.label_grouping_agg_step); let cursor_index = t_ctx.non_aggregate_expressions.len(); // Skipping all columns in sorter that not an aggregation arguments let mut offset = 0; for (i, agg) in plan.aggregates.iter().enumerate() { diff --git a/testing/groupby.test b/testing/groupby.test index 1c7124ecd..3a2a05086 100755 --- a/testing/groupby.test +++ b/testing/groupby.test @@ -199,6 +199,13 @@ do_execsql_test group_by_no_sorting_required { 2|113 3|97} +# Compile-time constants are moved to the end of the program. +# Verify that the jump to AggStep works correctly even when the location of the ',' constant has changed. +do_execsql_test group_by_no_sorting_required_and_const_agg_arg { + select group_concat(state, ',') from users group by age limit 2; +} {CA,PW,ME,AS,LA,OH,AL,UT,WA,MO,WA,SC,AR,CO,OK,ME,FM,AR,CT,MT,TN,FL,MA,ND,LA,NE,KS,IN,RI,NH,IL,FM,WA,MH,RI,SC,AS,IL,VA,MI,ID,ME,WY,TN,IN,IN,UT,WA,AZ,VA,NM,IA,MP,WY,RI,OR,OR,FM,WA,DC,RI,GU,TX,HI,IL,TX,WY,OH,TX,CT,KY,NE,MH,AR,MN,IL,NH,HI,NV,UT,FL,MS,NM,NJ,CA,MS,GA,MT,GA,AL,IN,SC,PA,FL,CT,PA,GA,RI,HI,WV,VT,IA,PR,FM,MA,TX,MS,LA,MD,PA,TX,WY +OR,SD,KS,MP,WA,VI,SC,SD,SD,MP,WA,MT,FM,IN,ME,OH,KY,RI,DC,MS,OK,VI,KY,MD,SC,OK,NY,WY,AK,MN,UT,NE,VA,MD,AZ,VI,SC,NV,IN,VA,HI,VI,MS,NE,WY,NY,GU,MT,AL,IA,VA,ND,MN,FM,IA,ID,IL,FL,PR,WA,AS,HI,NH,WI,FL,HI,AL,ID,DC,CT,IL,VT,AZ,VI,AK,PW,NC,SD,NV,WA,MO,MS,WY,VA,FM,MN,NH,MN,MT,TX,MS,FM,OH,GU,IN,WA,IA,PA,ID,MI,LA,GU,ND,AR,ND,WV,DC,NY,CO,CT,FM,CT,ND} + if {[info exists ::env(SQLITE_EXEC)] && ($::env(SQLITE_EXEC) eq "scripts/limbo-sqlite3-index-experimental" || $::env(SQLITE_EXEC) eq "sqlite3")} { do_execsql_test_on_specific_db {:memory:} group_by_no_sorting_required_reordered_columns { create table t0 (a INT, b INT, c INT);