Code

Opened 4 years ago

Closed 4 years ago

Last modified 17 months ago

#12869 closed (duplicate)

SELECT (1) AS [a] FROM [my_table] WHERE ([my_table].[id] = ? AND NOT ([my_table].[id] = ? )) (1, 1)

Reported by: kmt@… Owned by:
Component: Database layer (models, ORM) Version: 1.1
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Why is Django executing statements such as this:

    SELECT (1) AS [a] FROM [my_table] 
    WHERE ([my_table].[id] = ?  
    AND NOT ([my_table].[id] = ? )) (1, 1)

This happens when calling is_valid() on a formset created the following way:

    MyFormSet = modelformset_factory(Table, fields=['my_field'], extra=0)
    my_form_set = MyFormSet(request.POST,
                            queryset=Table.objects.all())

where Table and MyForm are as simple as, say:

    class Table(models.Model):
        my_field = models.CharField(max_length=10)
    
    class MyForm(forms.ModelForm):
        class Meta:
            model = Table

Hint: I looked at the call stack and the code responsible for it (in django/forms/models.py) is below:


   def _perform_unique_checks(self, unique_checks):
        import pdb; pdb.set_trace()
        bad_fields = set()
        form_errors = []

        for unique_check in unique_checks:
            # Try to look up an existing object with the same values as this
            # object's values for all the unique field.

            lookup_kwargs = {}
            for field_name in unique_check:
                lookup_value = self.cleaned_data[field_name]
                # ModelChoiceField will return an object instance rather than
                # a raw primary key value, so convert it to a pk value before
                # using it in a lookup.
                if isinstance(self.fields[field_name], ModelChoiceField):
                    lookup_value =  lookup_value.pk
                lookup_kwargs[str(field_name)] = lookup_value

            qs = self.instance.__class__._default_manager.filter(**lookup_kwargs)

            # Exclude the current object from the query if we are editing an
            # instance (as opposed to creating a new one)
            if self.instance.pk is not None:
                qs = qs.exclude(pk=self.instance.pk)

Basically the pk is both included for the uniqueness check and excluded. Looks like Django can be smarter and avoid such inefficiency.

Attachments (0)

Change History (2)

comment:1 Changed 4 years ago by kmtracey

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to duplicate
  • Status changed from new to closed

This problem has been fixed in current trunk, I'd guess by the fix for #12869. (As the fix was done as part of adding new function it was not backported to 1.1.X branch.)

comment:2 Changed 17 months ago by akaariai

  • Component changed from ORM aggregation to Database layer (models, ORM)

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.