From 029e5edddedefd2e6730660e10edcd3f8a2ee619 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Wed, 23 Apr 2025 13:00:01 +0300 Subject: [PATCH] Fix existing resolve_label() calls to work with new system --- core/translate/emitter.rs | 16 +++++------- core/translate/group_by.rs | 12 ++++----- core/translate/index.rs | 11 ++++---- core/translate/insert.rs | 40 ++++++++++++++++++++--------- core/translate/main_loop.rs | 48 ++++++++++++++++++++++------------- core/translate/order_by.rs | 5 ++-- core/translate/pragma.rs | 4 +-- core/translate/schema.rs | 18 ++++++------- core/translate/subquery.rs | 5 ++-- core/translate/transaction.rs | 4 +-- 10 files changed, 94 insertions(+), 69 deletions(-) diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 5b12e4375..86283fa64 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -159,8 +159,7 @@ fn epilogue( err_code: 0, description: String::new(), }); - - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); match txn_mode { TransactionMode::Read => program.emit_insn(Insn::Transaction { write: false }), @@ -297,7 +296,7 @@ pub fn emit_query<'a>( condition_metadata, &t_ctx.resolver, )?; - program.resolve_label(jump_target_when_true, program.offset()); + program.preassign_label_to_next_insn(jump_target_when_true); } // Set up main query execution loop @@ -308,8 +307,7 @@ pub fn emit_query<'a>( // Clean up and close the main execution loop close_loop(program, t_ctx, &plan.table_references)?; - - program.resolve_label(after_main_loop_label, program.offset()); + program.preassign_label_to_next_insn(after_main_loop_label); let mut order_by_necessary = plan.order_by.is_some() && !plan.contains_constant_false_condition; let order_by = plan.order_by.as_ref(); @@ -379,8 +377,7 @@ fn emit_program_for_delete( // Clean up and close the main execution loop close_loop(program, &mut t_ctx, &plan.table_references)?; - - program.resolve_label(after_main_loop_label, program.offset()); + program.preassign_label_to_next_insn(after_main_loop_label); // Finalize program epilogue(program, init_label, start_offset, TransactionMode::Write)?; @@ -516,8 +513,7 @@ fn emit_program_for_update( )?; emit_update_insns(&plan, &t_ctx, program)?; close_loop(program, &mut t_ctx, &plan.table_references)?; - - program.resolve_label(after_main_loop_label, program.offset()); + program.preassign_label_to_next_insn(after_main_loop_label); // Finalize program epilogue(program, init_label, start_offset, TransactionMode::Write)?; @@ -570,7 +566,7 @@ fn emit_update_insns( meta, &t_ctx.resolver, )?; - program.resolve_label(jump_target, program.offset()); + program.preassign_label_to_next_insn(jump_target); } let beg = program.alloc_registers( table_ref.table.columns().len() diff --git a/core/translate/group_by.rs b/core/translate/group_by.rs index 68f732cbb..6eeee3fa3 100644 --- a/core/translate/group_by.rs +++ b/core/translate/group_by.rs @@ -275,16 +275,18 @@ pub fn emit_group_by<'a>( "start new group if comparison is not equal", ); // If we are at a new group, continue. If we are at the same group, jump to the aggregation step (i.e. accumulate more values into the aggregations) + let label_jump_after_comparison = program.allocate_label(); program.emit_insn(Insn::Jump { - target_pc_lt: program.offset().add(1u32), + target_pc_lt: label_jump_after_comparison, target_pc_eq: agg_step_label, - target_pc_gt: program.offset().add(1u32), + target_pc_gt: label_jump_after_comparison, }); program.add_comment( program.offset(), "check if ended group had data, and output if so", ); + program.resolve_label(label_jump_after_comparison, program.offset()); program.emit_insn(Insn::Gosub { target_pc: label_subrtn_acc_output, return_reg: reg_subrtn_acc_output_return_offset, @@ -364,8 +366,7 @@ pub fn emit_group_by<'a>( cursor_id: sort_cursor, pc_if_next: label_grouping_loop_start, }); - - program.resolve_label(label_grouping_loop_end, program.offset()); + program.preassign_label_to_next_insn(label_grouping_loop_end); program.add_comment(program.offset(), "emit row for final group"); program.emit_insn(Insn::Gosub { @@ -505,8 +506,7 @@ pub fn emit_group_by<'a>( program.emit_insn(Insn::Return { return_reg: reg_subrtn_acc_clear_return_offset, }); - - program.resolve_label(label_group_by_end, program.offset()); + program.preassign_label_to_next_insn(label_group_by_end); Ok(()) } diff --git a/core/translate/index.rs b/core/translate/index.rs index 55222e40f..b01fb2921 100644 --- a/core/translate/index.rs +++ b/core/translate/index.rs @@ -149,8 +149,7 @@ pub fn translate_create_index( cursor_id: table_cursor_id, pc_if_empty: loop_end_label, }); - - program.resolve_label(loop_start_label, program.offset()); + program.preassign_label_to_next_insn(loop_start_label); // Loop start: // Collect index values into start_reg..rowid_reg @@ -185,7 +184,7 @@ pub fn translate_create_index( cursor_id: table_cursor_id, pc_if_next: loop_start_label, }); - program.resolve_label(loop_end_label, program.offset()); + program.preassign_label_to_next_insn(loop_end_label); // Open the index btree we created for writing to insert the // newly sorted index records. @@ -202,7 +201,7 @@ pub fn translate_create_index( cursor_id: sorter_cursor_id, pc_if_empty: sorted_loop_end, }); - program.resolve_label(sorted_loop_start, program.offset()); + program.preassign_label_to_next_insn(sorted_loop_start); let sorted_record_reg = program.alloc_register(); program.emit_insn(Insn::SorterData { pseudo_cursor: pseudo_cursor_id, @@ -226,7 +225,7 @@ pub fn translate_create_index( cursor_id: sorter_cursor_id, pc_if_next: sorted_loop_start, }); - program.resolve_label(sorted_loop_end, program.offset()); + program.preassign_label_to_next_insn(sorted_loop_end); // End of the outer loop // @@ -248,7 +247,7 @@ pub fn translate_create_index( // Epilogue: program.emit_halt(); - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); program.emit_transaction(true); program.emit_constant_insns(); program.emit_goto(start_offset); diff --git a/core/translate/insert.rs b/core/translate/insert.rs index 4ca7e6fca..78b338160 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -13,16 +13,15 @@ use crate::vdbe::insn::{IdxInsertFlags, RegisterOrLiteral}; use crate::vdbe::BranchOffset; use crate::{ schema::{Column, Schema}, - translate::expr::translate_expr, vdbe::{ builder::{CursorType, ProgramBuilder}, insn::Insn, }, - SymbolTable, }; -use crate::{Result, VirtualTable}; +use crate::{Result, SymbolTable, VirtualTable}; use super::emitter::Resolver; +use super::expr::{translate_expr_no_constant_opt, NoConstantOptReason}; #[allow(clippy::too_many_arguments)] pub fn translate_insert( @@ -144,12 +143,15 @@ pub fn translate_insert( if inserting_multiple_rows { let yield_reg = program.alloc_register(); let jump_on_definition_label = program.allocate_label(); + let start_offset_label = program.allocate_label(); program.emit_insn(Insn::InitCoroutine { yield_reg, jump_on_definition: jump_on_definition_label, - start_offset: program.offset().add(1u32), + start_offset: start_offset_label, }); + program.resolve_label(start_offset_label, program.offset()); + for value in values { populate_column_registers( &mut program, @@ -166,7 +168,7 @@ pub fn translate_insert( }); } program.emit_insn(Insn::EndCoroutine { yield_reg }); - program.resolve_label(jump_on_definition_label, program.offset()); + program.preassign_label_to_next_insn(jump_on_definition_label); program.emit_insn(Insn::OpenWrite { cursor_id, @@ -268,8 +270,7 @@ pub fn translate_insert( err_code: SQLITE_CONSTRAINT_PRIMARYKEY, description: format!("{}.{}", table_name.0, rowid_column_name), }); - - program.resolve_label(make_record_label, program.offset()); + program.preassign_label_to_next_insn(make_record_label); } match table.btree() { @@ -354,8 +355,8 @@ pub fn translate_insert( err_code: 0, description: String::new(), }); + program.preassign_label_to_next_insn(init_label); - program.resolve_label(init_label, program.offset()); program.emit_insn(Insn::Transaction { write: true }); program.emit_constant_insns(); program.emit_insn(Insn::Goto { @@ -557,18 +558,26 @@ fn populate_column_registers( } else { target_reg }; - translate_expr( + translate_expr_no_constant_opt( program, None, value.get(value_index).expect("value index out of bounds"), reg, resolver, + NoConstantOptReason::RegisterReuse, )?; if write_directly_to_rowid_reg { program.emit_insn(Insn::SoftNull { reg: target_reg }); } } else if let Some(default_expr) = mapping.default_value { - translate_expr(program, None, default_expr, target_reg, resolver)?; + translate_expr_no_constant_opt( + program, + None, + default_expr, + target_reg, + resolver, + NoConstantOptReason::RegisterReuse, + )?; } else { // Column was not specified as has no DEFAULT - use NULL if it is nullable, otherwise error // Rowid alias columns can be NULL because we will autogenerate a rowid in that case. @@ -618,7 +627,14 @@ fn translate_virtual_table_insert( let value_registers_start = program.alloc_registers(values[0].len()); for (i, expr) in values[0].iter().enumerate() { - translate_expr(program, None, expr, value_registers_start + i, resolver)?; + translate_expr_no_constant_opt( + program, + None, + expr, + value_registers_start + i, + resolver, + NoConstantOptReason::RegisterReuse, + )?; } /* * * Inserts for virtual tables are done in a single step. @@ -672,12 +688,12 @@ fn translate_virtual_table_insert( }); let halt_label = program.allocate_label(); + program.resolve_label(halt_label, program.offset()); program.emit_insn(Insn::Halt { err_code: 0, description: String::new(), }); - program.resolve_label(halt_label, program.offset()); program.resolve_label(init_label, program.offset()); program.emit_insn(Insn::Goto { diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index ff60bb305..a1cabc511 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -18,7 +18,10 @@ use crate::{ use super::{ aggregation::translate_aggregation_step, emitter::{OperationMode, TranslateCtx}, - expr::{translate_condition_expr, translate_expr, ConditionMetadata}, + expr::{ + translate_condition_expr, translate_expr, translate_expr_no_constant_opt, + ConditionMetadata, NoConstantOptReason, + }, group_by::is_column_in_group_by, optimizer::Optimizable, order_by::{order_by_sorter_insert, sorter_insert}, @@ -238,7 +241,7 @@ pub fn open_loop( jump_on_definition: BranchOffset::Offset(0), start_offset: coroutine_implementation_start, }); - program.resolve_label(loop_start, program.offset()); + program.preassign_label_to_next_insn(loop_start); // A subquery within the main loop of a parent query has no cursor, so instead of advancing the cursor, // it emits a Yield which jumps back to the main loop of the subquery itself to retrieve the next row. // When the subquery coroutine completes, this instruction jumps to the label at the top of the termination_label_stack, @@ -265,7 +268,7 @@ pub fn open_loop( condition_metadata, &t_ctx.resolver, )?; - program.resolve_label(jump_target_when_true, program.offset()); + program.preassign_label_to_next_insn(jump_target_when_true); } } Operation::Scan { iter_dir, .. } => { @@ -284,6 +287,7 @@ pub fn open_loop( pc_if_empty: loop_end, }); } + program.preassign_label_to_next_insn(loop_start); } else if let Some(vtab) = table.virtual_table() { let (start_reg, count, maybe_idx_str, maybe_idx_int) = if vtab .kind @@ -391,8 +395,8 @@ pub fn open_loop( idx_num: maybe_idx_int.unwrap_or(0) as usize, pc_if_empty: loop_end, }); + program.preassign_label_to_next_insn(loop_start); } - program.resolve_label(loop_start, program.offset()); if let Some(table_cursor_id) = table_cursor_id { if let Some(index_cursor_id) = index_cursor_id { @@ -419,7 +423,7 @@ pub fn open_loop( condition_metadata, &t_ctx.resolver, )?; - program.resolve_label(jump_target_when_true, program.offset()); + program.preassign_label_to_next_insn(jump_target_when_true); } } Operation::Search(search) => { @@ -527,7 +531,7 @@ pub fn open_loop( condition_metadata, &t_ctx.resolver, )?; - program.resolve_label(jump_target_when_true, program.offset()); + program.preassign_label_to_next_insn(jump_target_when_true); } } } @@ -815,6 +819,7 @@ pub fn close_loop( program.emit_insn(Insn::Goto { target_pc: loop_labels.loop_start, }); + program.preassign_label_to_next_insn(loop_labels.loop_end); } Operation::Scan { iter_dir, .. } => { program.resolve_label(loop_labels.next, program.offset()); @@ -844,6 +849,7 @@ pub fn close_loop( } other => unreachable!("Unsupported table reference type: {:?}", other), } + program.preassign_label_to_next_insn(loop_labels.loop_end); } Operation::Search(search) => { program.resolve_label(loop_labels.next, program.offset()); @@ -869,11 +875,10 @@ pub fn close_loop( }); } } + program.preassign_label_to_next_insn(loop_labels.loop_end); } } - program.resolve_label(loop_labels.loop_end, program.offset()); - // Handle OUTER JOIN logic. The reason this comes after the "loop end" mark is that we may need to still jump back // and emit a row with NULLs for the right table, and then jump back to the next row of the left table. if let Some(join_info) = table.join_info.as_ref() { @@ -913,7 +918,7 @@ pub fn close_loop( program.emit_insn(Insn::Goto { target_pc: lj_meta.label_match_flag_set_true, }); - program.resolve_label(label_when_right_table_notnull, program.offset()); + program.preassign_label_to_next_insn(label_when_right_table_notnull); } } } @@ -972,7 +977,14 @@ fn emit_seek( } } else { let expr = &seek_def.key[i].0; - translate_expr(program, Some(tables), &expr, reg, &t_ctx.resolver)?; + translate_expr_no_constant_opt( + program, + Some(tables), + &expr, + reg, + &t_ctx.resolver, + NoConstantOptReason::RegisterReuse, + )?; // If the seek key column is not verifiably non-NULL, we need check whether it is NULL, // and if so, jump to the loop end. // This is to avoid returning rows for e.g. SELECT * FROM t WHERE t.x > NULL, @@ -1046,7 +1058,7 @@ fn emit_seek_termination( is_index: bool, ) -> Result<()> { let Some(termination) = seek_def.termination.as_ref() else { - program.resolve_label(loop_start, program.offset()); + program.preassign_label_to_next_insn(loop_start); return Ok(()); }; @@ -1081,16 +1093,17 @@ fn emit_seek_termination( // if the seek key is shorter than the termination key, we need to translate the remaining suffix of the termination key. // if not, we just reuse what was emitted for the seek. } else if seek_len < termination.len { - translate_expr( + translate_expr_no_constant_opt( program, Some(tables), &seek_def.key[i].0, reg, &t_ctx.resolver, + NoConstantOptReason::RegisterReuse, )?; } } - program.resolve_label(loop_start, program.offset()); + program.preassign_label_to_next_insn(loop_start); let mut rowid_reg = None; if !is_index { rowid_reg = Some(program.alloc_register()); @@ -1177,11 +1190,12 @@ fn emit_autoindex( cursor_id: index_cursor_id, }); // Rewind source table + let label_ephemeral_build_loop_start = program.allocate_label(); program.emit_insn(Insn::Rewind { cursor_id: table_cursor_id, - pc_if_empty: label_ephemeral_build_end, + pc_if_empty: label_ephemeral_build_loop_start, }); - let offset_ephemeral_build_loop_start = program.offset(); + program.preassign_label_to_next_insn(label_ephemeral_build_loop_start); // Emit all columns from source table that are needed in the ephemeral index. // Also reserve a register for the rowid if the source table has rowids. let num_regs_to_reserve = index.columns.len() + table_has_rowid as usize; @@ -1215,8 +1229,8 @@ fn emit_autoindex( }); program.emit_insn(Insn::Next { cursor_id: table_cursor_id, - pc_if_next: offset_ephemeral_build_loop_start, + pc_if_next: label_ephemeral_build_loop_start, }); - program.resolve_label(label_ephemeral_build_end, program.offset()); + program.preassign_label_to_next_insn(label_ephemeral_build_end); Ok(index_cursor_id) } diff --git a/core/translate/order_by.rs b/core/translate/order_by.rs index 9793afdc9..ce9cbc4ac 100644 --- a/core/translate/order_by.rs +++ b/core/translate/order_by.rs @@ -124,8 +124,8 @@ pub fn emit_order_by( cursor_id: sort_cursor, pc_if_empty: sort_loop_end_label, }); + program.preassign_label_to_next_insn(sort_loop_start_label); - program.resolve_label(sort_loop_start_label, program.offset()); emit_offset(program, t_ctx, plan, sort_loop_next_label)?; program.emit_insn(Insn::SorterData { @@ -154,8 +154,7 @@ pub fn emit_order_by( cursor_id: sort_cursor, pc_if_next: sort_loop_start_label, }); - - program.resolve_label(sort_loop_end_label, program.offset()); + program.preassign_label_to_next_insn(sort_loop_end_label); Ok(()) } diff --git a/core/translate/pragma.rs b/core/translate/pragma.rs index 668c1f214..98d21c834 100644 --- a/core/translate/pragma.rs +++ b/core/translate/pragma.rs @@ -29,7 +29,7 @@ fn list_pragmas( } program.emit_halt(); - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); program.emit_constant_insns(); program.emit_goto(start_offset); } @@ -104,7 +104,7 @@ pub fn translate_pragma( }, }; program.emit_halt(); - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); program.emit_transaction(write); program.emit_constant_insns(); program.emit_goto(start_offset); diff --git a/core/translate/schema.rs b/core/translate/schema.rs index 49e73cf1b..760de4e41 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -37,7 +37,7 @@ pub fn translate_create_table( let init_label = program.emit_init(); let start_offset = program.offset(); program.emit_halt(); - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); program.emit_transaction(true); program.emit_constant_insns(); program.emit_goto(start_offset); @@ -148,7 +148,7 @@ pub fn translate_create_table( // TODO: SqlExec program.emit_halt(); - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); program.emit_transaction(true); program.emit_constant_insns(); program.emit_goto(start_offset); @@ -448,7 +448,7 @@ pub fn translate_create_virtual_table( }); let init_label = program.emit_init(); program.emit_halt(); - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); program.emit_transaction(true); program.emit_constant_insns(); return Ok(program); @@ -519,7 +519,7 @@ pub fn translate_create_virtual_table( }); program.emit_halt(); - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); program.emit_transaction(true); program.emit_constant_insns(); program.emit_goto(start_offset); @@ -545,7 +545,7 @@ pub fn translate_drop_table( let init_label = program.emit_init(); let start_offset = program.offset(); program.emit_halt(); - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); program.emit_transaction(true); program.emit_constant_insns(); program.emit_goto(start_offset); @@ -583,14 +583,14 @@ pub fn translate_drop_table( // 1. Remove all entries from the schema table related to the table we are dropping, except for triggers // loop to beginning of schema table let end_metadata_label = program.allocate_label(); + let metadata_loop = program.allocate_label(); program.emit_insn(Insn::Rewind { cursor_id: sqlite_schema_cursor_id, pc_if_empty: end_metadata_label, }); + program.preassign_label_to_next_insn(metadata_loop); // start loop on schema table - let metadata_loop = program.allocate_label(); - program.resolve_label(metadata_loop, program.offset()); program.emit_insn(Insn::Column { cursor_id: sqlite_schema_cursor_id, column: 2, @@ -627,7 +627,7 @@ pub fn translate_drop_table( cursor_id: sqlite_schema_cursor_id, pc_if_next: metadata_loop, }); - program.resolve_label(end_metadata_label, program.offset()); + program.preassign_label_to_next_insn(end_metadata_label); // end of loop on schema table // 2. Destroy the indices within a loop @@ -696,7 +696,7 @@ pub fn translate_drop_table( // end of the program program.emit_halt(); - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); program.emit_transaction(true); program.emit_constant_insns(); diff --git a/core/translate/subquery.rs b/core/translate/subquery.rs index 71cb72348..17434edc2 100644 --- a/core/translate/subquery.rs +++ b/core/translate/subquery.rs @@ -52,7 +52,7 @@ pub fn emit_subquery<'a>( t_ctx: &mut TranslateCtx<'a>, ) -> Result { let yield_reg = program.alloc_register(); - let coroutine_implementation_start_offset = program.offset().add(1u32); + let coroutine_implementation_start_offset = program.allocate_label(); match &mut plan.query_type { SelectQueryType::Subquery { yield_reg: y, @@ -91,6 +91,7 @@ pub fn emit_subquery<'a>( jump_on_definition: subquery_body_end_label, start_offset: coroutine_implementation_start_offset, }); + program.preassign_label_to_next_insn(coroutine_implementation_start_offset); // Normally we mark each LIMIT value as a constant insn that is emitted only once, but in the case of a subquery, // we need to initialize it every time the subquery is run; otherwise subsequent runs of the subquery will already // have the LIMIT counter at 0, and will never return rows. @@ -103,6 +104,6 @@ pub fn emit_subquery<'a>( let result_column_start_reg = emit_query(program, plan, &mut metadata)?; program.resolve_label(end_coroutine_label, program.offset()); program.emit_insn(Insn::EndCoroutine { yield_reg }); - program.resolve_label(subquery_body_end_label, program.offset()); + program.preassign_label_to_next_insn(subquery_body_end_label); Ok(result_column_start_reg) } diff --git a/core/translate/transaction.rs b/core/translate/transaction.rs index 60e00e73b..11c0a8a10 100644 --- a/core/translate/transaction.rs +++ b/core/translate/transaction.rs @@ -33,7 +33,7 @@ pub fn translate_tx_begin( } } program.emit_halt(); - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); program.emit_goto(start_offset); Ok(program) } @@ -52,7 +52,7 @@ pub fn translate_tx_commit(_tx_name: Option) -> Result { rollback: false, }); program.emit_halt(); - program.resolve_label(init_label, program.offset()); + program.preassign_label_to_next_insn(init_label); program.emit_goto(start_offset); Ok(program) }