Opened 10 months ago

Closed 10 months ago

Last modified 10 months ago

#34996 closed New feature (wontfix)

Enhance update_or_create() method with upsert SQL.

Reported by: Jordan Bae Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Context

Current QuerySet.update_or_create method work like below

  1. open transaction or savepoint
  2. get_or_create with lock
  3. exist -> update and not exist - create

I want to suggest how about refactoring this with upsert SQL (ex. INSERT INTO ... ON DUPLICATE KEY UPDATE)

    def update_or_create(self, defaults=None, create_defaults=None, **kwargs):
        """
        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.
        """
        update_defaults = defaults or {}
        if create_defaults is None:
            create_defaults = update_defaults

        self._for_write = True
        with transaction.atomic(using=self.db):
            # 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
            )
            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._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. auto_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._meta.local_concrete_fields:
                    if not (
                        field.primary_key or field.__class__.pre_save is Field.pre_save
                    ):
                        update_fields.add(field.name)
                        if field.name != field.attname:
                            update_fields.add(field.attname)
                obj.save(using=self.db, update_fields=update_fields)
            else:
                obj.save(using=self.db)
        return obj, False

Strength

  • Performance: when updates, there is no need transaction and lock. and it's single query.
  • Maintenance: It can be simple for maintenance.

Consideration

  • database compatibility: need to check support upsert SQL in the all of databases.


Change History (2)

comment:1 by Mariusz Felisiak, 10 months ago

Resolution: wontfix
Status: newclosed
Summary: Enhance update_or_create method with upsert sqlEnhance update_or_create() method with upsert SQL.
Type: Cleanup/optimizationNew feature

Thanks for this ticket, however, this change would be backward incompatible as get_or_create() is documented to be a shortcut for the pattern:

try:
    <<get>>
    <<save>>
except DoesNotExist:
    <<update>>

With your proposition save() would not be called anymore (and folks have a custom logic in their save() methods). Moreover support for "UPSERT" varies in databases, and it's already possible to do this with passing a single object to the bulk_create().

comment:2 by Jordan Bae, 10 months ago

Thanks for sharing ur thought! I didn't think about backward incompatible!

Note: See TracTickets for help on using tickets.
Back to Top