Opened 14 years ago

Last modified 5 years ago

#13677 assigned Bug

ModelFormSet may query wrong database backend

Reported by: Pierre Chifflier Owned by: Tobias Kunze
Component: Forms Version: dev
Severity: Normal Keywords:
Cc: dgouldin@…, rixx@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

When using a ModelFormSet with a queryset using a DB different from the 'default' one, the validation of the fields fails.

debugging the code shows that the db used is the wrong one:

-> self.instance.clean_fields(exclude=exclude)
  /usr/lib/pymodules/python2.5/django/db/models/base.py(902)clean_fields()
-> setattr(self, f.attname, f.clean(raw_value, self))
  /usr/lib/pymodules/python2.5/django/db/models/fields/__init__.py(194)clean()
-> self.validate(value, model_instance)
  /usr/lib/pymodules/python2.5/django/db/models/fields/related.py(831)validate()
-> qs = self.rel.to._default_manager.filter(**{self.rel.field_name:value})
> /usr/lib/pymodules/python2.5/django/db/models/manager.py(141)filter()
-> return self.get_query_set().filter(*args, **kwargs)
(Pdb) p self.get_query_set().db
'default'

The ModelFormSet was created by creating Form with a queryset:

def make_projecttask_formset(p, extra=0):
    class _ProjectTaskForm(forms.ModelForm):
        project = forms.ModelChoiceField(label="Project",
                queryset=Project.objects.using('otherdb').filter(project_id=p.project_id))
...
    return modelformset_factory(ew_projecttask, form=_ProjectTaskForm, extra=extra)

Expected behavior: used db should be the same as in the queryset

Note: it seems the problems is known (bug not fixed):

Attachments (1)

13677.diff (3.1 KB ) - added by David Gouldin 14 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 by Russell Keith-Magee, 14 years ago

Component: UncategorizedDatabase layer (models, ORM)
milestone: 1.3
Triage Stage: UnreviewedAccepted

comment:2 by David Gouldin, 14 years ago

Cc: dgouldin@… added
Owner: changed from nobody to David Gouldin

by David Gouldin, 14 years ago

Attachment: 13677.diff added

comment:3 by David Gouldin, 14 years ago

Has patch: set

comment:4 by Julien Phalip, 13 years ago

Severity: Normal
Type: Bug

comment:5 by patchhammer, 13 years ago

Easy pickings: unset
Patch needs improvement: set

13677.diff fails to apply cleanly on to trunk

comment:6 by Jacob, 13 years ago

milestone: 1.3

Milestone 1.3 deleted

comment:11 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:12 by Tim Graham, 9 years ago

Component: Database layer (models, ORM)Forms
Summary: wrong backend with multidb and modelformsetModelFormSet may query wrong database backend

comment:13 by Tobias Kunze, 7 years ago

Cc: rixx@… added
Has patch: unset
Patch needs improvement: unset
Resolution: worksforme
Status: newclosed

I tried to build a ModelFormSet that uses the alternative database, and as long as you specify for related objects to use the alternative database, too, the validation is successful. (If you forget to specify the second database, it throws a very explicit error stating that this model relation is not possible).

My test case is avaliable here: https://github.com/rixx/django_ticket13667 - please provide a failing example if you want to reopen this ticket.

comment:14 by Tim Graham, 7 years ago

Just to be sure, could you clarify what the mistake is in the test in the patch attached to the ticket (13677.diff​)?

comment:15 by Tobias Kunze, 7 years ago

Has patch: set
Resolution: worksforme
Status: closednew

I'm sorry: I made a mistake in reproducing the issue and porting the patch to current Django. After porting the patch I can confirm that the regression test is valid. I'm not quite sure the implementation is what we want, but I'll push it and we can discuss the PR.

comment:16 by Tobias Kunze, 7 years ago

Owner: changed from David Gouldin to Tobias Kunze
Status: newassigned
Last edited 7 years ago by Tim Graham (previous) (diff)

comment:17 by Tim Graham, 7 years ago

Patch needs improvement: set

I left some comments for improvement on the PR.

comment:18 by Tobias Kunze, 5 years ago

Patch needs improvement: unset

comment:19 by Tobias Kunze, 5 years ago

Version: 1.2master

comment:20 by Mariusz Felisiak, 5 years ago

Needs tests: set
Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top