From 413f30559e8a674deb552db80335a03a51f8a955 Mon Sep 17 00:00:00 2001 From: Dave Gaeddert Date: Thu, 23 Jul 2026 12:11:25 -0500 Subject: [PATCH 1/6] Add ON CONFLICT SET overrides and RETURNING created-flag plumbing Extends the insert compiler and dialect so a single INSERT ... ON CONFLICT DO UPDATE can (a) override the SET clause per column with compiled expressions and (b) append a raw "(xmax = 0)" column to RETURNING. Inert until upsert() uses it. --- plain-postgres/plain/postgres/dialect.py | 62 ++++++++++++++----- plain-postgres/plain/postgres/sql/compiler.py | 55 +++++++++++++++- plain-postgres/plain/postgres/sql/query.py | 6 ++ 3 files changed, 105 insertions(+), 18 deletions(-) diff --git a/plain-postgres/plain/postgres/dialect.py b/plain-postgres/plain/postgres/dialect.py index b49c8f142a..797dd89cfa 100644 --- a/plain-postgres/plain/postgres/dialect.py +++ b/plain-postgres/plain/postgres/dialect.py @@ -380,14 +380,21 @@ def lookup_cast(lookup_type: str, field: Field | None = None) -> str: return lookup -def returning_columns(fields: list[Field]) -> str: - """Return the RETURNING clause SQL for the given fields, or "" when empty.""" - if not fields: - return "" +def returning_columns(fields: list[Field], extra: str = "") -> str: + """Return the RETURNING clause SQL for the given fields, or "" when empty. + + `extra` is a raw SQL expression appended as a trailing RETURNING column. + upsert() uses it to return the "(xmax = 0)" created flag alongside the + field columns. + """ columns = [ f"{quote_name(field.model.model_options.db_table)}.{quote_name(field.column)}" for field in fields ] + if extra: + columns.append(extra) + if not columns: + return "" return "RETURNING {}".format(", ".join(columns)) @@ -583,15 +590,40 @@ def on_conflict_suffix_sql( on_conflict: OnConflict | None, update_fields: Iterable[str], unique_fields: Iterable[str], + conflict_overrides: dict[str, str] | None = None, ) -> str: - if on_conflict == OnConflict.UPDATE: - return "ON CONFLICT({}) DO UPDATE SET {}".format( - ", ".join(map(quote_name, unique_fields)), - ", ".join( - [ - f"{field} = EXCLUDED.{field}" - for field in map(quote_name, update_fields) - ] - ), - ) - return "" + if on_conflict != OnConflict.UPDATE: + return "" + + conflict_overrides = conflict_overrides or {} + unique_columns = list(unique_fields) + update_columns = list(update_fields) + + # Each base update column is set to its EXCLUDED value (the value the + # INSERT proposed), except where conflict_overrides supplies a per-column + # SQL fragment (e.g. an atomic "count = count + 1" counter). + assignments = [] + for column in update_columns: + quoted = quote_name(column) + if column in conflict_overrides: + assignments.append(f"{quoted} = {conflict_overrides[column]}") + else: + assignments.append(f"{quoted} = EXCLUDED.{quoted}") + + # Override-only columns aren't among the EXCLUDED update columns, so append + # them after (a counter column, say, that isn't otherwise being set). + for column, fragment in conflict_overrides.items(): + if column not in update_columns: + assignments.append(f"{quote_name(column)} = {fragment}") + + if not assignments: + # Nothing to update -- Postgres still requires a DO UPDATE SET body, + # and only DO UPDATE (not DO NOTHING) returns the conflicting row via + # RETURNING. Set a unique column to itself as a no-op. + first = quote_name(unique_columns[0]) + assignments.append(f"{first} = EXCLUDED.{first}") + + return "ON CONFLICT({}) DO UPDATE SET {}".format( + ", ".join(map(quote_name, unique_columns)), + ", ".join(assignments), + ) diff --git a/plain-postgres/plain/postgres/sql/compiler.py b/plain-postgres/plain/postgres/sql/compiler.py index f170700724..02c83df3f2 100644 --- a/plain-postgres/plain/postgres/sql/compiler.py +++ b/plain-postgres/plain/postgres/sql/compiler.py @@ -1483,12 +1483,19 @@ def as_sql( # ty: ignore[invalid-method-override] # Returns list for internal placeholder_rows, param_rows = self.assemble_as_sql(fields, value_rows) + # conflict_defaults compile to per-column SQL fragments (with their own + # params) that override the default "col = EXCLUDED.col" assignment. + # Their params follow the VALUES params, since DO UPDATE SET comes after + # VALUES in the statement. + conflict_overrides, conflict_params = self._conflict_override_sql() conflict_suffix_sql = on_conflict_suffix_sql( fields, # ty: ignore[invalid-argument-type] self.query.on_conflict, (f.column for f in self.query.update_fields), (f.column for f in self.query.unique_fields), + conflict_overrides, ) + extra_returning = "(xmax = 0)" if self.query.returning_created else "" if self.returning_fields: # Use RETURNING clause to get inserted values result.append( @@ -1496,15 +1503,57 @@ def as_sql( # ty: ignore[invalid-method-override] # Returns list for internal ) if conflict_suffix_sql: result.append(conflict_suffix_sql) - if returning := returning_columns(self.returning_fields): + if returning := returning_columns(self.returning_fields, extra_returning): result.append(returning) - return [(" ".join(result), tuple(chain.from_iterable(param_rows)))] + params = tuple(chain.from_iterable(param_rows)) + tuple(conflict_params) + return [(" ".join(result), params)] # Bulk insert without returning fields result.append(bulk_insert_sql(fields, placeholder_rows)) # ty: ignore[invalid-argument-type] if conflict_suffix_sql: result.append(conflict_suffix_sql) - return [(" ".join(result), tuple(p for ps in param_rows for p in ps))] + params = tuple(p for ps in param_rows for p in ps) + tuple(conflict_params) + return [(" ".join(result), params)] + + def _conflict_override_sql(self) -> tuple[dict[str, str], list[Any]]: + """Compile conflict_defaults into {column: SQL fragment} plus params. + + Reuses the same value-compilation as UPDATE ... SET so an override can + be a plain value or an expression (e.g. F("count") + 1, which resolves + to the target row's existing column for an atomic counter). + """ + overrides: dict[str, str] = {} + params: list[Any] = [] + for field, val in self.query.conflict_defaults.items(): + if isinstance(val, ResolvableExpression): + val = val.resolve_expression( + self.query, allow_joins=False, for_save=True + ) + if val.contains_aggregate: + raise FieldError( + "Aggregate functions are not allowed in this query " + f"({field.name}={val!r})." + ) + if val.contains_over_clause: + raise FieldError( + "Window expressions are not allowed in this query " + f"({field.name}={val!r})." + ) + val = field.get_db_prep_save(val, connection=self.connection) + if hasattr(field, "get_placeholder"): + placeholder = field.get_placeholder(val, self, self.connection) # ty: ignore[call-non-callable] + else: + placeholder = "%s" + if hasattr(val, "as_sql"): + sql, val_params = self.compile(val) + overrides[field.column] = placeholder % sql + params.extend(val_params) + elif val is not None: + overrides[field.column] = placeholder + params.append(val) + else: + overrides[field.column] = "NULL" + return overrides, params def execute_sql( # ty: ignore[invalid-method-override] self, returning_fields: list | None = None diff --git a/plain-postgres/plain/postgres/sql/query.py b/plain-postgres/plain/postgres/sql/query.py index ca9e641cc2..52e5e9cf50 100644 --- a/plain-postgres/plain/postgres/sql/query.py +++ b/plain-postgres/plain/postgres/sql/query.py @@ -2604,6 +2604,8 @@ def __init__( on_conflict: OnConflict | None = None, update_fields: list[Field] | None = None, unique_fields: list[Field] | None = None, + conflict_defaults: dict[Field, Any] | None = None, + returning_created: bool = False, **kwargs: Any, ) -> None: super().__init__(*args, **kwargs) @@ -2612,6 +2614,10 @@ def __init__( self.on_conflict = on_conflict self.update_fields: list[Field] = update_fields or [] self.unique_fields: list[Field] = unique_fields or [] + # Per-column DO UPDATE SET overrides (upsert conflict_defaults) and the + # flag for the trailing "(xmax = 0)" created column in RETURNING. + self.conflict_defaults: dict[Field, Any] = conflict_defaults or {} + self.returning_created: bool = returning_created def insert_values(self, fields: list[Any], objs: list[Any]) -> None: self.fields = fields From 0bb25ad1ca19522dc0b4a45f0611a811e5803c8e Mon Sep 17 00:00:00 2001 From: Dave Gaeddert Date: Thu, 23 Jul 2026 12:13:24 -0500 Subject: [PATCH 2/6] Add QuerySet.upsert() and remove update_or_create() upsert(**values, unique_fields=[...], defaults=..., create_defaults=..., conflict_defaults=...) runs one INSERT ... ON CONFLICT DO UPDATE ... RETURNING and returns (obj, created), hydrating obj from the post-write row. defaults apply on insert and conflict-update; create_defaults on insert only; conflict_defaults override the SET per column (plain values or expressions such as F("count") + 1). created comes from the RETURNING (xmax = 0) flag. Removes update_or_create() and migrates every caller (flags, sessions, the FK reverse and M2M related managers) to upsert(). The M2M manager keeps its add-if-created behavior. --- plain-flags/plain/flags/flags.py | 3 +- .../plain/postgres/fields/related_managers.py | 41 ++++- plain-postgres/plain/postgres/query.py | 149 +++++++++++++----- .../app/examples/models/relationships.py | 6 + plain-postgres/tests/public/test_m2m.py | 31 +++- plain-postgres/tests/public/test_upsert.py | 130 +++++++++++++++ plain-sessions/plain/sessions/core.py | 5 +- 7 files changed, 314 insertions(+), 51 deletions(-) create mode 100644 plain-postgres/tests/public/test_upsert.py diff --git a/plain-flags/plain/flags/flags.py b/plain-flags/plain/flags/flags.py index 9e603bc27c..ebe3bc6d2f 100644 --- a/plain-flags/plain/flags/flags.py +++ b/plain-flags/plain/flags/flags.py @@ -80,9 +80,10 @@ def retrieve_or_compute_value(self) -> Any: # Create an associated DB Flag that we can use to enable/disable # and tie the results to - flag_obj, _ = Flag.query.update_or_create( + flag_obj, _ = Flag.query.upsert( name=flag_name, defaults={"used_at": timezone.now()}, + unique_fields=["name"], ) if not flag_obj.enabled: diff --git a/plain-postgres/plain/postgres/fields/related_managers.py b/plain-postgres/plain/postgres/fields/related_managers.py index 10be1a4ff2..6be3bc338b 100644 --- a/plain-postgres/plain/postgres/fields/related_managers.py +++ b/plain-postgres/plain/postgres/fields/related_managers.py @@ -210,10 +210,24 @@ def get_or_create(self, **kwargs: Any) -> tuple[T, bool]: kwargs[self.field.name] = self.instance return self.model.query.get_or_create(**kwargs) - def update_or_create(self, **kwargs: Any) -> tuple[T, bool]: + def upsert( + self, + *, + defaults: dict[str, Any] | None = None, + create_defaults: dict[str, Any] | None = None, + conflict_defaults: dict[str, Any] | None = None, + unique_fields: list[str], + **kwargs: Any, + ) -> tuple[T, bool]: self._check_fk_val() kwargs[self.field.name] = self.instance - return self.model.query.update_or_create(**kwargs) + return self.model.query.upsert( + defaults=defaults, + create_defaults=create_defaults, + conflict_defaults=conflict_defaults, + unique_fields=unique_fields, + **kwargs, + ) def remove(self, *objs: T, bulk: bool = True) -> None: # remove() is only provided if the ForeignKeyField can have a value of null @@ -494,12 +508,25 @@ def get_or_create( self.add(obj, through_defaults=through_defaults) return obj, created - def update_or_create( - self, *, through_defaults: dict[str, Any] | None = None, **kwargs: Any + def upsert( + self, + *, + through_defaults: dict[str, Any] | None = None, + defaults: dict[str, Any] | None = None, + create_defaults: dict[str, Any] | None = None, + conflict_defaults: dict[str, Any] | None = None, + unique_fields: list[str], + **kwargs: Any, ) -> tuple[T, bool]: - obj, created = self.model.query.update_or_create(**kwargs) - # We only need to add() if created because if we got an object back - # from get() then the relationship already exists. + obj, created = self.model.query.upsert( + defaults=defaults, + create_defaults=create_defaults, + conflict_defaults=conflict_defaults, + unique_fields=unique_fields, + **kwargs, + ) + # We only need to add() if created because if the row already existed + # the relationship does too. if created: self.add(obj, through_defaults=through_defaults) return obj, created diff --git a/plain-postgres/plain/postgres/query.py b/plain-postgres/plain/postgres/query.py index fc287f45c0..521dfccb3e 100644 --- a/plain-postgres/plain/postgres/query.py +++ b/plain-postgres/plain/postgres/query.py @@ -905,52 +905,121 @@ def get_or_create( pass raise - def update_or_create( + def upsert( self, + *, defaults: dict[str, Any] | None = None, create_defaults: dict[str, Any] | None = None, + conflict_defaults: dict[str, Any] | None = None, + unique_fields: list[str], **kwargs: Any, ) -> tuple[T, bool]: """ - Look up an object with the given kwargs, updating one with defaults - if it exists, otherwise create a new one. Optionally, an object can - be created with different values than defaults by using - create_defaults. - Return a tuple (object, created), where created is a boolean - specifying whether an object was created. + Insert a row, or update the existing row that conflicts on + unique_fields, in a single INSERT ... ON CONFLICT DO UPDATE statement. + Return a tuple (object, created), where created is True when a new row + was inserted and False when an existing row was updated. The object is + hydrated from the post-write row -- no second query. + + Value sources: + - **kwargs: the identifying and inserted values (must include the + unique_fields). Applied on both insert and conflict-update. + - defaults: applied on both insert and conflict-update. + - create_defaults: extra values applied on insert only. + - conflict_defaults: per-column overrides for the conflict-update SET. + Each value may be a plain value or an expression (e.g. + F("count") + 1 for an atomic counter). A column here is set on + conflict even if it isn't otherwise being updated. + + On conflict, every non-unique, non-PK column drawn from kwargs and + defaults is set to the value the INSERT proposed, minus any column an + override in conflict_defaults replaces. create_defaults never take part + in the update. The merged result is not validated -- consistent with + the other bulk write paths. + + unique_fields must name the primary key or a UniqueConstraint declared + on the model without a condition or expressions, and every unique field + must have a non-null value (NULL never conflicts in Postgres). """ - if create_defaults is None: - update_defaults = create_defaults = defaults or {} - else: - update_defaults = defaults or {} - with transaction.atomic(): - # Lock the row so that a concurrent update is blocked until - # update_or_create() has performed its save. - obj, created = self.select_for_update().get_or_create( - create_defaults, **kwargs + meta = self.model._model_meta + + if not unique_fields: + raise ValueError("upsert() requires unique_fields.") + if not self.model.model_options.unique_fields_match_constraint( + set(unique_fields) + ): + raise ValueError( + f"upsert() unique_fields {unique_fields} on {self.model.__name__} " + "must name the primary key or a UniqueConstraint declared on the " + "model without a condition or expressions." ) - if created: - return obj, created - for k, v in resolve_callables(update_defaults): - setattr(obj, k, v) - - update_fields = set(update_defaults) - concrete_field_names = self.model._model_meta._non_pk_concrete_field_names - # update_fields does not support non-concrete fields. - if concrete_field_names.issuperset(update_fields): - # Add fields which are set on pre_save(), e.g. update_now fields. - # This is to maintain backward compatibility as these fields - # are not updated unless explicitly specified in the - # update_fields list. - for field in self.model._model_meta.local_concrete_fields: - if not ( - field.primary_key or field.__class__.pre_save is Field.pre_save - ): - update_fields.add(field.name) - obj.update(fields=update_fields) - else: - obj.update() - return obj, False + + defaults = defaults or {} + create_defaults = create_defaults or {} + conflict_defaults = conflict_defaults or {} + + # The inserted row: create_defaults first, then defaults, then kwargs, + # so the more explicit source wins on any overlap. Callables in each are + # resolved (matching the rest of the get/create family). + insert_values: dict[str, Any] = {} + for source in (create_defaults, defaults, kwargs): + insert_values.update(resolve_callables(source)) + + unique_field_objs = [meta.get_forward_field(name) for name in unique_fields] + for field in unique_field_objs: + assert field.name is not None + if insert_values.get(field.name) is None: + raise ValueError( + f"upsert() requires a non-null {field.name}; NULL never " + "conflicts in Postgres, so it cannot be upserted." + ) + + # The conflict-update columns: everything from kwargs and defaults that + # isn't a unique field or the PK. create_defaults are insert-only, so + # they never appear here. + unique_field_names = {f.name for f in unique_field_objs} + update_names = { + name for name in (*kwargs, *defaults) if name not in unique_field_names + } + update_field_objs = [] + for name in update_names: + field = meta.get_forward_field(name) + if not field.primary_key: + update_field_objs.append(field) + + conflict_default_objs = { + meta.get_forward_field(name): value + for name, value in conflict_defaults.items() + } + + obj = self.model(**insert_values) + obj._prepare_related_fields_for_save(operation_name="upsert") + + fields = list(meta.local_concrete_fields) + if obj.id is None: + id_field = meta.get_forward_field("id") + fields = [f for f in fields if f is not id_field] + + returning_fields = list(meta.concrete_fields) + with transaction.atomic(savepoint=False): + rows = self._insert( + [obj], + fields=fields, + returning_fields=returning_fields, + on_conflict=OnConflict.UPDATE, + update_fields=update_field_objs, + unique_fields=unique_field_objs, + conflict_defaults=conflict_default_objs, + returning_created=True, + ) + + assert rows is not None + row = rows[0] + created = bool(row[-1]) + result = self.model.from_db( + (f.name for f in returning_fields), row[: len(returning_fields)] + ) + return cast(T, result), created def _extract_model_params( self, defaults: dict[str, Any] | None, **kwargs: Any @@ -1500,6 +1569,8 @@ def _insert( on_conflict: OnConflict | None = None, update_fields: list[Field] | None = None, unique_fields: list[Field] | None = None, + conflict_defaults: dict[Field, Any] | None = None, + returning_created: bool = False, ) -> list[tuple[Any, ...]] | None: """ Insert a new record for the given model. This provides an interface to @@ -1510,6 +1581,8 @@ def _insert( on_conflict=on_conflict if on_conflict else None, update_fields=update_fields, unique_fields=unique_fields, + conflict_defaults=conflict_defaults, + returning_created=returning_created, ) query.insert_values(fields, objs) # InsertQuery returns SQLInsertCompiler which has different execute_sql signature diff --git a/plain-postgres/tests/app/examples/models/relationships.py b/plain-postgres/tests/app/examples/models/relationships.py index 275b252e80..d300a53a9e 100644 --- a/plain-postgres/tests/app/examples/models/relationships.py +++ b/plain-postgres/tests/app/examples/models/relationships.py @@ -14,6 +14,12 @@ class Tag(postgres.Model): to="Widget", field="tags" ) + model_options = postgres.Options( + constraints=[ + postgres.UniqueConstraint(fields=["name"], name="unique_tag_name"), + ] + ) + @postgres.register_model class WidgetTag(postgres.Model): diff --git a/plain-postgres/tests/public/test_m2m.py b/plain-postgres/tests/public/test_m2m.py index cddd731ca5..cdfbe15f4d 100644 --- a/plain-postgres/tests/public/test_m2m.py +++ b/plain-postgres/tests/public/test_m2m.py @@ -31,13 +31,38 @@ def test_create_unique_constraint(db): assert Widget.query.count() == 1 -def test_update_or_create_unique_constraint(db): - Widget.query.update_or_create(name="Toyota", size="Tundra") - Widget.query.update_or_create(name="Toyota", size="Tundra") +def test_upsert_unique_constraint(db): + Widget.query.upsert(name="Toyota", size="Tundra", unique_fields=["name", "size"]) + Widget.query.upsert(name="Toyota", size="Tundra", unique_fields=["name", "size"]) assert Widget.query.count() == 1 +def test_m2m_manager_upsert_adds_relationship_on_insert(db): + """The M2M manager's upsert() adds the through-row only when it inserts a + new target row -- if the target already existed the relationship does too. + """ + widget = Widget.query.create(name="Tesla", size="Model 3") + + tag, created = widget.tags.upsert(name="GPS", unique_fields=["name"]) + assert created is True + assert widget.tags.query.count() == 1 + assert widget.tags.query.first() == tag + + +def test_m2m_manager_upsert_does_not_readd_on_conflict(db): + widget = Widget.query.create(name="Tesla", size="Model 3") + gps = Tag.query.create(name="GPS") + widget.tags.add(gps) + + # The tag already exists and is already related; upsert must not create a + # duplicate through-row. + tag, created = widget.tags.upsert(name="GPS", unique_fields=["name"]) + assert created is False + assert tag.id == gps.id + assert widget.tags.query.count() == 1 + + def test_many_to_many_forward_accessor(db): """Test that the forward ManyToManyField accessor works.""" widget = Widget.query.create(name="Tesla", size="Model 3") diff --git a/plain-postgres/tests/public/test_upsert.py b/plain-postgres/tests/public/test_upsert.py new file mode 100644 index 0000000000..2a08a151f3 --- /dev/null +++ b/plain-postgres/tests/public/test_upsert.py @@ -0,0 +1,130 @@ +"""QuerySet.upsert() inserts a row or updates the conflicting one. + +One INSERT ... ON CONFLICT (unique_fields) DO UPDATE ... RETURNING statement. +Returns (obj, created): obj is hydrated from the post-write row -- no second +query -- and created is True on insert, False on conflict-update. +""" + +from __future__ import annotations + +import pytest +from app.examples.models.relationships import Widget +from app.examples.models.upsert import UpsertItem + +from plain.postgres.expressions import F + + +def test_upsert_inserts_new_row(db): + obj, created = UpsertItem.query.upsert(key="a", value=1, unique_fields=["key"]) + + assert created is True + assert obj.id is not None + assert obj.key == "a" + assert obj.value == 1 + assert UpsertItem.query.get(key="a").value == 1 + + +def test_upsert_updates_conflicting_row(db): + UpsertItem(key="a", value=1).create() + existing_id = UpsertItem.query.get(key="a").id + + obj, created = UpsertItem.query.upsert(key="a", value=99, unique_fields=["key"]) + + assert created is False + # The updated row keeps its primary key, and obj carries the merged value. + assert obj.id == existing_id + assert obj.value == 99 + assert UpsertItem.query.count() == 1 + assert UpsertItem.query.get(key="a").value == 99 + + +def test_upsert_defaults_apply_on_insert_and_update(db): + obj, created = UpsertItem.query.upsert( + key="a", defaults={"value": 5}, unique_fields=["key"] + ) + assert (created, obj.value) == (True, 5) + + obj, created = UpsertItem.query.upsert( + key="a", defaults={"value": 7}, unique_fields=["key"] + ) + assert (created, obj.value) == (False, 7) + + +def test_upsert_create_defaults_apply_on_insert_only(db): + obj, created = UpsertItem.query.upsert( + key="a", + defaults={"value": 1}, + create_defaults={"label": "created"}, + unique_fields=["key"], + ) + assert (created, obj.label) == (True, "created") + + # On conflict, create_defaults is not applied, so the label is untouched + # while defaults still updates value. + obj, created = UpsertItem.query.upsert( + key="a", + defaults={"value": 2}, + create_defaults={"label": "ignored-on-conflict"}, + unique_fields=["key"], + ) + assert created is False + assert obj.label == "created" + assert obj.value == 2 + + +def test_upsert_conflict_defaults_increment_counter_atomically(db): + UpsertItem(key="a", value=10).create() + + obj, created = UpsertItem.query.upsert( + key="a", + value=0, # the value the INSERT would have proposed (ignored on conflict) + conflict_defaults={"value": F("value") + 1}, + unique_fields=["key"], + ) + + assert created is False + assert obj.value == 11 + assert UpsertItem.query.get(key="a").value == 11 + + +def test_upsert_conflict_defaults_apply_on_insert_uses_inserted_value(db): + # On insert there's no existing row, so the inserted value stands; the + # conflict_defaults override only takes effect on a later conflict. + obj, created = UpsertItem.query.upsert( + key="a", + value=3, + conflict_defaults={"value": F("value") + 100}, + unique_fields=["key"], + ) + assert (created, obj.value) == (True, 3) + + +def test_upsert_all_unique_fields_is_idempotent(db): + # When every inserted column is a unique field there's nothing to update; + # the second call must still return the existing row (created=False). + obj1, created1 = Widget.query.upsert( + name="Toyota", size="Tundra", unique_fields=["name", "size"] + ) + obj2, created2 = Widget.query.upsert( + name="Toyota", size="Tundra", unique_fields=["name", "size"] + ) + + assert created1 is True + assert created2 is False + assert obj1.id == obj2.id + assert Widget.query.count() == 1 + + +def test_upsert_requires_unique_fields(db): + with pytest.raises(ValueError, match="requires unique_fields"): + UpsertItem.query.upsert(key="a", unique_fields=[]) + + +def test_upsert_unique_fields_must_match_a_constraint(db): + with pytest.raises(ValueError, match="must name the primary key"): + UpsertItem.query.upsert(key="a", value=1, unique_fields=["value"]) + + +def test_upsert_rejects_null_unique_value(db): + with pytest.raises(ValueError, match="non-null"): + UpsertItem.query.upsert(key=None, unique_fields=["key"]) diff --git a/plain-sessions/plain/sessions/core.py b/plain-sessions/plain/sessions/core.py index 51d35b5316..3733c0b3f9 100644 --- a/plain-sessions/plain/sessions/core.py +++ b/plain-sessions/plain/sessions/core.py @@ -161,7 +161,7 @@ def create(self) -> None: def save(self) -> None: """ - Save the current session data to the database using update_or_create. + Save the current session data to the database using upsert. """ data = self._get_session_data(no_load=False) @@ -169,13 +169,14 @@ def save(self) -> None: if self.session_key is None: self.session_key = self._get_new_session_key() - self._session_instance, created = self._model.query.update_or_create( + self._session_instance, created = self._model.query.upsert( session_key=self.session_key, defaults={ "session_data": data, "expires_at": timezone.now() + timedelta(seconds=settings.SESSION_COOKIE_AGE), }, + unique_fields=["session_key"], ) if created: From 22f49b30ca14ba2454a5fa605b0967d9a96ef544 Mon Sep 17 00:00:00 2001 From: Dave Gaeddert Date: Thu, 23 Jul 2026 12:15:22 -0500 Subject: [PATCH 3/6] Document upsert() in plain-postgres docs and rules --- .claude/rules/plain-postgres.md | 4 +- plain-postgres/plain/postgres/README.md | 42 +++++++++++++++++++ .../agents/.claude/rules/plain-postgres.md | 4 +- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/.claude/rules/plain-postgres.md b/.claude/rules/plain-postgres.md index ff79127867..85345cc31b 100644 --- a/.claude/rules/plain-postgres.md +++ b/.claude/rules/plain-postgres.md @@ -63,11 +63,11 @@ Use `Model.query` to build querysets (e.g., `User.query.filter(is_active=True)`) - Use `.annotate(Count(...))` instead of calling `.count()` per row - Fetch all data in the view — templates should never trigger queries - Use `.exists()` not `.count() > 0`, `.count()` not `len(qs)` -- Use `bulk_create`/`bulk_update` for batch ops, `bulk_upsert` for atomic insert-or-update, `.update()`/`.delete()` for mass ops +- Use `bulk_create`/`bulk_update` for batch ops, `upsert`/`bulk_upsert` for atomic insert-or-update (single row / many), `.update()`/`.delete()` for mass ops - Use `.values_list()` when you only need specific columns - Wrap multi-step writes in `transaction.atomic()` - Instance writes are `obj.create()` (always INSERT) and `obj.update()` (always UPDATE; `update(fields=[...])` limits the columns) — there is no `save()`, `force_insert`, or `force_update`. Constructing an instance then `create()`-ing it inserts; a hand-set `id` that collides raises `IntegrityError`. -- `create()`/`update()` raise `ValidationError` (not raw `psycopg.IntegrityError`) on a declared unique/check constraint violation, even a raced one — the DB enforces it, so inside an open `transaction.atomic()` the violation aborts the transaction (wrap the write in its own `atomic()` to catch and keep using the transaction). Set-based writes (`QuerySet.update()`/`bulk_create()`) raise raw `psycopg.IntegrityError`. Retrying on conflict? `except (psycopg.IntegrityError, ValidationError)`, or `bulk_upsert(objs, update_fields=[...], unique_fields=[...])` for an atomic insert-or-update +- `create()`/`update()` raise `ValidationError` (not raw `psycopg.IntegrityError`) on a declared unique/check constraint violation, even a raced one — the DB enforces it, so inside an open `transaction.atomic()` the violation aborts the transaction (wrap the write in its own `atomic()` to catch and keep using the transaction). Set-based writes (`QuerySet.update()`/`bulk_create()`) raise raw `psycopg.IntegrityError`. Retrying on conflict? `except (psycopg.IntegrityError, ValidationError)`, or `upsert(**values, unique_fields=[...])` (single row) / `bulk_upsert(objs, update_fields=[...], unique_fields=[...])` (many) for an atomic insert-or-update - Always paginate list queries — unbounded querysets get slower as data grows Run `uv run plain docs postgres` for full patterns with code examples. diff --git a/plain-postgres/plain/postgres/README.md b/plain-postgres/plain/postgres/README.md index 50dd20fdaf..fb80ad1598 100644 --- a/plain-postgres/plain/postgres/README.md +++ b/plain-postgres/plain/postgres/README.md @@ -418,6 +418,48 @@ CacheItem.query.bulk_upsert( objects by that key, so it's safe to run concurrently without deadlocking on overlapping keys. +#### Use `upsert` for a single insert-or-update + +`upsert(*, unique_fields, defaults=None, create_defaults=None, conflict_defaults=None, **kwargs)` +is the single-row counterpart to `bulk_upsert`. It runs one +`INSERT ... ON CONFLICT (unique_fields) DO UPDATE SET ... RETURNING` statement and +returns `(obj, created)` — `created` is `True` when a new row was inserted, +`False` when the conflicting row was updated. The object is hydrated from the +post-write row, so there's no second query. + +```python +# Insert the flag, or refresh used_at on the existing one. +flag, created = Flag.query.upsert( + name="beta-dashboard", + defaults={"used_at": timezone.now()}, + unique_fields=["name"], +) +``` + +Value sources: + +- `**kwargs` and `defaults` are applied on **both** insert and conflict-update. + `kwargs` carries the identifying values (including `unique_fields`). +- `create_defaults` is applied on **insert only** — extras that must not change + when the row already exists. +- `conflict_defaults` overrides the `DO UPDATE SET` for specific columns. A value + can be a plain value or an expression, so `{"count": F("count") + 1}` is an + atomic counter that reads the existing row: + +```python +view, created = PageView.query.upsert( + path="/home", + conflict_defaults={"count": F("count") + 1}, + unique_fields=["path"], +) +``` + +On conflict, every non-unique, non-PK column from `kwargs`/`defaults` is set to the +value the INSERT proposed, minus any column a `conflict_defaults` override +replaces. As with `bulk_upsert`, `unique_fields` must name the primary key or a +`UniqueConstraint` (no condition, no expressions), every unique field must be +non-null, and the merged result is not validated. + #### Use queryset `.update()` / `.delete()` for mass operations ```python diff --git a/plain-postgres/plain/postgres/agents/.claude/rules/plain-postgres.md b/plain-postgres/plain/postgres/agents/.claude/rules/plain-postgres.md index ff79127867..85345cc31b 100644 --- a/plain-postgres/plain/postgres/agents/.claude/rules/plain-postgres.md +++ b/plain-postgres/plain/postgres/agents/.claude/rules/plain-postgres.md @@ -63,11 +63,11 @@ Use `Model.query` to build querysets (e.g., `User.query.filter(is_active=True)`) - Use `.annotate(Count(...))` instead of calling `.count()` per row - Fetch all data in the view — templates should never trigger queries - Use `.exists()` not `.count() > 0`, `.count()` not `len(qs)` -- Use `bulk_create`/`bulk_update` for batch ops, `bulk_upsert` for atomic insert-or-update, `.update()`/`.delete()` for mass ops +- Use `bulk_create`/`bulk_update` for batch ops, `upsert`/`bulk_upsert` for atomic insert-or-update (single row / many), `.update()`/`.delete()` for mass ops - Use `.values_list()` when you only need specific columns - Wrap multi-step writes in `transaction.atomic()` - Instance writes are `obj.create()` (always INSERT) and `obj.update()` (always UPDATE; `update(fields=[...])` limits the columns) — there is no `save()`, `force_insert`, or `force_update`. Constructing an instance then `create()`-ing it inserts; a hand-set `id` that collides raises `IntegrityError`. -- `create()`/`update()` raise `ValidationError` (not raw `psycopg.IntegrityError`) on a declared unique/check constraint violation, even a raced one — the DB enforces it, so inside an open `transaction.atomic()` the violation aborts the transaction (wrap the write in its own `atomic()` to catch and keep using the transaction). Set-based writes (`QuerySet.update()`/`bulk_create()`) raise raw `psycopg.IntegrityError`. Retrying on conflict? `except (psycopg.IntegrityError, ValidationError)`, or `bulk_upsert(objs, update_fields=[...], unique_fields=[...])` for an atomic insert-or-update +- `create()`/`update()` raise `ValidationError` (not raw `psycopg.IntegrityError`) on a declared unique/check constraint violation, even a raced one — the DB enforces it, so inside an open `transaction.atomic()` the violation aborts the transaction (wrap the write in its own `atomic()` to catch and keep using the transaction). Set-based writes (`QuerySet.update()`/`bulk_create()`) raise raw `psycopg.IntegrityError`. Retrying on conflict? `except (psycopg.IntegrityError, ValidationError)`, or `upsert(**values, unique_fields=[...])` (single row) / `bulk_upsert(objs, update_fields=[...], unique_fields=[...])` (many) for an atomic insert-or-update - Always paginate list queries — unbounded querysets get slower as data grows Run `uv run plain docs postgres` for full patterns with code examples. From a08b6eaf5207654cc6fa125605306072ac9423d8 Mon Sep 17 00:00:00 2001 From: Dave Gaeddert Date: Thu, 23 Jul 2026 12:35:56 -0500 Subject: [PATCH 4/6] Share upsert compilation and validation helpers Extract one _compile_assignment_value helper for the UPDATE ... SET and INSERT ... ON CONFLICT DO UPDATE SET value cascade, so conflict_defaults now goes through the same related-field (prepare_database_save) branch. Keep returning_columns() pure and move the (xmax = 0) created-flag trick next to the returning_created flag. Share the unique_fields and non-null key checks between upsert() and bulk_upsert(), and validate upsert()'s merged keys with the same FieldError path get_or_create() uses. --- plain-postgres/plain/postgres/dialect.py | 15 +- plain-postgres/plain/postgres/query.py | 88 +++++++----- plain-postgres/plain/postgres/sql/compiler.py | 130 ++++++++---------- .../0021_upsertowner_upsertitem_owner.py | 27 ++++ .../tests/app/examples/models/upsert.py | 10 ++ plain-postgres/tests/public/test_upsert.py | 47 ++++++- 6 files changed, 198 insertions(+), 119 deletions(-) create mode 100644 plain-postgres/tests/app/examples/migrations/0021_upsertowner_upsertitem_owner.py diff --git a/plain-postgres/plain/postgres/dialect.py b/plain-postgres/plain/postgres/dialect.py index 797dd89cfa..e22436d6fa 100644 --- a/plain-postgres/plain/postgres/dialect.py +++ b/plain-postgres/plain/postgres/dialect.py @@ -380,21 +380,14 @@ def lookup_cast(lookup_type: str, field: Field | None = None) -> str: return lookup -def returning_columns(fields: list[Field], extra: str = "") -> str: - """Return the RETURNING clause SQL for the given fields, or "" when empty. - - `extra` is a raw SQL expression appended as a trailing RETURNING column. - upsert() uses it to return the "(xmax = 0)" created flag alongside the - field columns. - """ +def returning_columns(fields: list[Field]) -> str: + """Return the RETURNING clause SQL for the given fields, or "" when empty.""" + if not fields: + return "" columns = [ f"{quote_name(field.model.model_options.db_table)}.{quote_name(field.column)}" for field in fields ] - if extra: - columns.append(extra) - if not columns: - return "" return "RETURNING {}".format(", ".join(columns)) diff --git a/plain-postgres/plain/postgres/query.py b/plain-postgres/plain/postgres/query.py index 521dfccb3e..63e30b29fd 100644 --- a/plain-postgres/plain/postgres/query.py +++ b/plain-postgres/plain/postgres/query.py @@ -7,7 +7,7 @@ import copy import operator import warnings -from collections.abc import Callable, Iterator, Sequence +from collections.abc import Callable, Iterable, Iterator, Sequence from functools import cached_property from itertools import islice from typing import TYPE_CHECKING, Any, Never, Self, cast, overload @@ -687,25 +687,49 @@ def bulk_create( return objs - def _check_bulk_upsert_options( - self, - update_fields: list[Field], - unique_fields: list[Field], + def _validate_upsert_unique_fields( + self, unique_field_names: Sequence[str | None], *, operation_name: str ) -> None: - model_name = self.model.__name__ + """Require unique_fields, and that they match a usable model constraint. - if not unique_fields: - raise ValueError("bulk_upsert() requires unique_fields.") + Shared by upsert() and bulk_upsert(); operation_name names the caller in + the error messages. + """ + if not unique_field_names: + raise ValueError(f"{operation_name}() requires unique_fields.") if not self.model.model_options.unique_fields_match_constraint( - {f.name for f in unique_fields} + set(unique_field_names) ): - names = [f.name for f in unique_fields] raise ValueError( - f"bulk_upsert() unique_fields {names} on {model_name} must name " - "the primary key or a UniqueConstraint declared on the model " - "without a condition or expressions." + f"{operation_name}() unique_fields {unique_field_names} on " + f"{self.model.__name__} must name the primary key or a " + "UniqueConstraint declared on the model without a condition or " + "expressions." + ) + + def _reject_null_upsert_key( + self, field: Field, value: Any, *, operation_name: str + ) -> None: + """Reject a null value for a conflict key field. + + Shared by upsert() and bulk_upsert(); NULL never conflicts in Postgres, + so a null unique-field value can't be upserted. + """ + if value is None: + raise ValueError( + f"{operation_name}() requires a non-null {field.name}; NULL never " + "conflicts in Postgres, so it cannot be upserted." ) + def _check_bulk_upsert_options( + self, + update_fields: list[Field], + unique_fields: list[Field], + ) -> None: + self._validate_upsert_unique_fields( + [f.name for f in unique_fields], operation_name="bulk_upsert" + ) + if not update_fields: raise ValueError("bulk_upsert() requires update_fields.") if any(not f.concrete for f in update_fields): @@ -758,12 +782,7 @@ def bulk_upsert( key = [] for field in unique_fields_objs: value = field.value_from_object(obj) - if value is None: - raise ValueError( - f"bulk_upsert() requires a non-null {field.name} on every " - "object; NULL never conflicts in Postgres, so it cannot " - "be upserted." - ) + self._reject_null_upsert_key(field, value, operation_name="bulk_upsert") key.append(value) keyed.append((tuple(key), obj)) @@ -943,16 +962,7 @@ def upsert( """ meta = self.model._model_meta - if not unique_fields: - raise ValueError("upsert() requires unique_fields.") - if not self.model.model_options.unique_fields_match_constraint( - set(unique_fields) - ): - raise ValueError( - f"upsert() unique_fields {unique_fields} on {self.model.__name__} " - "must name the primary key or a UniqueConstraint declared on the " - "model without a condition or expressions." - ) + self._validate_upsert_unique_fields(unique_fields, operation_name="upsert") defaults = defaults or {} create_defaults = create_defaults or {} @@ -965,14 +975,16 @@ def upsert( for source in (create_defaults, defaults, kwargs): insert_values.update(resolve_callables(source)) + # Reject typo'd keys with a clean FieldError before they fail later and + # more confusingly, matching get_or_create()'s validation. + self._validate_model_field_names(insert_values) + unique_field_objs = [meta.get_forward_field(name) for name in unique_fields] for field in unique_field_objs: assert field.name is not None - if insert_values.get(field.name) is None: - raise ValueError( - f"upsert() requires a non-null {field.name}; NULL never " - "conflicts in Postgres, so it cannot be upserted." - ) + self._reject_null_upsert_key( + field, insert_values.get(field.name), operation_name="upsert" + ) # The conflict-update columns: everything from kwargs and defaults that # isn't a unique field or the PK. create_defaults are insert-only, so @@ -1031,9 +1043,14 @@ def _extract_model_params( defaults = defaults or {} params = {k: v for k, v in kwargs.items() if LOOKUP_SEP not in k} params.update(defaults) + self._validate_model_field_names(params) + return params + + def _validate_model_field_names(self, names: Iterable[str]) -> None: + """Raise FieldError if any name isn't a model field or settable property.""" property_names = self.model._model_meta._property_names invalid_params = [] - for param in params: + for param in names: try: self.model._model_meta.get_field(param) except FieldDoesNotExist: @@ -1047,7 +1064,6 @@ def _extract_model_params( "', '".join(sorted(invalid_params)), ) ) - return params def first(self) -> T | None: """Return the first object of a query or None if no match is found.""" diff --git a/plain-postgres/plain/postgres/sql/compiler.py b/plain-postgres/plain/postgres/sql/compiler.py index 02c83df3f2..e6e3f1383c 100644 --- a/plain-postgres/plain/postgres/sql/compiler.py +++ b/plain-postgres/plain/postgres/sql/compiler.py @@ -183,6 +183,50 @@ def pre_sql_setup( group_by = self.get_group_by(self.select + extra_select, order_by) return extra_select, order_by, group_by + def _compile_assignment_value( + self, field: Any, val: Any + ) -> tuple[str, Sequence[Any]]: + """Compile one ``SET col = `` right-hand side to SQL plus params. + + Shared by UPDATE ... SET and INSERT ... ON CONFLICT DO UPDATE SET so a + written value can be a plain value, a model instance (for a related + field), or an expression (e.g. F("count") + 1). Returns the RHS SQL + fragment; the caller prepends the target column. + """ + if isinstance(val, ResolvableExpression): + val = val.resolve_expression(self.query, allow_joins=False, for_save=True) + if val.contains_aggregate: + raise FieldError( + "Aggregate functions are not allowed in this query " + f"({field.name}={val!r})." + ) + if val.contains_over_clause: + raise FieldError( + "Window expressions are not allowed in this query " + f"({field.name}={val!r})." + ) + elif hasattr(val, "prepare_database_save"): + if isinstance(field, RelatedField): + val = val.prepare_database_save(field) + else: + raise TypeError( + f"Tried to update field {field} with a model instance, {val!r}. " + f"Use a value compatible with {field.__class__.__name__}." + ) + val = field.get_db_prep_save(val, connection=self.connection) + + if hasattr(field, "get_placeholder"): + placeholder = field.get_placeholder(val, self, self.connection) + else: + placeholder = "%s" + if hasattr(val, "as_sql"): + sql, params = self.compile(val) + return placeholder % sql, params + elif val is not None: + return placeholder, [val] + else: + return "NULL", [] + def get_group_by( self, select: list[Any], order_by: list[Any] ) -> list[SqlWithParams]: @@ -1495,7 +1539,6 @@ def as_sql( # ty: ignore[invalid-method-override] # Returns list for internal (f.column for f in self.query.unique_fields), conflict_overrides, ) - extra_returning = "(xmax = 0)" if self.query.returning_created else "" if self.returning_fields: # Use RETURNING clause to get inserted values result.append( @@ -1503,7 +1546,12 @@ def as_sql( # ty: ignore[invalid-method-override] # Returns list for internal ) if conflict_suffix_sql: result.append(conflict_suffix_sql) - if returning := returning_columns(self.returning_fields, extra_returning): + if returning := returning_columns(self.returning_fields): + if self.query.returning_created: + # xmax is 0 on a freshly inserted row and non-zero on a row + # touched by the ON CONFLICT DO UPDATE, so (xmax = 0) is the + # created flag returned alongside the field columns. + returning += ", (xmax = 0)" result.append(returning) params = tuple(chain.from_iterable(param_rows)) + tuple(conflict_params) return [(" ".join(result), params)] @@ -1519,40 +1567,16 @@ def _conflict_override_sql(self) -> tuple[dict[str, str], list[Any]]: """Compile conflict_defaults into {column: SQL fragment} plus params. Reuses the same value-compilation as UPDATE ... SET so an override can - be a plain value or an expression (e.g. F("count") + 1, which resolves - to the target row's existing column for an atomic counter). + be a plain value, a model instance (for a related field), or an + expression (e.g. F("count") + 1, which resolves to the target row's + existing column for an atomic counter). """ overrides: dict[str, str] = {} params: list[Any] = [] for field, val in self.query.conflict_defaults.items(): - if isinstance(val, ResolvableExpression): - val = val.resolve_expression( - self.query, allow_joins=False, for_save=True - ) - if val.contains_aggregate: - raise FieldError( - "Aggregate functions are not allowed in this query " - f"({field.name}={val!r})." - ) - if val.contains_over_clause: - raise FieldError( - "Window expressions are not allowed in this query " - f"({field.name}={val!r})." - ) - val = field.get_db_prep_save(val, connection=self.connection) - if hasattr(field, "get_placeholder"): - placeholder = field.get_placeholder(val, self, self.connection) # ty: ignore[call-non-callable] - else: - placeholder = "%s" - if hasattr(val, "as_sql"): - sql, val_params = self.compile(val) - overrides[field.column] = placeholder % sql - params.extend(val_params) - elif val is not None: - overrides[field.column] = placeholder - params.append(val) - else: - overrides[field.column] = "NULL" + rhs, rhs_params = self._compile_assignment_value(field, val) + overrides[field.column] = rhs + params.extend(rhs_params) return overrides, params def execute_sql( # ty: ignore[invalid-method-override] @@ -1668,45 +1692,9 @@ def as_sql( qn = self.quote_name_unless_alias values, update_params = [], [] for field, val in query_values: - if isinstance(val, ResolvableExpression): - val = val.resolve_expression( - self.query, allow_joins=False, for_save=True - ) - if val.contains_aggregate: - raise FieldError( - "Aggregate functions are not allowed in this query " - f"({field.name}={val!r})." - ) - if val.contains_over_clause: - raise FieldError( - "Window expressions are not allowed in this query " - f"({field.name}={val!r})." - ) - elif hasattr(val, "prepare_database_save"): - if isinstance(field, RelatedField): - val = val.prepare_database_save(field) - else: - raise TypeError( - f"Tried to update field {field} with a model instance, {val!r}. " - f"Use a value compatible with {field.__class__.__name__}." - ) - val = field.get_db_prep_save(val, connection=self.connection) - - # Getting the placeholder for the field. - if hasattr(field, "get_placeholder"): - placeholder = field.get_placeholder(val, self, self.connection) - else: - placeholder = "%s" - name = field.column - if hasattr(val, "as_sql"): - sql, params = self.compile(val) - values.append(f"{qn(name)} = {placeholder % sql}") - update_params.extend(params) - elif val is not None: - values.append(f"{qn(name)} = {placeholder}") - update_params.append(val) - else: - values.append(f"{qn(name)} = NULL") + rhs, rhs_params = self._compile_assignment_value(field, val) + values.append(f"{qn(field.column)} = {rhs}") + update_params.extend(rhs_params) table = self.query.base_table result = [ f"UPDATE {qn(table)} SET", # ty: ignore[invalid-argument-type] diff --git a/plain-postgres/tests/app/examples/migrations/0021_upsertowner_upsertitem_owner.py b/plain-postgres/tests/app/examples/migrations/0021_upsertowner_upsertitem_owner.py new file mode 100644 index 0000000000..9dfce7138e --- /dev/null +++ b/plain-postgres/tests/app/examples/migrations/0021_upsertowner_upsertitem_owner.py @@ -0,0 +1,27 @@ +# Generated by Plain 0.154.0 on 2026-07-23 17:30 + +from plain import postgres +from plain.postgres import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("examples", "0020_upsertitem"), + ] + + operations = [ + migrations.CreateModel( + name="UpsertOwner", + fields=[ + ("id", postgres.PrimaryKeyField()), + ("name", postgres.TextField(max_length=100)), + ], + ), + migrations.AddField( + model_name="upsertitem", + name="owner", + field=postgres.ForeignKeyField( + allow_null=True, on_delete=postgres.CASCADE, to="examples.upsertowner" + ), + ), + ] diff --git a/plain-postgres/tests/app/examples/models/upsert.py b/plain-postgres/tests/app/examples/models/upsert.py index e33b74526d..bc9ae8b5d2 100644 --- a/plain-postgres/tests/app/examples/models/upsert.py +++ b/plain-postgres/tests/app/examples/models/upsert.py @@ -6,11 +6,21 @@ from plain.postgres import types +@postgres.register_model +class UpsertOwner(postgres.Model): + name = types.TextField(max_length=100) + + query: postgres.QuerySet[UpsertOwner] = postgres.QuerySet() + + @postgres.register_model class UpsertItem(postgres.Model): key = types.TextField(max_length=100) value = types.IntegerField(default=0) label = types.TextField(default="", required=False) + owner = types.ForeignKeyField( + UpsertOwner, on_delete=postgres.CASCADE, allow_null=True, required=False + ) query: postgres.QuerySet[UpsertItem] = postgres.QuerySet() diff --git a/plain-postgres/tests/public/test_upsert.py b/plain-postgres/tests/public/test_upsert.py index 2a08a151f3..ddeb1cf47c 100644 --- a/plain-postgres/tests/public/test_upsert.py +++ b/plain-postgres/tests/public/test_upsert.py @@ -9,8 +9,9 @@ import pytest from app.examples.models.relationships import Widget -from app.examples.models.upsert import UpsertItem +from app.examples.models.upsert import UpsertItem, UpsertOwner +from plain.postgres.exceptions import FieldError from plain.postgres.expressions import F @@ -128,3 +129,47 @@ def test_upsert_unique_fields_must_match_a_constraint(db): def test_upsert_rejects_null_unique_value(db): with pytest.raises(ValueError, match="non-null"): UpsertItem.query.upsert(key=None, unique_fields=["key"]) + + +def test_upsert_conflict_defaults_accepts_related_instance(db): + # A model instance as a conflict_defaults value exercises the related-field + # branch of assignment-value compilation (prepare_database_save). + owner = UpsertOwner(name="owner").create() + UpsertItem(key="a", value=1).create() + + obj, created = UpsertItem.query.upsert( + key="a", + value=2, + conflict_defaults={"owner": owner}, + unique_fields=["key"], + ) + + assert created is False + assert obj.owner is not None + assert obj.owner.id == owner.id + reloaded = UpsertItem.query.get(key="a") + assert reloaded.owner is not None + assert reloaded.owner.id == owner.id + + +def test_upsert_rejects_unknown_field_name(db): + with pytest.raises(FieldError, match="typo_field"): + UpsertItem.query.upsert( + key="a", defaults={"typo_field": 1}, unique_fields=["key"] + ) + + +def test_upsert_insert_is_one_statement(db, capture_queries): + with capture_queries() as queries: + UpsertItem.query.upsert(key="a", value=1, unique_fields=["key"]) + + assert len(queries) == 1 + + +def test_upsert_conflict_is_one_statement(db, capture_queries): + UpsertItem(key="a", value=1).create() + + with capture_queries() as queries: + UpsertItem.query.upsert(key="a", value=2, unique_fields=["key"]) + + assert len(queries) == 1 From e133dad342a91778bd243abab1dec31a3b61e059 Mon Sep 17 00:00:00 2001 From: Dave Gaeddert Date: Thu, 23 Jul 2026 14:25:55 -0500 Subject: [PATCH 5/6] Spell out defaults= on related-manager get_or_create() wrappers Match the explicit-keyword style the upsert() wrappers already use, instead of letting defaults ride along inside **kwargs. --- .../plain/postgres/fields/related_managers.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/plain-postgres/plain/postgres/fields/related_managers.py b/plain-postgres/plain/postgres/fields/related_managers.py index 6be3bc338b..b40bf36e67 100644 --- a/plain-postgres/plain/postgres/fields/related_managers.py +++ b/plain-postgres/plain/postgres/fields/related_managers.py @@ -205,10 +205,12 @@ def create(self, **kwargs: Any) -> T: kwargs[self.field.name] = self.instance return self.model.query.create(**kwargs) - def get_or_create(self, **kwargs: Any) -> tuple[T, bool]: + def get_or_create( + self, *, defaults: dict[str, Any] | None = None, **kwargs: Any + ) -> tuple[T, bool]: self._check_fk_val() kwargs[self.field.name] = self.instance - return self.model.query.get_or_create(**kwargs) + return self.model.query.get_or_create(defaults=defaults, **kwargs) def upsert( self, @@ -499,9 +501,13 @@ def create( return new_obj def get_or_create( - self, *, through_defaults: dict[str, Any] | None = None, **kwargs: Any + self, + *, + defaults: dict[str, Any] | None = None, + through_defaults: dict[str, Any] | None = None, + **kwargs: Any, ) -> tuple[T, bool]: - obj, created = self.model.query.get_or_create(**kwargs) + obj, created = self.model.query.get_or_create(defaults=defaults, **kwargs) # We only need to add() if created because if we got an object back # from get() then the relationship already exists. if created: From 51d6f66ae90386384987b65879f666980f02a18f Mon Sep 17 00:00:00 2001 From: Dave Gaeddert Date: Thu, 23 Jul 2026 16:03:47 -0500 Subject: [PATCH 6/6] upsert() takes field references for unique_fields QuerySet.upsert()'s unique_fields now accepts Field references (Model.field) via the shared _validate_field_refs() check; names are derived at the unique_fields_match_constraint boundary. defaults/create_defaults/ conflict_defaults/**kwargs stay string-keyed -- that's the kwargs idiom. The related-manager upsert() wrappers forward list[Field]. Migrates the internal callers: plain-sessions (Session.session_key) and plain-flags (Flag.name). --- plain-flags/plain/flags/flags.py | 2 +- plain-postgres/plain/postgres/README.md | 9 ++-- .../plain/postgres/fields/related_managers.py | 5 +- plain-postgres/plain/postgres/query.py | 22 ++++---- plain-postgres/tests/public/test_m2m.py | 12 +++-- plain-postgres/tests/public/test_upsert.py | 50 +++++++++++++------ plain-sessions/plain/sessions/core.py | 2 +- 7 files changed, 66 insertions(+), 36 deletions(-) diff --git a/plain-flags/plain/flags/flags.py b/plain-flags/plain/flags/flags.py index ebe3bc6d2f..bfc7793a74 100644 --- a/plain-flags/plain/flags/flags.py +++ b/plain-flags/plain/flags/flags.py @@ -83,7 +83,7 @@ def retrieve_or_compute_value(self) -> Any: flag_obj, _ = Flag.query.upsert( name=flag_name, defaults={"used_at": timezone.now()}, - unique_fields=["name"], + unique_fields=[Flag.name], ) if not flag_obj.enabled: diff --git a/plain-postgres/plain/postgres/README.md b/plain-postgres/plain/postgres/README.md index 7ed008f2b6..6ce1d3deab 100644 --- a/plain-postgres/plain/postgres/README.md +++ b/plain-postgres/plain/postgres/README.md @@ -434,14 +434,17 @@ post-write row, so there's no second query. flag, created = Flag.query.upsert( name="beta-dashboard", defaults={"used_at": timezone.now()}, - unique_fields=["name"], + unique_fields=[Flag.name], ) ``` +`unique_fields` takes field references (`Model.field`), like `bulk_upsert`. The +value sources below stay string-keyed — they follow the `kwargs` idiom. + Value sources: - `**kwargs` and `defaults` are applied on **both** insert and conflict-update. - `kwargs` carries the identifying values (including `unique_fields`). + `kwargs` carries the identifying values (including the unique fields). - `create_defaults` is applied on **insert only** — extras that must not change when the row already exists. - `conflict_defaults` overrides the `DO UPDATE SET` for specific columns. A value @@ -452,7 +455,7 @@ Value sources: view, created = PageView.query.upsert( path="/home", conflict_defaults={"count": F("count") + 1}, - unique_fields=["path"], + unique_fields=[PageView.path], ) ``` diff --git a/plain-postgres/plain/postgres/fields/related_managers.py b/plain-postgres/plain/postgres/fields/related_managers.py index b40bf36e67..de48629d76 100644 --- a/plain-postgres/plain/postgres/fields/related_managers.py +++ b/plain-postgres/plain/postgres/fields/related_managers.py @@ -13,6 +13,7 @@ from collections.abc import Callable, Iterable from plain.postgres.base import Model + from plain.postgres.fields.base import Field from plain.postgres.fields.related import ForeignKeyField, ManyToManyField import builtins @@ -218,7 +219,7 @@ def upsert( defaults: dict[str, Any] | None = None, create_defaults: dict[str, Any] | None = None, conflict_defaults: dict[str, Any] | None = None, - unique_fields: list[str], + unique_fields: list[Field], **kwargs: Any, ) -> tuple[T, bool]: self._check_fk_val() @@ -521,7 +522,7 @@ def upsert( defaults: dict[str, Any] | None = None, create_defaults: dict[str, Any] | None = None, conflict_defaults: dict[str, Any] | None = None, - unique_fields: list[str], + unique_fields: list[Field], **kwargs: Any, ) -> tuple[T, bool]: obj, created = self.model.query.upsert( diff --git a/plain-postgres/plain/postgres/query.py b/plain-postgres/plain/postgres/query.py index 00e877555b..50c0927591 100644 --- a/plain-postgres/plain/postgres/query.py +++ b/plain-postgres/plain/postgres/query.py @@ -932,7 +932,7 @@ def upsert( defaults: dict[str, Any] | None = None, create_defaults: dict[str, Any] | None = None, conflict_defaults: dict[str, Any] | None = None, - unique_fields: list[str], + unique_fields: list[Field], **kwargs: Any, ) -> tuple[T, bool]: """ @@ -958,13 +958,18 @@ def upsert( in the update. The merged result is not validated -- consistent with the other bulk write paths. - unique_fields must name the primary key or a UniqueConstraint declared - on the model without a condition or expressions, and every unique field - must have a non-null value (NULL never conflicts in Postgres). + unique_fields takes field references (`Model.field`) and must name the + primary key or a UniqueConstraint declared on the model without a + condition or expressions; every unique field must have a non-null value + (NULL never conflicts in Postgres). The value sources above stay + string-keyed -- they follow the kwargs idiom, not field references. """ meta = self.model._model_meta - self._validate_upsert_unique_fields(unique_fields, operation_name="upsert") + self._validate_field_refs(unique_fields, where="upsert() unique_fields") + self._validate_upsert_unique_fields( + [f.name for f in unique_fields], operation_name="upsert" + ) defaults = defaults or {} create_defaults = create_defaults or {} @@ -981,8 +986,7 @@ def upsert( # more confusingly, matching get_or_create()'s validation. self._validate_model_field_names(insert_values) - unique_field_objs = [meta.get_forward_field(name) for name in unique_fields] - for field in unique_field_objs: + for field in unique_fields: assert field.name is not None self._reject_null_upsert_key( field, insert_values.get(field.name), operation_name="upsert" @@ -991,7 +995,7 @@ def upsert( # The conflict-update columns: everything from kwargs and defaults that # isn't a unique field or the PK. create_defaults are insert-only, so # they never appear here. - unique_field_names = {f.name for f in unique_field_objs} + unique_field_names = {f.name for f in unique_fields} update_names = { name for name in (*kwargs, *defaults) if name not in unique_field_names } @@ -1022,7 +1026,7 @@ def upsert( returning_fields=returning_fields, on_conflict=OnConflict.UPDATE, update_fields=update_field_objs, - unique_fields=unique_field_objs, + unique_fields=unique_fields, conflict_defaults=conflict_default_objs, returning_created=True, ) diff --git a/plain-postgres/tests/public/test_m2m.py b/plain-postgres/tests/public/test_m2m.py index cdfbe15f4d..5588aa686d 100644 --- a/plain-postgres/tests/public/test_m2m.py +++ b/plain-postgres/tests/public/test_m2m.py @@ -32,8 +32,12 @@ def test_create_unique_constraint(db): def test_upsert_unique_constraint(db): - Widget.query.upsert(name="Toyota", size="Tundra", unique_fields=["name", "size"]) - Widget.query.upsert(name="Toyota", size="Tundra", unique_fields=["name", "size"]) + Widget.query.upsert( + name="Toyota", size="Tundra", unique_fields=[Widget.name, Widget.size] + ) + Widget.query.upsert( + name="Toyota", size="Tundra", unique_fields=[Widget.name, Widget.size] + ) assert Widget.query.count() == 1 @@ -44,7 +48,7 @@ def test_m2m_manager_upsert_adds_relationship_on_insert(db): """ widget = Widget.query.create(name="Tesla", size="Model 3") - tag, created = widget.tags.upsert(name="GPS", unique_fields=["name"]) + tag, created = widget.tags.upsert(name="GPS", unique_fields=[Tag.name]) assert created is True assert widget.tags.query.count() == 1 assert widget.tags.query.first() == tag @@ -57,7 +61,7 @@ def test_m2m_manager_upsert_does_not_readd_on_conflict(db): # The tag already exists and is already related; upsert must not create a # duplicate through-row. - tag, created = widget.tags.upsert(name="GPS", unique_fields=["name"]) + tag, created = widget.tags.upsert(name="GPS", unique_fields=[Tag.name]) assert created is False assert tag.id == gps.id assert widget.tags.query.count() == 1 diff --git a/plain-postgres/tests/public/test_upsert.py b/plain-postgres/tests/public/test_upsert.py index ddeb1cf47c..e89d4984be 100644 --- a/plain-postgres/tests/public/test_upsert.py +++ b/plain-postgres/tests/public/test_upsert.py @@ -16,7 +16,9 @@ def test_upsert_inserts_new_row(db): - obj, created = UpsertItem.query.upsert(key="a", value=1, unique_fields=["key"]) + obj, created = UpsertItem.query.upsert( + key="a", value=1, unique_fields=[UpsertItem.key] + ) assert created is True assert obj.id is not None @@ -29,7 +31,9 @@ def test_upsert_updates_conflicting_row(db): UpsertItem(key="a", value=1).create() existing_id = UpsertItem.query.get(key="a").id - obj, created = UpsertItem.query.upsert(key="a", value=99, unique_fields=["key"]) + obj, created = UpsertItem.query.upsert( + key="a", value=99, unique_fields=[UpsertItem.key] + ) assert created is False # The updated row keeps its primary key, and obj carries the merged value. @@ -41,12 +45,12 @@ def test_upsert_updates_conflicting_row(db): def test_upsert_defaults_apply_on_insert_and_update(db): obj, created = UpsertItem.query.upsert( - key="a", defaults={"value": 5}, unique_fields=["key"] + key="a", defaults={"value": 5}, unique_fields=[UpsertItem.key] ) assert (created, obj.value) == (True, 5) obj, created = UpsertItem.query.upsert( - key="a", defaults={"value": 7}, unique_fields=["key"] + key="a", defaults={"value": 7}, unique_fields=[UpsertItem.key] ) assert (created, obj.value) == (False, 7) @@ -56,7 +60,7 @@ def test_upsert_create_defaults_apply_on_insert_only(db): key="a", defaults={"value": 1}, create_defaults={"label": "created"}, - unique_fields=["key"], + unique_fields=[UpsertItem.key], ) assert (created, obj.label) == (True, "created") @@ -66,7 +70,7 @@ def test_upsert_create_defaults_apply_on_insert_only(db): key="a", defaults={"value": 2}, create_defaults={"label": "ignored-on-conflict"}, - unique_fields=["key"], + unique_fields=[UpsertItem.key], ) assert created is False assert obj.label == "created" @@ -80,7 +84,7 @@ def test_upsert_conflict_defaults_increment_counter_atomically(db): key="a", value=0, # the value the INSERT would have proposed (ignored on conflict) conflict_defaults={"value": F("value") + 1}, - unique_fields=["key"], + unique_fields=[UpsertItem.key], ) assert created is False @@ -95,7 +99,7 @@ def test_upsert_conflict_defaults_apply_on_insert_uses_inserted_value(db): key="a", value=3, conflict_defaults={"value": F("value") + 100}, - unique_fields=["key"], + unique_fields=[UpsertItem.key], ) assert (created, obj.value) == (True, 3) @@ -104,10 +108,10 @@ def test_upsert_all_unique_fields_is_idempotent(db): # When every inserted column is a unique field there's nothing to update; # the second call must still return the existing row (created=False). obj1, created1 = Widget.query.upsert( - name="Toyota", size="Tundra", unique_fields=["name", "size"] + name="Toyota", size="Tundra", unique_fields=[Widget.name, Widget.size] ) obj2, created2 = Widget.query.upsert( - name="Toyota", size="Tundra", unique_fields=["name", "size"] + name="Toyota", size="Tundra", unique_fields=[Widget.name, Widget.size] ) assert created1 is True @@ -123,12 +127,26 @@ def test_upsert_requires_unique_fields(db): def test_upsert_unique_fields_must_match_a_constraint(db): with pytest.raises(ValueError, match="must name the primary key"): - UpsertItem.query.upsert(key="a", value=1, unique_fields=["value"]) + UpsertItem.query.upsert(key="a", value=1, unique_fields=[UpsertItem.value]) def test_upsert_rejects_null_unique_value(db): with pytest.raises(ValueError, match="non-null"): - UpsertItem.query.upsert(key=None, unique_fields=["key"]) + UpsertItem.query.upsert(key=None, unique_fields=[UpsertItem.key]) + + +def test_upsert_string_unique_field_rejected(db): + with pytest.raises(TypeError, match="takes field references, not strings"): + UpsertItem.query.upsert( + key="a", + value=1, + unique_fields=["key"], # ty: ignore[invalid-argument-type] + ) + + +def test_upsert_wrong_model_unique_field_rejected(db): + with pytest.raises(FieldError, match="belongs to a different model"): + UpsertItem.query.upsert(key="a", value=1, unique_fields=[Widget.name]) def test_upsert_conflict_defaults_accepts_related_instance(db): @@ -141,7 +159,7 @@ def test_upsert_conflict_defaults_accepts_related_instance(db): key="a", value=2, conflict_defaults={"owner": owner}, - unique_fields=["key"], + unique_fields=[UpsertItem.key], ) assert created is False @@ -155,13 +173,13 @@ def test_upsert_conflict_defaults_accepts_related_instance(db): def test_upsert_rejects_unknown_field_name(db): with pytest.raises(FieldError, match="typo_field"): UpsertItem.query.upsert( - key="a", defaults={"typo_field": 1}, unique_fields=["key"] + key="a", defaults={"typo_field": 1}, unique_fields=[UpsertItem.key] ) def test_upsert_insert_is_one_statement(db, capture_queries): with capture_queries() as queries: - UpsertItem.query.upsert(key="a", value=1, unique_fields=["key"]) + UpsertItem.query.upsert(key="a", value=1, unique_fields=[UpsertItem.key]) assert len(queries) == 1 @@ -170,6 +188,6 @@ def test_upsert_conflict_is_one_statement(db, capture_queries): UpsertItem(key="a", value=1).create() with capture_queries() as queries: - UpsertItem.query.upsert(key="a", value=2, unique_fields=["key"]) + UpsertItem.query.upsert(key="a", value=2, unique_fields=[UpsertItem.key]) assert len(queries) == 1 diff --git a/plain-sessions/plain/sessions/core.py b/plain-sessions/plain/sessions/core.py index 3733c0b3f9..675813437b 100644 --- a/plain-sessions/plain/sessions/core.py +++ b/plain-sessions/plain/sessions/core.py @@ -176,7 +176,7 @@ def save(self) -> None: "expires_at": timezone.now() + timedelta(seconds=settings.SESSION_COOKIE_AGE), }, - unique_fields=["session_key"], + unique_fields=[self._model.session_key], ) if created: