Merge 'Fix UPDATE straight up not working on non-unique indexes' from Jussi Saurio

we were skipping `MakeRecord` for the new values from the `SET` clause
if the index wasn't unique, effectively emitting NULLs. Although this
would actually already fail in the `IdxInsert` instruction because the
record register didn't actually contain a record.
this has been (I think) caught in at least 1. limbo stress 2.
antithesis, but I incorrectly assumed it was something more edge-casey
instead of broken like this in a fairly basic way

Closes #1689

core/translate/emitter.rs

@@ -1133,10 +1133,6 @@ fn emit_update_insns(
     }
 
     for (index, (idx_cursor_id, record_reg)) in plan.indexes_to_update.iter().zip(&index_cursors) {
-        if !index.unique {
-            continue;
-        }
-
         let num_cols = index.columns.len();
         // allocate scratch registers for the index columns plus rowid
         let idx_start_reg = program.alloc_registers(num_cols + 1);
@@ -1161,6 +1157,7 @@ fn emit_update_insns(
             amount: 0,
         });
 
+        // this record will be inserted into the index later
         program.emit_insn(Insn::MakeRecord {
             start_reg: idx_start_reg,
             count: num_cols + 1,
@@ -1168,6 +1165,11 @@ fn emit_update_insns(
             index_name: Some(index.name.clone()),
         });
 
+        if !index.unique {
+            continue;
+        }
+
+        // check if the record already exists in the index for unique indexes and abort if so
         let constraint_check = program.allocate_label();
         program.emit_insn(Insn::NoConflict {
             cursor_id: *idx_cursor_id,
@@ -1280,7 +1282,7 @@ fn emit_update_insns(
             let num_regs = index.columns.len() + 1;
             let start_reg = program.alloc_registers(num_regs);
 
-            // Emit columns that are part of the index
+            // Delete existing index key
             index
                 .columns
                 .iter()
@@ -1304,6 +1306,7 @@ fn emit_update_insns(
                 cursor_id: idx_cursor_id,
             });
 
+            // Insert new index key (filled further above with values from set_clauses)
             program.emit_insn(Insn::IdxInsert {
                 cursor_id: idx_cursor_id,
                 record_reg: record_reg,

core/vdbe/execute.rs

@@ -3982,7 +3982,12 @@ pub fn op_idx_insert(
             let cursor = cursor.as_btree_mut();
             let record = match &state.registers[record_reg] {
                 Register::Record(ref r) => r,
-                _ => return Err(LimboError::InternalError("expected record".into())),
+                o => {
+                    return Err(LimboError::InternalError(format!(
+                        "expected record, got {:?}",
+                        o
+                    )));
+                }
             };
             // To make this reentrant in case of `moved_before` = false, we need to check if the previous cursor.insert started
             // a write/balancing operation. If it did, it means we already moved to the place we wanted.

testing/update.test

@@ -170,3 +170,18 @@ do_execsql_test_on_specific_db {:memory:} update_cache_full_regression_test_#162
     UPDATE t SET x = randomblob(4096) WHERE rowid = 1;
     SELECT count(*) FROM t;
 } {1}
+
+do_execsql_test_on_specific_db {:memory:} update_index_regression_test {
+    CREATE TABLE t(x, y);
+    CREATE INDEX tx ON t (x);
+    CREATE UNIQUE INDEX tyu ON t (y);
+    INSERT INTO t VALUES (1, 1);
+    SELECT x FROM t; -- uses tx index
+    SELECT y FROM t; -- uses ty index
+    UPDATE t SET x=2, y=2;
+    SELECT x FROM t; -- uses tx index
+    SELECT y FROM t; -- uses ty index    
+} {1
+1
+2
+2}