From b7a431b79e1953b89e88582a4c2a9593ff98b0bc Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Thu, 25 Sep 2025 16:26:08 -0400 Subject: [PATCH 1/3] Fix ungrouped aggregate with offset clause --- core/translate/aggregation.rs | 56 +++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/core/translate/aggregation.rs b/core/translate/aggregation.rs index bf6347a20..44a16dbc4 100644 --- a/core/translate/aggregation.rs +++ b/core/translate/aggregation.rs @@ -33,9 +33,6 @@ pub fn emit_ungrouped_aggregation<'a>( func: agg.func.clone(), }); } - // we now have the agg results in (agg_start_reg..agg_start_reg + aggregates.len() - 1) - // we need to call translate_expr on each result column, but replace the expr with a register copy in case any part of the - // result column expression matches a) a group by column or b) an aggregation result. for (i, agg) in plan.aggregates.iter().enumerate() { t_ctx .resolver @@ -44,19 +41,46 @@ pub fn emit_ungrouped_aggregation<'a>( } t_ctx.resolver.enable_expr_to_reg_cache(); - // This always emits a ResultRow because currently it can only be used for a single row result - // Limit is None because we early exit on limit 0 and the max rows here is 1 - emit_select_result( - program, - &t_ctx.resolver, - plan, - None, - None, - t_ctx.reg_nonagg_emit_once_flag, - t_ctx.reg_offset, - t_ctx.reg_result_cols_start.unwrap(), - t_ctx.limit_ctx, - )?; + // Handle OFFSET for ungrouped aggregates + // Since we only have one result row, either skip it (offset > 0) or emit it + if let Some(offset_reg) = t_ctx.reg_offset { + let done_label = program.allocate_label(); + + // If offset > 0, jump to end (skip the single row) + program.emit_insn(Insn::IfPos { + reg: offset_reg, + target_pc: done_label, + decrement_by: 0, + }); + + // Offset is 0, fall through to emit the row + emit_select_result( + program, + &t_ctx.resolver, + plan, + None, + None, + t_ctx.reg_nonagg_emit_once_flag, + None, // we've already handled offset + t_ctx.reg_result_cols_start.unwrap(), + t_ctx.limit_ctx, + )?; + + program.resolve_label(done_label, program.offset()); + } else { + // No offset specified, just emit the row + emit_select_result( + program, + &t_ctx.resolver, + plan, + None, + None, + t_ctx.reg_nonagg_emit_once_flag, + t_ctx.reg_offset, + t_ctx.reg_result_cols_start.unwrap(), + t_ctx.limit_ctx, + )?; + } Ok(()) } From 05cab4bb919636145497d95848af5f1544813672 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Thu, 25 Sep 2025 16:38:21 -0400 Subject: [PATCH 2/3] Add test for ungrouped aggregate with offset --- testing/offset.test | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/testing/offset.test b/testing/offset.test index a88dd8967..720fbebac 100644 --- a/testing/offset.test +++ b/testing/offset.test @@ -58,3 +58,9 @@ do_execsql_test_on_specific_db {:memory:} select-limit-comma-offset-equivalence 4 5} +# https://github.com/tursodatabase/turso/issues/3300 +do_execsql_test_on_specific_db {:memory:} select-ungrouped-aggregate-with-offset-limit { + CREATE TABLE t(a INTEGER); + INSERT INTO t VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10); + SELECT COUNT(a) FROM t LIMIT 1 OFFSET 1; +} {} From 429a9e4bf08abd7e96948a683eeec99348208084 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Thu, 25 Sep 2025 16:47:19 -0400 Subject: [PATCH 3/3] Put comment back --- core/translate/aggregation.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/translate/aggregation.rs b/core/translate/aggregation.rs index 44a16dbc4..9a3a98ce3 100644 --- a/core/translate/aggregation.rs +++ b/core/translate/aggregation.rs @@ -26,6 +26,7 @@ pub fn emit_ungrouped_aggregation<'a>( plan: &'a SelectPlan, ) -> Result<()> { let agg_start_reg = t_ctx.reg_agg_start.unwrap(); + for (i, agg) in plan.aggregates.iter().enumerate() { let agg_result_reg = agg_start_reg + i; program.emit_insn(Insn::AggFinal { @@ -33,6 +34,9 @@ pub fn emit_ungrouped_aggregation<'a>( func: agg.func.clone(), }); } + // we now have the agg results in (agg_start_reg..agg_start_reg + aggregates.len() - 1) + // we need to call translate_expr on each result column, but replace the expr with a register copy in case any part of the + // result column expression matches a) a group by column or b) an aggregation result. for (i, agg) in plan.aggregates.iter().enumerate() { t_ctx .resolver