From 9c6037718dcbb22c11cd12f1f7c7213e581bc2e1 Mon Sep 17 00:00:00 2001
From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com>
Date: Tue, 22 Sep 2026 14:06:39 +0000
Subject: [PATCH] fix(base): SqlBaseKvStore.__setitem__ updated nothing on
 SQLite, it duplicated

Update-vs-insert was decided from a SELECT's rowcount, which is -1 on SQLite,
so writing an existing key always inserted a duplicate row and reads kept
returning the old value. Same rowcount trap #4 fixed in __len__.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 sqldol/base.py                        | 11 ++++--
 sqldol/tests/test_kv_store_setitem.py | 48 +++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 sqldol/tests/test_kv_store_setitem.py

diff --git a/sqldol/base.py b/sqldol/base.py
index 68e8aa6..e02b50a 100644
--- a/sqldol/base.py
+++ b/sqldol/base.py
@@ -310,9 +310,14 @@ def __setitem__(self, key, value):
         query = self._table_selection_query.where(filter)
 
         with self.engine.connect() as connection:
-            result = connection.execute(query)
-
-            if result.rowcount == 1:
+            # Note: Existence is asked of the rows themselves, not of ``rowcount``,
+            # which a SELECT reports as -1 on drivers that don't pre-buffer results
+            # (SQLite, for one) -- the same trap ``__len__`` used to fall into. Going
+            # by ``rowcount`` there made every write to an existing key an INSERT, so
+            # the table grew a duplicate row and the old value kept being read back.
+            key_is_present = connection.execute(query).first() is not None
+
+            if key_is_present:
                 query = update(self.table).values(**value).where(filter)
             else:
                 query = insert(self.table).values(**value)
diff --git a/sqldol/tests/test_kv_store_setitem.py b/sqldol/tests/test_kv_store_setitem.py
new file mode 100644
index 0000000..f6f6785
--- /dev/null
+++ b/sqldol/tests/test_kv_store_setitem.py
@@ -0,0 +1,48 @@
+"""Tests for ``SqlBaseKvStore.__setitem__``: writing an existing key must update it.
+
+Uses in-memory SQLite only -- no external service, no credentials.
+"""
+
+import pytest
+from sqlalchemy import create_engine, text
+
+from sqldol.stores import SqlDictStore
+
+TABLE_NAME = "t"
+KEY_COLUMN = "k"
+
+
+@pytest.fixture
+def engine():
+    """An in-memory SQLite engine holding ``t(k, v)`` with rows a/1 and b/2."""
+    engine = create_engine("sqlite:///:memory:")
+    with engine.connect() as connection:
+        connection.execute(text(f"CREATE TABLE {TABLE_NAME} (k TEXT, v TEXT)"))
+        connection.execute(
+            text(f"INSERT INTO {TABLE_NAME} VALUES ('a', '1'), ('b', '2')")
+        )
+        connection.commit()
+    return engine
+
+
+def test_setitem_on_an_existing_key_updates_instead_of_duplicating(engine):
+    """It used to decide update-vs-insert from a SELECT's ``rowcount``, which is -1
+    on SQLite, so it always inserted: ``len`` grew and ``store[k]`` kept returning
+    the old value."""
+    store = SqlDictStore(engine, TABLE_NAME, key_columns=KEY_COLUMN)
+
+    store["a"] = {"k": "a", "v": "10"}
+
+    assert store["a"] == {"k": "a", "v": "10"}
+    assert len(store) == 2
+    assert sorted(store) == ["a", "b"]
+
+
+def test_setitem_on_a_new_key_inserts(engine):
+    store = SqlDictStore(engine, TABLE_NAME, key_columns=KEY_COLUMN)
+
+    store["c"] = {"k": "c", "v": "3"}
+    store["c"] = {"k": "c", "v": "4"}
+
+    assert store["c"] == {"k": "c", "v": "4"}
+    assert len(store) == 3