Merge 'Fix: all indexes need to be updated if the rowid changes' from Jussi Saurio

Found when running simulator in #2641
All indexes store the rowid as the last column, so whenever the rowid of
a given row changes the index entry must also be deleted and reinserted
with the new index.

Reviewed-by: Nikita Sivukhin (@sivukhin)

Closes #2712

core/translate/emitter.rs

@@ -1063,8 +1063,10 @@ fn emit_update_insns(
             dest: idx_rowid_reg,
         });
 
+        // Skip over the UNIQUE constraint failure if the existing row is the one that we are currently changing
+        let original_rowid_reg = beg;
         program.emit_insn(Insn::Eq {
-            lhs: rowid_reg,
+            lhs: original_rowid_reg,
             rhs: idx_rowid_reg,
             target_pc: constraint_check,
             flags: CmpInsFlags::default(), // TODO: not sure what type of comparison flag is needed

core/translate/update.rs

@@ -349,17 +349,26 @@ pub fn prepare_update_plan(
     // Check what indexes will need to be updated by checking set_clauses and see
     // if a column is contained in an index.
     let indexes = schema.get_indices(table_name.as_str());
-    let indexes_to_update = indexes
+    let rowid_alias_used = set_clauses
         .iter()
-        .filter(|index| {
-            index.columns.iter().any(|index_column| {
-                set_clauses
-                    .iter()
-                    .any(|(set_index_column, _)| index_column.pos_in_table == *set_index_column)
+        .any(|(idx, _)| columns[*idx].is_rowid_alias);
+    let indexes_to_update = if rowid_alias_used {
+        // If the rowid alias is used in the SET clause, we need to update all indexes
+        indexes.to_vec()
+    } else {
+        // otherwise we need to update the indexes whose columns are set in the SET clause
+        indexes
+            .iter()
+            .filter(|index| {
+                index.columns.iter().any(|index_column| {
+                    set_clauses
+                        .iter()
+                        .any(|(set_index_column, _)| index_column.pos_in_table == *set_index_column)
+                })
             })
-        })
-        .cloned()
-        .collect();
+            .cloned()
+            .collect()
+    };
 
     Ok(Plan::Update(UpdatePlan {
         table_references,

testing/update.test

@@ -366,3 +366,15 @@ do_execsql_test_on_specific_db {:memory:} row-values-repeated-values-should-take
     INSERT INTO test (id, name) VALUES (1, 'test');
     UPDATE test SET (name, name) = ('mordor', 'shire') RETURNING id, name;
 } {1|shire}
+
+do_execsql_test_on_specific_db {:memory:} rowid-update-updates-all-indexes {
+    CREATE TABLE t (a integer primary key, b unique, c unique);
+    INSERT INTO t VALUES (1,1,1);
+    UPDATE t SET a = 2, b = 3;
+    SELECT * from t;
+    -- massage optimizer into using b and c indexes respectively
+    SELECT * from t WHERE b > 0;
+    SELECT * from t WHERE c > 0;
+} {2|3|1
+2|3|1
+2|3|1}