Quote reserved words in ON CONFLICT (#425)

Author: jayvdb

butane_core/src/db/pg.rs

@@ -866,6 +866,7 @@ fn change_column(table: &ATable, old: &AColumn, new: &AColumn) -> Result<String>
     Ok(result)
 }
 
+/// Write SQL that performs an insert or update.
 pub fn sql_insert_or_replace_with_placeholders(
     table: &str,
     columns: &[Column],
@@ -882,13 +883,24 @@ pub fn sql_insert_or_replace_with_placeholders(
         n + 1
     });
     write!(w, ")").unwrap();
-    write!(w, " ON CONFLICT ({}) DO ", pkcol.name()).unwrap();
+    write!(
+        w,
+        " ON CONFLICT ({}) DO ",
+        helper::quote_reserved_word(pkcol.name())
+    )
+    .unwrap();
     if columns.len() > 1 {
         write!(w, "UPDATE SET (").unwrap();
         helper::list_columns(columns, w);
         write!(w, ") = (").unwrap();
         columns.iter().fold("", |sep, c| {
-            write!(w, "{}excluded.{}", sep, c.name()).unwrap();
+            write!(
+                w,
+                "{}excluded.{}",
+                sep,
+                helper::quote_reserved_word(c.name())
+            )
+            .unwrap();
             ", "
         });
         write!(w, ")").unwrap();

butane_core/src/db/sqlite.rs

@@ -885,7 +885,12 @@ pub fn sql_insert_or_update(table: &str, columns: &[Column], pkcol: &Column, w:
         ", "
     });
     write!(w, ")").unwrap();
-    write!(w, " ON CONFLICT ({}) DO ", pkcol.name()).unwrap();
+    write!(
+        w,
+        " ON CONFLICT ({}) DO ",
+        helper::quote_reserved_word(pkcol.name())
+    )
+    .unwrap();
     if columns.len() > 1 {
         write!(w, "UPDATE SET (").unwrap();
         helper::list_columns(columns, w);

butane_core/src/db/turso.rs

@@ -1185,7 +1185,12 @@ pub fn sql_insert_or_update(table: &str, columns: &[Column], pkcol: &Column, w:
         ", "
     });
     write!(w, ")").unwrap();
-    write!(w, " ON CONFLICT ({}) DO ", pkcol.name()).unwrap();
+    write!(
+        w,
+        " ON CONFLICT ({}) DO ",
+        helper::quote_reserved_word(pkcol.name())
+    )
+    .unwrap();
     if columns.len() > 1 {
         write!(w, "UPDATE SET (").unwrap();
         helper::list_columns(columns, w);

butane_core/tests/autopk.rs

@@ -82,3 +82,81 @@ async fn auto_increment(conn: ConnectionAsync) {
     assert!(matches!(pk_val2, SqlVal::Int(_)), "Second PK should be Int");
     assert_ne!(pk_val, pk_val2, "PKs should be different");
 }
+
+/// Test that `insert_or_replace` correctly quotes reserved word primary keys in ON CONFLICT clause.
+#[butane_test(nomigrate)]
+async fn reserved_word(conn: ConnectionAsync) {
+    // Create table with "order" as primary key (a reserved SQL keyword)
+    let pkcol = Column::new("order", SqlType::BigInt);
+    let bar_column = Column::new("bar", SqlType::Text);
+
+    let mut table = ATable::new("reserved_pkey_test".to_string());
+
+    // Create "order" column as primary key with auto-increment
+    let order_col = AColumn::new(
+        pkcol.name(),
+        DeferredSqlType::KnownId(TypeIdentifier::Ty(SqlType::BigInt)),
+        false, // not nullable
+        true,  // is primary key
+        true,  // auto-increment (this is what we're testing!)
+        false, // not unique (pk already implies unique)
+        None,  // no default
+        None,  // no foreign key
+    );
+    table.add_column(order_col);
+
+    // Create bar column
+    let bar_col = AColumn::new(
+        bar_column.name(),
+        DeferredSqlType::KnownId(TypeIdentifier::Ty(SqlType::Text)),
+        false, // not nullable
+        false, // not pk
+        false, // not auto
+        false, // not unique
+        None,  // no default
+        None,  // no foreign key
+    );
+    table.add_column(bar_col);
+
+    // Generate and execute CREATE TABLE
+    let backend = conn.backend();
+    let adb = ADB::default();
+    let create_sql = backend
+        .create_migration_sql(&adb, vec![Operation::AddTable(table)])
+        .unwrap();
+
+    conn.execute(&create_sql).await.unwrap();
+
+    // Now test insert_or_replace which should properly quote the "order" column in the ON CONFLICT clause
+    let columns = [pkcol.clone(), bar_column.clone()];
+    let values = [SqlValRef::BigInt(1), SqlValRef::Text("first")];
+
+    // First insert
+    conn.insert_or_replace("reserved_pkey_test", &columns, &pkcol, &values)
+        .await
+        .unwrap();
+
+    // Second insert with same pk should update
+    let values2 = [SqlValRef::BigInt(1), SqlValRef::Text("updated")];
+    conn.insert_or_replace("reserved_pkey_test", &columns, &pkcol, &values2)
+        .await
+        .unwrap();
+
+    // Verify only one row exists after the upsert.
+    // TODO: There is a bug in turso that fails when using COUNT(anything) on this table.
+    let query_columns = [pkcol.clone(), bar_column.clone()];
+    let mut result = conn
+        .query("reserved_pkey_test", &query_columns, None, None, None, None)
+        .await
+        .unwrap();
+
+    let mut count = 0;
+    while result.next().unwrap().is_some() {
+        count += 1;
+    }
+    assert_eq!(
+        count, 1,
+        "Expected exactly 1 row after upsert, found {}",
+        count
+    );
+}