Merge 'Fix deleting previous rowid when rowid is in the Set Clause' from Pedro Muniz

Closes #1888 . This PR fixes UPDATE translation by not emitting an
ephemeral plan when we are doing a `RowIdEq` search. Also, we should
delete the previous rowid when the rowid is in the set clause.

Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>

Closes #1891

core/translate/emitter.rs

@@ -1067,6 +1067,13 @@ fn emit_update_insns(
             });
         }
 
+        // If we are updating the rowid, we cannot rely on overwrite on the
+        // Insert instruction to update the cell. We need to first delete the current cell
+        // and later insert the updated record
+        if has_user_provided_rowid {
+            program.emit_insn(Insn::Delete { cursor_id });
+        }
+
         program.emit_insn(Insn::Insert {
             cursor: cursor_id,
             key_reg: rowid_set_clause_reg.unwrap_or(beg),

core/translate/optimizer/mod.rs

@@ -116,9 +116,6 @@ fn optimize_update_plan(plan: &mut UpdatePlan, schema: &Schema) -> Result<()> {
         plan.contains_constant_false_condition = true;
         return Ok(());
     }
-    if let Some(ephemeral_plan) = &mut plan.ephemeral_plan {
-        optimize_select_plan(ephemeral_plan, schema)?;
-    }
     let _ = optimize_table_access(
         schema,
         &mut plan.table_references,

core/translate/update.rs

@@ -1,7 +1,8 @@
 use std::rc::Rc;
 
 use crate::schema::{BTreeTable, Column, Type};
-use crate::translate::plan::{Operation, QueryDestination, SelectPlan};
+use crate::translate::optimizer::optimize_select_plan;
+use crate::translate::plan::{Operation, QueryDestination, Search, SelectPlan};
 use crate::vdbe::builder::CursorType;
 use crate::{
     bail_parse_error,
@@ -171,7 +172,6 @@ pub fn prepare_update_plan(
         })
         .collect::<Result<Vec<(usize, Expr)>, crate::LimboError>>()?;
 
-    let mut where_clause = vec![];
     let mut result_columns = vec![];
     if let Some(returning) = &mut body.returning {
         for rc in returning.iter_mut() {
@@ -210,7 +210,8 @@ pub fn prepare_update_plan(
         accum || columns[*idx].is_rowid_alias
     });
 
-    let (ephemeral_plan, where_clause) = if rowid_alias_used {
+    let (ephemeral_plan, mut where_clause) = if rowid_alias_used {
+        let mut where_clause = vec![];
         let internal_id = program.table_reference_counter.next();
 
         let joined_tables = vec![JoinedTable {
@@ -260,7 +261,7 @@ pub fn prepare_update_plan(
 
         let temp_cursor_id = program.alloc_cursor_id(CursorType::BTreeTable(table.clone()));
 
-        let ephemeral_plan = SelectPlan {
+        let mut ephemeral_plan = SelectPlan {
             table_references,
             result_columns: vec![ResultSetColumn {
                 expr: Expr::RowId {
@@ -285,8 +286,24 @@ pub fn prepare_update_plan(
             distinctness: super::plan::Distinctness::NonDistinct,
             values: vec![],
         };
-        (Some(ephemeral_plan), vec![])
+
+        optimize_select_plan(&mut ephemeral_plan, schema)?;
+        let table = ephemeral_plan
+            .table_references
+            .joined_tables()
+            .first()
+            .unwrap();
+        // We do not need to emit an ephemeral plan if we are not going to loop over the table values
+        if matches!(table.op, Operation::Search(Search::RowidEq { .. })) {
+            (None, vec![])
+        } else {
+            (Some(ephemeral_plan), vec![])
+        }
     } else {
+        (None, vec![])
+    };
+
+    if ephemeral_plan.is_none() {
         // Parse the WHERE clause
         parse_where(
             body.where_clause.as_ref().map(|w| *w.clone()),
@@ -294,7 +311,6 @@ pub fn prepare_update_plan(
             Some(&result_columns),
             &mut where_clause,
         )?;
-        (None, where_clause)
     };
 
     // Parse the LIMIT/OFFSET clause

testing/update.test

@@ -253,3 +253,10 @@ do_execsql_test_in_memory_any_error update_primary_key_unique_constraint_error {
     INSERT INTO t(b) VALUES (100), (200), (300);
     UPDATE t SET b = 2;
 }
+
+do_execsql_test_on_specific_db {:memory:} update-single-rowid {
+    CREATE TABLE t(x INTEGER PRIMARY KEY);
+    INSERT INTO t VALUES (1);
+    UPDATE t SET x = 2 WHERE x = 1;
+    SELECT * FROM t;
+} {2}