Opened 15 years ago
Last modified 6 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 |
Pull Requests: | |||
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):
- discussed in http://groups.google.com/group/django-developers/browse_thread/thread/22e52a2fd03eac82
- someone proposed a patch at http://github.com/aparo/django/commit/edcdc1d9364224fcbc3b810b9d9fa19a10cd537c (only the fields/related part imho)
According to the ticket's flags, the next step(s) to move this issue forward are:
- To add tests to the patch, then uncheck the "Needs tests" flag on the ticket.
- To improve the patch as described in the pull request review comments or on this ticket, then uncheck "Patch needs improvement".
If creating a new pull request, include a link to the pull request in the ticket comment when making that update. The usual format is:
[https://github.com/django/django/pull/#### PR]
.
Change History (17)
comment:1 by , 15 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|---|
milestone: | → 1.3 |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 14 years ago
Cc: | added |
---|---|
Owner: | changed from | to
by , 14 years ago
Attachment: | 13677.diff added |
---|
comment:3 by , 14 years ago
Has patch: | set |
---|
comment:4 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:5 by , 14 years ago
Easy pickings: | unset |
---|---|
Patch needs improvement: | set |
comment:12 by , 10 years ago
Component: | Database layer (models, ORM) → Forms |
---|---|
Summary: | wrong backend with multidb and modelformset → ModelFormSet may query wrong database backend |
comment:13 by , 8 years ago
Cc: | added |
---|---|
Has patch: | unset |
Patch needs improvement: | unset |
Resolution: | → worksforme |
Status: | new → closed |
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 , 8 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 , 8 years ago
Has patch: | set |
---|---|
Resolution: | worksforme |
Status: | closed → new |
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 , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:17 by , 8 years ago
Patch needs improvement: | set |
---|
I left some comments for improvement on the PR.
comment:18 by , 6 years ago
Patch needs improvement: | unset |
---|
comment:19 by , 6 years ago
Version: | 1.2 → master |
---|
comment:20 by , 6 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
13677.diff fails to apply cleanly on to trunk