upsert() now requires a value for every primary key column

A record missing a primary key value - or with None for one - can
never match an existing row, because a NULL primary key never
satisfies ON CONFLICT. Such records were previously inserted as
brand new rows (silently, for multi-record upsert_all calls) or
triggered a KeyError after the insert had already happened. Both
cases, including upserting an empty record, now raise
PrimaryKeyRequired before any SQL is executed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnLnhsH25Nnv7LHhekUfPd

docs/changelog.rst

@@ -14,6 +14,7 @@ Unreleased
 - The ``sqlite-utils drop-table`` command now refuses to drop a view, and ``drop-view`` refuses to drop a table. Previously each would silently drop the wrong type of object if the name matched. Both now exit with an error suggesting the correct command to use.
 - Migrations applied by the new :ref:`migrations system <migrations>` now run inside a transaction, together with the record of the migration having been applied. If a migration raises an exception its changes are rolled back and it stays pending, so it can be safely re-applied after the error is fixed. Migrations that cannot run inside a transaction, such as those executing ``VACUUM``, can opt out using ``@migrations(transactional=False)`` - see :ref:`migrations_transactions`.
 - ``table.upsert()`` and ``table.upsert_all()`` now detect the primary key or compound primary key of an existing table, so the ``pk=`` argument is no longer required when upserting into a table that already has a primary key.
+- ``table.upsert()`` and ``table.upsert_all()`` now raise ``PrimaryKeyRequired`` if a record is missing a value for any primary key column, or has a value of ``None`` for one. Previously such records - which can never match an existing row - were quietly inserted as brand new rows, or triggered a confusing ``KeyError`` after the insert had already taken place.
 - ``db.table(table_name).insert({})`` can now be used to insert a row consisting entirely of default values into an existing table, using ``INSERT INTO ... DEFAULT VALUES``. (:issue:`759`)
 
 .. _v4_0rc1:

docs/python-api.rst

@@ -1021,6 +1021,8 @@ Note that the ``pk`` and ``column_order`` parameters here are optional if you ar
 
 An ``upsert_all()`` method is also available, which behaves like ``insert_all()`` but performs upserts instead.
 
+Every record passed to ``upsert()`` or ``upsert_all()`` must include a value for each primary key column - a record without one could never match an existing row, so a ``sqlite_utils.db.PrimaryKeyRequired`` exception is raised instead of quietly inserting a new row.
+
 .. note::
     ``.upsert()`` and ``.upsert_all()`` in sqlite-utils 1.x worked like ``.insert(..., replace=True)`` and ``.insert_all(..., replace=True)`` do in 2.x. See `issue #66 <https://github.com/simonw/sqlite-utils/issues/66>`__ for details of this change.
 

sqlite_utils/db.py

@@ -3199,6 +3199,11 @@ class Table(Queryable):
         # Dict-mode insert({}) has no explicit columns; SQLite spells that as
         # DEFAULT VALUES. List mode with no columns is a different input shape.
         if not list_mode and not all_columns:
+            if upsert:
+                raise PrimaryKeyRequired(
+                    "upsert() requires a value for the primary key - "
+                    "an empty record cannot be upserted"
+                )
             or_clause = ""
             if replace:
                 or_clause = " OR REPLACE"
@@ -3286,6 +3291,26 @@ class Table(Queryable):
 
         # Everything from here on is for upsert=True
         pk_cols = [pk] if isinstance(pk, str) else list(pk)
+        # Every record must provide a value for every primary key column - a
+        # NULL primary key never matches ON CONFLICT, so the record would be
+        # inserted as a brand new row instead of upserted
+        missing_pk_cols = [c for c in pk_cols if c not in all_columns]
+        if missing_pk_cols:
+            raise PrimaryKeyRequired(
+                "upsert() requires a value for the primary key column{}: {}".format(
+                    "s" if len(missing_pk_cols) > 1 else "",
+                    ", ".join(missing_pk_cols),
+                )
+            )
+        pk_indexes = [all_columns.index(c) for c in pk_cols]
+        for record_values in values:
+            if any(record_values[i] is None for i in pk_indexes):
+                raise PrimaryKeyRequired(
+                    "upsert() requires a value for the primary key column{}: {}".format(
+                        "s" if len(pk_cols) > 1 else "",
+                        ", ".join(pk_cols),
+                    )
+                )
         non_pk_cols = [c for c in all_columns if c not in pk_cols]
         conflict_sql = ", ".join(quote_identifier(c) for c in pk_cols)
 

tests/test_upsert.py

@@ -49,6 +49,42 @@ def test_upsert_error_if_no_pk(fresh_db):
         table.upsert({"id": 1, "name": "Cleo"})
 
 
+@pytest.mark.parametrize("use_old_upsert", (False, True))
+def test_upsert_empty_record_errors(use_old_upsert):
+    db = Database(memory=True, use_old_upsert=use_old_upsert)
+    table = db["table"]
+    table.insert({"id": 1, "name": "Cleo"}, pk="id")
+    with pytest.raises(PrimaryKeyRequired):
+        table.upsert({}, pk="id")
+    with pytest.raises(PrimaryKeyRequired):
+        table.upsert_all([{}, {}], pk="id")
+    # No rows can have been inserted
+    assert table.count == 1
+
+
+@pytest.mark.parametrize("use_old_upsert", (False, True))
+def test_upsert_missing_pk_value_errors(use_old_upsert):
+    db = Database(memory=True, use_old_upsert=use_old_upsert)
+    table = db["table"]
+    table.insert({"id": 1, "name": "Cleo"}, pk="id")
+    # Records that omit the pk column entirely
+    with pytest.raises(PrimaryKeyRequired):
+        table.upsert_all([{"name": "Pancakes"}, {"name": "Marnie"}], pk="id")
+    # A record with an explicit None pk value can never conflict
+    with pytest.raises(PrimaryKeyRequired):
+        table.upsert({"id": None, "name": "Pancakes"}, pk="id")
+    assert list(table.rows) == [{"id": 1, "name": "Cleo"}]
+
+
+def test_upsert_missing_compound_pk_value_errors(fresh_db):
+    table = fresh_db["table"]
+    table.insert({"a": "x", "b": "y", "v": 1}, pk=("a", "b"))
+    # Missing one component of the detected compound primary key
+    with pytest.raises(PrimaryKeyRequired):
+        table.upsert({"a": "x", "v": 2})
+    assert list(table.rows) == [{"a": "x", "b": "y", "v": 1}]
+
+
 def test_upsert_error_if_existing_table_has_no_pk(fresh_db):
     table = fresh_db.create_table("table", {"id": int, "name": str})
     with pytest.raises(PrimaryKeyRequired):