From 5ad3e5244bc0189620b1288004305e6f65bc96f7 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Fri, 26 Sep 2025 18:52:43 -0400 Subject: [PATCH 1/4] Fix explicit ON CONFLICT target of non-rowid alias primary keys in UPSERT --- core/translate/upsert.rs | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/core/translate/upsert.rs b/core/translate/upsert.rs index 38ef1d5d6..78ad0fe53 100644 --- a/core/translate/upsert.rs +++ b/core/translate/upsert.rs @@ -128,21 +128,22 @@ fn effective_collation_for_index_col(idx_col: &IndexColumn, table: &Table) -> St /// If no target is specified, it is an automatic match for PRIMARY KEY pub fn upsert_matches_pk(upsert: &Upsert, table: &Table) -> bool { let Some(t) = upsert.index.as_ref() else { - // Omitted target is automatic - return true; + // omitted target matches everything, CatchAll handled elsewhere + return false; }; - if !t.targets.len().eq(&1) { + if t.targets.len() != 1 { return false; } - let pk = table - .columns() - .iter() - .find(|c| c.is_rowid_alias || c.primary_key) - .unwrap_or(ROWID_COLUMN); - extract_target_key(&t.targets[0].expr).is_some_and(|tk| { - tk.col_name - .eq_ignore_ascii_case(pk.name.as_ref().unwrap_or(&String::new())) - }) + // Only treat as PK if the PK is the rowid alias (INTEGER PRIMARY KEY) + let pk = table.columns().iter().find(|c| c.is_rowid_alias); + if let Some(pkcol) = pk { + extract_target_key(&t.targets[0].expr).is_some_and(|tk| { + tk.col_name + .eq_ignore_ascii_case(pkcol.name.as_ref().unwrap_or(&String::new())) + }) + } else { + false + } } /// Returns array of chaned column indicies and whether rowid was changed. @@ -300,17 +301,17 @@ pub fn resolve_upsert_target( return Ok(ResolvedUpsertTarget::CatchAll); } - // Targeted: must match PK or a non-partial UNIQUE index. + // Targeted: must match PK, only if PK is a rowid alias if upsert_matches_pk(upsert, table) { return Ok(ResolvedUpsertTarget::PrimaryKey); } + // Otherwise match a UNIQUE index, also covering non-rowid PRIMARY KEYs for idx in schema.get_indices(table.get_name()) { if idx.unique && upsert_matches_index(upsert, idx, table) { return Ok(ResolvedUpsertTarget::Index(Arc::clone(idx))); } } - // Match SQLite’s error text: crate::bail_parse_error!( "ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint" ); From 2e186ce8fa8ed89aea2e95593f7ab75828df767e Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Fri, 26 Sep 2025 18:57:30 -0400 Subject: [PATCH 2/4] Add regression test for upsert explict non-rowid alias PK --- testing/upsert.test | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/testing/upsert.test b/testing/upsert.test index cab46cc1b..b1a849a60 100644 --- a/testing/upsert.test +++ b/testing/upsert.test @@ -337,6 +337,14 @@ do_execsql_test_on_specific_db {:memory:} upsert-doubly-qualified-target { SELECT * FROM dq; } {1|new} +# https://github.com/tursodatabase/turso/issues/3384 +do_execsql_test_on_specific_db {:memory:} upsert-non-rowid-pk-target { + create table phonebook(name text primary key, phonenumber text, validDate date); + insert into phonebook values ('Alice','704-545-3333','2018-10-10'); + insert into phonebook values ('Alice','704-111-1111','2018-10-20') on conflict (name) do update set phonenumber=excluded.phonenumber, validDate=excluded.validDate; + SELECT phonenumber, validDate FROM phonebook; +} {704-111-1111|2018-10-20} + # TODO: uncomment these when we support collations in indexes # (right now it errors on Parse Error: cannot use expressions in CREATE INDEX) From 8517355c0c77164136b174343c615ac2699a380f Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Fri, 26 Sep 2025 19:06:33 -0400 Subject: [PATCH 3/4] make clippy happy --- core/translate/upsert.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/translate/upsert.rs b/core/translate/upsert.rs index 78ad0fe53..96ae01bac 100644 --- a/core/translate/upsert.rs +++ b/core/translate/upsert.rs @@ -21,7 +21,7 @@ use crate::{ emit_returning_results, translate_expr, translate_expr_no_constant_opt, walk_expr_mut, NoConstantOptReason, ReturningValueRegisters, }, - insert::{Insertion, ROWID_COLUMN}, + insert::Insertion, plan::ResultSetColumn, }, util::normalize_ident, From 16d1e7e6a90ab8f4e7ff2b63b0489d6dd06882b3 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Sat, 27 Sep 2025 07:51:59 -0400 Subject: [PATCH 4/4] Rename function and update comment to match behavior --- core/translate/upsert.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/translate/upsert.rs b/core/translate/upsert.rs index 96ae01bac..4696f10b9 100644 --- a/core/translate/upsert.rs +++ b/core/translate/upsert.rs @@ -124,9 +124,8 @@ fn effective_collation_for_index_col(idx_col: &IndexColumn, table: &Table) -> St .unwrap_or_else(|| "binary".to_string()) } -/// Match ON CONFLICT target to the PRIMARY KEY, if any. -/// If no target is specified, it is an automatic match for PRIMARY KEY -pub fn upsert_matches_pk(upsert: &Upsert, table: &Table) -> bool { +/// Match ON CONFLICT target to the PRIMARY KEY/rowid alias. +pub fn upsert_matches_rowid_alias(upsert: &Upsert, table: &Table) -> bool { let Some(t) = upsert.index.as_ref() else { // omitted target matches everything, CatchAll handled elsewhere return false; @@ -302,7 +301,7 @@ pub fn resolve_upsert_target( } // Targeted: must match PK, only if PK is a rowid alias - if upsert_matches_pk(upsert, table) { + if upsert_matches_rowid_alias(upsert, table) { return Ok(ResolvedUpsertTarget::PrimaryKey); }