#31301 closed Bug (fixed)
bulk_create with ForeignKey fields with mixed filled/None values fails.
Reported by: | Hans Aarne Liblik | Owned by: | Hans Aarne Liblik |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | oracle sql |
Cc: | Marti Raudsepp | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
I have a model with ForeignKey (null=True) to another model
class A(models.Model): name = models.CharField() class B(models.Model): name = models.CharField() a = models.ForeignKey(A, on_delete=models.CASCADE, null=True)
When I try to use bulk_create as follows
B.objects.bulk_create([ B(name='first', a_id=1) B(name='seconds', a_id=None) ])
the query fails with error
django.db.utils.DatabaseError: ORA-01790: expression must have same datatype as corresponding expression
I got bulk_create working if the ForeignKey references are either All None or All NOT None. It seems like the fix introduced in https://code.djangoproject.com/ticket/22669 fixes the case for NumberFields and CharFields, but ForeignKey field 'target_field' for me is 'AutoField' which is not defined in
django/db/backends/oracle/utils.py#52 class BulkInsertMapper:
Manually editing that this file and adding
class BulkInsertMapper: ... types: { ... ... 'AutoField': NUMBER }
fixes the issue for me.
Change History (21)
comment:1 by , 5 years ago
Easy pickings: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 5 years ago
Replying to Hans Aarne Liblik:
Any progress on this? If not, I can take over.
It is looking harder than I expected. I am just a newbie. I will come back later. Feel free to take over
comment:5 by , 5 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:6 by , 5 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:7 by , 5 years ago
Cc: | added |
---|
comment:9 by , 5 years ago
Has patch: | set |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:10 by , 5 years ago
Triage Stage: | Ready for checkin → Accepted |
---|
comment:11 by , 5 years ago
Patch needs improvement: | set |
---|
comment:12 by , 5 years ago
Needs tests: | set |
---|
comment:13 by , 5 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
Summary: | bulk_create with ForeignKey fields with mixed filled/None values fails → bulk_create with ForeignKey fields with mixed filled/None values fails. |
Triage Stage: | Accepted → Ready for checkin |
Version: | 3.0 → master |
comment:16 by , 5 years ago
No, this doesn't qualify for a backport, see https://docs.djangoproject.com/en/3.0/internals/release-process/ for more details.
comment:17 by , 5 years ago
@felixxm, are you sure this doesn't qualify?
It seems like a significant issue to me; the backport criteria does list "Crashing bugs".
If I understand correctly, it causes different database backends to behave differently? Which is always a bad surprise.
comment:18 by , 5 years ago
It doesn't qualify for a backport from the same reason that #22669 didn't qualify. It's not a regression it existed since 9fe050a27598cdce7f2ef7a332b4818b493a8702 (Django 1.4) so it's not crucial.
follow-up: 21 comment:19 by , 5 years ago
What's your suggested solution for users who are hitting this bug now? I can think of:
- Fork Django, backport the fix manually, and continue to maintain the fork until Django 3.1.0 is released
- Upgrade to Django development "master" branch, which sometimes breaks third-party Django addons
- Stop using Django until August 2020 rolls around (just sarcastic, sorry!)
Am I missing something? This is behavior that everyone agrees is a bug, and a helpful user provided pull request to fix it. Yet you are leaving users hanging without a good solution for ~5 months.
Seems like a bug in the backport policy itself :)
comment:20 by , 5 years ago
I'm sorry Marti but that's our policy. We're fixing dozens of bugs in each month and only few of them are backported. We don't backport all bugfixes to minimize the risk of regressions.
comment:21 by , 5 years ago
Replying to Marti Raudsepp:
- Fork Django, backport the fix manually, and continue to maintain the fork until Django 3.1.0 is released and upgrading is viable
I think that's clearly the way to go if the fix is critical to your application. You may have to update your fork once a month if Django periodic minor release contains security fixes, but that's a price to pay for Django general stability where you can almost blindly apply minor upgrades without worrying. And even with this strict backport policy, it has happened in the past that such fixes have introduced regressions, so I don't think we should relax this policy.
That seems like the correct approach. Would you be interested in submitting a Github PR that addresses the issue?
Based on c4e2fc5d9872c9a0c9c052a2e124f8a9b87de9b4 adding a nullable foreign key on
NullableFields
to another test model of the same module should be sufficient to augment thetest_bulk_insert_nullable_fields
test to cover this case.