Fix index removal and additions + add smoketest (#1444)

crates/core/src/db/datastore/locking_tx_datastore/committed_state.rs

@@ -321,6 +321,22 @@ impl CommittedState {
         Ok(())
     }
 
+    /// After replaying all old transactions,
+    /// inserts and deletes into the system tables
+    /// might not be reflected in the schemas of the built tables.
+    /// So we must re-schema every built table.
+    pub(super) fn reschema_tables(&mut self) -> Result<()> {
+        // For already built tables, we need to reschema them to account for constraints et al.
+        let mut schemas = Vec::with_capacity(self.tables.len());
+        for table_id in self.tables.keys().copied() {
+            schemas.push(self.schema_for_table_raw(&ExecutionContext::default(), table_id)?);
+        }
+        for (table, schema) in self.tables.values_mut().zip(schemas) {
+            table.with_mut_schema(|s| *s = schema);
+        }
+        Ok(())
+    }
+
     /// After replaying all old transactions, tables which have rows will
     /// have been created in memory, but tables with no rows will not have
     /// been created. This function ensures that they are created.

crates/core/src/db/datastore/locking_tx_datastore/datastore.rs

@@ -113,6 +113,7 @@ impl Locking {
         // `build_missing_tables` must be called before indexes.
         // Honestly this should maybe just be one big procedure.
         // See John Carmack's philosophy on this.
+        committed_state.reschema_tables()?;
         committed_state.build_missing_tables()?;
         committed_state.build_indexes()?;
         committed_state.build_sequence_state(&mut sequence_state)?;
@@ -390,7 +391,7 @@ impl MutTxDatastore for Locking {
     }
 
     fn drop_index_mut_tx(&self, tx: &mut Self::MutTx, index_id: IndexId) -> Result<()> {
-        tx.drop_index(index_id, self.database_address)
+        tx.drop_index(index_id, true, self.database_address)
     }
 
     fn index_id_from_name_mut_tx(&self, tx: &Self::MutTx, index_name: &str) -> Result<Option<IndexId>> {
@@ -414,11 +415,13 @@ impl MutTxDatastore for Locking {
     }
 
     fn drop_constraint_mut_tx(&self, tx: &mut Self::MutTx, constraint_id: ConstraintId) -> Result<()> {
-        tx.drop_constraint(constraint_id, self.database_address)
+        let ctx = &ExecutionContext::internal(self.database_address);
+        tx.drop_constraint(ctx, constraint_id)
     }
 
     fn constraint_id_from_name(&self, tx: &Self::MutTx, constraint_name: &str) -> Result<Option<ConstraintId>> {
-        tx.constraint_id_from_name(constraint_name, self.database_address)
+        let ctx = &ExecutionContext::internal(self.database_address);
+        tx.constraint_id_from_name(ctx, constraint_name)
     }
 
     fn iter_mut_tx<'a>(
@@ -1254,8 +1257,8 @@ mod tests {
                 IdxSchema { id: seq_start + 1, table, col: 1, name: "name_idx", unique: true },
             ]),
             map_array([
-                ConstraintRow { constraint_id: seq_start,     table_id: table, columns: col(0), constraints: Constraints::unique(), constraint_name: "ct_Foo_id_idx_unique" },
-                ConstraintRow { constraint_id: seq_start + 1, table_id: table, columns: col(1), constraints: Constraints::unique(), constraint_name: "ct_Foo_name_idx_unique" }
+                ConstraintRow { constraint_id: seq_start,     table_id: table, columns: col(0), constraints: Constraints::unique(), constraint_name: "ct_Foo_id_unique" },
+                ConstraintRow { constraint_id: seq_start + 1, table_id: table, columns: col(1), constraints: Constraints::unique(), constraint_name: "ct_Foo_name_unique" }
             ]),
              map_array([
                 SequenceRow { id: seq_start, table, col_pos: 0, name: "seq_Foo_id", start: 1 }
@@ -1747,16 +1750,17 @@ mod tests {
     fn test_create_index_pre_commit() -> ResultTest<()> {
         let (datastore, tx, table_id) = setup_table()?;
         datastore.commit_mut_tx_for_test(tx)?;
+
         let mut tx = datastore.begin_mut_tx(IsolationLevel::Serializable);
         let row = u32_str_u32(0, "Foo", 18); // 0 will be ignored.
         datastore.insert_mut_tx(&mut tx, table_id, row)?;
         datastore.commit_mut_tx_for_test(tx)?;
+
         let mut tx = datastore.begin_mut_tx(IsolationLevel::Serializable);
         let index_def = IndexDef::btree("age_idx".into(), ColId(2), true);
         datastore.create_index_mut_tx(&mut tx, table_id, index_def)?;
         let ctx = ExecutionContext::default();
         let query = query_st_tables(&ctx, &tx);
-
         let seq_start = FIRST_NON_SYSTEM_ID;
         let index_rows = query.scan_st_indexes()?;
         #[rustfmt::skip]
@@ -1782,7 +1786,7 @@ mod tests {
                 cols: _,
                 value: _,
             }))) => (),
-            _ => panic!("Expected an unique constraint violation error."),
+            e => panic!("Expected an unique constraint violation error but got {e:?}."),
         }
         #[rustfmt::skip]
         assert_eq!(all_rows(&datastore, &tx, table_id), vec![u32_str_u32(1, "Foo", 18)]);