Use strict tables, any

Author: simolus3

crates/core/src/schema/management.rs

@@ -141,7 +141,7 @@ fn update_tables(
             let mut create = CreateTableStatement::from(table);
             if table.direct {
                 for column in &table.columns {
-                    create.push_column(&column.name, &column.type_name);
+                    create.push_any_column(&column.name);
                 }
             }
 
@@ -184,24 +184,20 @@ fn direct_table_migration(
 
     struct ExistingColumn<'a> {
         column: &'a Column,
-        index_in_table: usize,
         found_in_old: bool,
     }
 
     let mut new_columns: Vec<_> = new
         .columns
         .iter()
-        .enumerate()
-        .map(|(i, column)| ExistingColumn {
+        .map(|column| ExistingColumn {
             column,
-            index_in_table: i,
             found_in_old: false,
         })
         .collect();
     new_columns.sort_by(|a, b| a.column.name.cmp(&b.column.name));
 
     let mut deleted_columns = vec![];
-    let mut changed_column_types = vec![];
 
     for old_column in &old.columns {
         let Ok(new_column_index) =
@@ -214,18 +210,12 @@ fn direct_table_migration(
         let new_column = &mut new_columns[new_column_index];
         new_column.found_in_old = true;
 
-        if !new_column
-            .column
-            .type_name
-            .eq_ignore_ascii_case(&old_column.type_name)
-        {
-            changed_column_types.push((new_column.index_in_table, &new_column.column.type_name));
-        }
+        // For found columns, the type doesn't matter as we generate ANY types for all of them.
     }
 
     new_columns.retain(|c| !c.found_in_old);
 
-    if new_columns.is_empty() && deleted_columns.is_empty() && changed_column_types.is_empty() {
+    if new_columns.is_empty() && deleted_columns.is_empty() {
         return Ok(()); // Nothing to migrate.
     }
 
@@ -250,48 +240,11 @@ fn direct_table_migration(
         }
     }
 
-    if !changed_column_types.is_empty() {
-        // To change column types, we change the CREATE TABLE statement for the table. As long as
-        // we do this in a way that doesn't alter the order of existing columns, this doesn't
-        // corrupt data (column types in non-strict tables only affects type affinity for inserts
-        // and updates). The proper way to run this migration requires copying data, which we want
-        // to avoid.
-        let schema_writable_before = db.has_writable_schema();
-        if !schema_writable_before {
-            db.set_writable_schema(true)?;
-        }
-
-        let mut new_create_table = CreateTableStatement::from(new);
-        let mut changed_column_types = changed_column_types.iter().peekable();
-
-        for (i, column) in old.columns.iter().enumerate() {
-            let changed_type = changed_column_types
-                .next_if(|(index, _)| *index == i)
-                .map(|(_, type_name)| type_name.as_str());
-
-            new_create_table.push_column(&column.name, changed_type.unwrap_or(&column.type_name));
-        }
-
-        let new_create_table = new_create_table.finish();
-
-        {
-            let stmt = db
-                .prepare_v2("UPDATE sqlite_schema SET sql = ? WHERE type = 'table' AND name = ?")?;
-            stmt.bind_text(1, &new_create_table.sql, Destructor::STATIC)?;
-            stmt.bind_text(2, &new.name, Destructor::STATIC)?;
-            stmt.exec()?;
-        }
-
-        if !schema_writable_before {
-            db.set_writable_schema(false)?;
-        }
-    }
-
     // Add new columns, drop old ones
     for new_column in new_columns {
         let mut stmt = SqlBuffer::new();
         stmt.alter_table(&new.name);
-        stmt.add_column(new_column.column);
+        stmt.add_column(&new_column.column.name, "ANY");
         db.exec_safe_str(&stmt.sql)?;
     }
 

crates/core/src/schema/table_info.rs

@@ -506,6 +506,7 @@ pub enum PendingStatementValue {
 
 pub struct CreateTableStatement {
     create_table: SqlBuffer,
+    direct: bool,
 }
 
 impl From<&Table> for CreateTableStatement {
@@ -526,18 +527,26 @@ impl From<&Table> for CreateTableStatement {
             create_table.push_str(", data TEXT");
         }
 
-        Self { create_table }
+        Self {
+            create_table,
+            direct: value.direct,
+        }
     }
 }
 
 impl CreateTableStatement {
-    pub fn push_column(&mut self, name: &str, type_name: &str) {
+    pub fn push_any_column(&mut self, name: &str) {
         self.create_table.push_char(',');
-        self.create_table.column_definition(name, type_name);
+        self.create_table.column_definition(name, "ANY");
     }
 
     pub fn finish(mut self) -> SqlBuffer {
-        self.create_table.push_str(");");
+        self.create_table.push_char(')');
+        if self.direct {
+            self.create_table.push_str(" STRICT");
+        }
+
+        self.create_table.push_char(';');
         self.create_table
     }
 }

crates/core/src/utils/database.rs

@@ -1,4 +1,4 @@
-use core::ffi::{CStr, c_char, c_int};
+use core::ffi::{CStr, c_char};
 
 use alloc::ffi::CString;
 use num_traits::FromPrimitive;
@@ -83,37 +83,6 @@ impl Database {
         statement.bind_text(1, param, Destructor::STATIC)?;
         statement.exec()
     }
-
-    pub fn has_writable_schema(self) -> bool {
-        let mut result: c_int = 0;
-        let _ = sqlite::db_config(
-            self.sqlite,
-            sqlite::DBCONFIG_WRITABLE_SCHEMA,
-            -1, // negative to leave the setting unchanged.
-            &mut result,
-        );
-        result != 0
-    }
-
-    pub fn set_writable_schema(self, enable: bool) -> Result<()> {
-        let mut result = 0;
-        let expected_result = if enable { 1 } else { 0 };
-
-        let _ = sqlite::db_config(
-            self.sqlite,
-            sqlite::DBCONFIG_WRITABLE_SCHEMA,
-            expected_result,
-            &mut result,
-        );
-
-        if expected_result != result {
-            return Err(PowerSyncError::state_error(
-                "Could not apply writable schema",
-            ));
-        }
-
-        Ok(())
-    }
 }
 
 pub struct Statement {

crates/core/src/utils/sql_buffer.rs

@@ -6,9 +6,7 @@ use core::{
 use alloc::{format, string::String};
 
 use crate::{
-    error::PowerSyncError,
-    schema::{Column, SchemaTable},
-    views::table_columns_to_json_object_with_filter,
+    error::PowerSyncError, schema::SchemaTable, views::table_columns_to_json_object_with_filter,
 };
 
 const DOUBLE_QUOTE: char = '"';
@@ -125,9 +123,9 @@ impl SqlBuffer {
         let _ = self.identifier().write_str(name);
     }
 
-    pub fn add_column(&mut self, column: &Column) {
+    pub fn add_column(&mut self, name: &str, type_name: &str) {
         self.push_str("ADD COLUMN ");
-        self.column_definition(&column.name, &column.type_name);
+        self.column_definition(name, type_name);
     }
 
     pub fn column_definition(&mut self, name: &str, type_name: &str) {

crates/sqlite_nostd/src/capi.rs

@@ -7,7 +7,6 @@ use alloc::borrow::ToOwned;
 use alloc::ffi::CString;
 
 pub use crate::bindings::{
-    SQLITE_DBCONFIG_WRITABLE_SCHEMA as DBCONFIG_WRITABLE_SCHEMA,
     SQLITE_DETERMINISTIC as DETERMINISTIC, SQLITE_DIRECTONLY as DIRECTONLY,
     SQLITE_INDEX_CONSTRAINT_EQ as INDEX_CONSTRAINT_EQ,
     SQLITE_INDEX_CONSTRAINT_GE as INDEX_CONSTRAINT_GE,
@@ -50,13 +49,12 @@ mod aliased {
         sqlite3_column_value as column_value, sqlite3_commit_hook as commit_hook,
         sqlite3_context_db_handle as context_db_handle,
         sqlite3_create_function_v2 as create_function_v2,
-        sqlite3_create_module_v2 as create_module_v2, sqlite3_db_config as db_config,
-        sqlite3_declare_vtab as declare_vtab, sqlite3_errcode as errcode, sqlite3_errmsg as errmsg,
-        sqlite3_error_offset as error_offset, sqlite3_exec as exec, sqlite3_finalize as finalize,
-        sqlite3_free as free, sqlite3_get_autocommit as get_autocommit,
-        sqlite3_get_auxdata as get_auxdata, sqlite3_libversion as libversion,
-        sqlite3_libversion_number as libversion_number, sqlite3_malloc as malloc,
-        sqlite3_malloc64 as malloc64, sqlite3_mutex_alloc as mutex_alloc,
+        sqlite3_create_module_v2 as create_module_v2, sqlite3_declare_vtab as declare_vtab,
+        sqlite3_errcode as errcode, sqlite3_errmsg as errmsg, sqlite3_error_offset as error_offset,
+        sqlite3_exec as exec, sqlite3_finalize as finalize, sqlite3_free as free,
+        sqlite3_get_autocommit as get_autocommit, sqlite3_get_auxdata as get_auxdata,
+        sqlite3_libversion as libversion, sqlite3_libversion_number as libversion_number,
+        sqlite3_malloc as malloc, sqlite3_malloc64 as malloc64, sqlite3_mutex_alloc as mutex_alloc,
         sqlite3_mutex_enter as mutex_enter, sqlite3_mutex_free as mutex_free,
         sqlite3_mutex_leave as mutex_leave, sqlite3_mutex_try as mutex_try,
         sqlite3_next_stmt as next_stmt, sqlite3_open as open, sqlite3_prepare_v2 as prepare_v2,
@@ -227,16 +225,6 @@ pub fn close(db: *mut sqlite3) -> c_int {
     unsafe { invoke_sqlite!(close, db) }
 }
 
-pub extern "C" fn db_config(
-    db: *mut sqlite3,
-    op: c_uint,
-    option: c_int,
-    result: &mut c_int,
-) -> c_int {
-    let ptr = core::ptr::from_mut(result);
-    unsafe { invoke_sqlite!(db_config, db, op as c_int, option, ptr) }
-}
-
 pub fn vtab_config(db: *mut sqlite3, options: u32) -> c_int {
     unsafe { invoke_sqlite!(vtab_config, db, options as i32) }
 }

dart/test/schema_test.dart

@@ -370,7 +370,7 @@ END''',
         )[0].columnAt(0);
         expect(
           createTable,
-          'CREATE TABLE "users"(id TEXT PRIMARY KEY NOT NULL /* ps-managed */,"name" text)',
+          'CREATE TABLE "users"(id TEXT PRIMARY KEY NOT NULL /* ps-managed */,"name" ANY) STRICT',
         );
 
         final triggers = db