From e11fc7f8b279358e1e306c56d1b02c6a4673159b Mon Sep 17 00:00:00 2001 From: Michael Cuffaro Date: Wed, 20 Nov 2024 13:56:03 +0100 Subject: [PATCH] properly support SQLite in rename_column() --- src/cli.rs | 1 - src/toolkit.rs | 99 +++++++++++++++++++++++++++++++++++++------------- src/valve.rs | 13 ++++++- 3 files changed, 85 insertions(+), 28 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 739aaea..f9923a0 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1373,7 +1373,6 @@ pub async fn rename_column( } }; if load_table { - valve.set_interactive(false); valve .load_tables(&vec![table], true) .await diff --git a/src/toolkit.rs b/src/toolkit.rs index 8638b96..ed527ba 100644 --- a/src/toolkit.rs +++ b/src/toolkit.rs @@ -2099,22 +2099,46 @@ pub fn get_label_for_column( /// Given a table name, a column name, and a new name for the column, rename the column to the /// new name in the database table and associated views, using the given database transaction. pub async fn rename_db_column_tx( + db_kind: &DbKind, + config: &ValveConfig, tx: &mut Transaction<'_, sqlx::Any>, table: &str, column: &str, - new_name: &str, + new_col_name: &str, ) -> Result<()> { - for table in [ - table, - &format!("{table}_conflict"), - &format!("{table}_view"), - &format!("{table}_text_view"), - ] { - let sql = format!(r#"ALTER TABLE "{table}" RENAME COLUMN "{column}" TO "{new_name}""#); + // Although PostgreSQL supports an ALTER VIEW statement, SQLite does not, so we have to + // manually drop and recreate views in that case. For simplicitly, then, we use the manual + // method in both cases. + + // Begin by dropping the standard and text views: + let sql = format!(r#"DROP VIEW "{table}_text_view""#); + let query = sqlx_query(&sql); + query.execute(tx.acquire().await?).await?; + let sql = format!(r#"DROP VIEW "{table}_view""#); + let query = sqlx_query(&sql); + query.execute(tx.acquire().await?).await?; + + // Next, rename the column in both the table and its corresponding conflict table: + for suffix in ["", "_conflict"] { + let sql = format!( + r#"ALTER TABLE "{table}{suffix}" RENAME COLUMN "{column}" TO "{new_col_name}""# + ); let query = sqlx_query(&sql); query.execute(tx.acquire().await?).await?; } + // Now recreate both views: + let sql = get_sql_for_standard_view(table, db_kind); + let query = sqlx_query(&sql); + query.execute(tx.acquire().await?).await?; + // For text views, we need to indicate the change to the column name via a HashMap from the + // old name to the new: + let mut changes = HashMap::new(); + changes.insert(column, new_col_name); + let sql = get_sql_for_text_view(&config.table, table, None, Some(changes), db_kind); + let query = sqlx_query(&sql); + query.execute(tx.acquire().await?).await?; + Ok(()) } @@ -2125,16 +2149,13 @@ pub async fn rename_db_table_tx( config: &ValveConfig, tx: &mut Transaction<'_, sqlx::Any>, table: &str, - new_name: &str, + new_tbl_name: &str, ) -> Result<()> { - for suffix in ["", "_conflict"] { - let sql = format!(r#"ALTER TABLE "{table}{suffix}" RENAME TO "{new_name}{suffix}""#); - let query = sqlx_query(&sql); - query.execute(tx.acquire().await?).await?; - } + // Although PostgreSQL supports an ALTER VIEW statement, SQLite does not, so we have to + // manually drop and recreate views in that case. For simplicitly, then, we use the manual + // method in both cases. - // Although PostgreSQL supports ALTER VIEW, SQLite does not, so we have to manually drop - // and recreate: + // Begin by dropping the standard and text views: let sql = format!(r#"DROP VIEW "{table}_text_view""#); let query = sqlx_query(&sql); query.execute(tx.acquire().await?).await?; @@ -2142,10 +2163,18 @@ pub async fn rename_db_table_tx( let query = sqlx_query(&sql); query.execute(tx.acquire().await?).await?; - let sql = get_sql_for_standard_view(new_name, db_kind); + // Next, rename the table and its corresponding conflict table: + for suffix in ["", "_conflict"] { + let sql = format!(r#"ALTER TABLE "{table}{suffix}" RENAME TO "{new_tbl_name}{suffix}""#); + let query = sqlx_query(&sql); + query.execute(tx.acquire().await?).await?; + } + + // Now recreate both views: + let sql = get_sql_for_standard_view(new_tbl_name, db_kind); let query = sqlx_query(&sql); query.execute(tx.acquire().await?).await?; - let sql = get_sql_for_text_view(&config.table, table, Some(new_name), db_kind); + let sql = get_sql_for_text_view(&config.table, table, Some(new_tbl_name), None, db_kind); let query = sqlx_query(&sql); query.execute(tx.acquire().await?).await?; @@ -2348,17 +2377,24 @@ pub fn get_sql_for_standard_view(table: &str, kind: &DbKind) -> String { /// [get_sql_for_standard_view()]. Unlike the standard view generated by that function, the view /// generated by this function (called my_table_text_view) always shows all of the values (which are /// all rendered as text) of every column in the table, even when those values contain SQL datatype -/// errors. If `base_table` is given, then the view will be constructed over it, otherwise it will -/// be built over table. In either case, `table` will be used to look up the table's columns. This -/// is useful in case there is a mismatch between the configured table name and the name of the -/// corresponding database table, which can occur during the process of renaming a table. +/// errors. If `base_table_change` is given, then the view will be constructed over the table +/// indicated by it, otherwise it will be built over `table`. In either case, `table` is used to +/// actually look up the table's columns in the tables configuration. Similarly, if +/// `base_column_changes` is given, which is a map from currently configured column names to the +/// "new" names that should be used when constructing the view, then use the new name for a column +/// if it appears in the map. Otherwise just use the currently configured name. Note that +/// `base_table_change` and `base_column_changes` are used in the case where there is a mismatch +/// between the configured table/column name and the name of the corresponding database +/// table/column. Such mismatches can occur when one is in the middle of the process of renaming a +/// table or column. pub fn get_sql_for_text_view( tables_config: &HashMap, table: &str, - base_table: Option<&str>, + base_table_change: Option<&str>, + base_column_changes: Option>, kind: &DbKind, ) -> String { - let base_table = match base_table { + let base_table = match base_table_change { None => table, Some(name) => name, }; @@ -2371,7 +2407,20 @@ pub fn get_sql_for_text_view( let real_columns = &tables_config .get(table) - .and_then(|t| Some(t.column.keys().map(|k| k.to_string()).collect::>())) + .and_then(|t| { + Some( + t.column_order + .iter() + .map(|col| match &base_column_changes { + Some(changes) => changes + .get(&col.as_str()) + .unwrap_or(&col.as_str()) + .to_string(), + None => col.to_string(), + }) + .collect::>(), + ) + }) .expect(&format!("Undefined table '{}'", table)); // Add a second "text view" such that the datatypes of all values are TEXT and appear diff --git a/src/valve.rs b/src/valve.rs index 8fb220c..e8ab44a 100644 --- a/src/valve.rs +++ b/src/valve.rs @@ -1324,6 +1324,7 @@ impl Valve { self.reconfigure() } + // TODO: This function doesn't seem to be able to handle a table name with spaces. /// Given a table name, the path of the table's associated TSV file, and other parameters /// used as input to the function, [guess()], guess the table's configuration on the basis /// of the contents of the TSV file and the other, and add it to the database. @@ -1822,7 +1823,15 @@ impl Valve { // otherwise, when we call save_table() on the data table, when it tries to find the // newly renamed column it won't be there and it will save all the values in the column // as null. - rename_db_column_tx(&mut tx, table, column, new_name).await?; + rename_db_column_tx( + &self.db_kind, + &self.config, + &mut tx, + table, + column, + new_name, + ) + .await?; // Commit the transaction: tx.commit().await?; @@ -2660,7 +2669,7 @@ impl Valve { let create_view_sql = get_sql_for_standard_view(&table, &self.db_kind); let create_text_view_sql = - get_sql_for_text_view(tables_config, &table, None, &self.db_kind); + get_sql_for_text_view(tables_config, &table, None, None, &self.db_kind); table_statements.push(create_view_sql); table_statements.push(create_text_view_sql); }