Re: [Django] #33649: bulk_create() with ignore_conflicts=True and ForeignKey fails
Django
-------------------------------------+-------------------------------------
Reporter: Markus Friedrich | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: bulk_create | Triage Stage:
ForeignKey ignore_conflicts | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Markus Friedrich):
Thanks for you very fast answer and the provided links.
I know that ignore_conflicts=True disables setting the primary key for all
DB's and that some DB's are not able to set the primary key using
bulk_create at all.
That's why the ForeignKey of Article does not use the primary key (id) of
Reporter as to_field but used the uuid field of Reporter. This uuid field
is unique=True and its value is a UUID4 value which is even set if the
object is not already saved. This enables the use of bulk_create even for
DB's which are not able to set a primary auto inc key.
I found now that may test case even fails with the same error with >=3.2
if ignore_conflicts is removed if a DB is used which cannot set the pk in
bulk_create. (e.g. older sqlite3 (I have tested with sqlite3-3.27))
This means with this error message you have intentionally completely
removed the functionality to use bulk_create together with a ForeignKey on
all databases which are not able to set the pk in bulk_create!
If this is really intended I'm OK with it and need to workaround this.
(even thought I do not understand why there should be a data-loss problem
in this case (with a uuid as to_field of the ForeignKey))
(I also do not understand your last sentence: I do not have any already
saved reporter in my test case (but in my real case). But as detected now
it even fails without the ignore_conflicts, see above)
--
Ticket URL: <https://code.djangoproject.com/ticket/33649#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Markus Friedrich | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: bulk_create | Triage Stage:
ForeignKey ignore_conflicts | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
> This means with this error message you have intentionally completely
removed the functionality to use bulk_create together with a ForeignKey on
all databases which are not able to set the pk in bulk_create!
Why? This error message is only raised when a related object **is not
saved**. The current behavior of `bulk_create()` and `bulk_update()`
reflects the `save()` behavior.
> If this is really intended I'm OK with it and need to workaround this.
(even thought I do not understand why there should be a data-loss problem
in this case (with a uuid as to_field of the ForeignKey))
As far as I'm aware there is still a data loss possibility 🤔, because the
assigned UUID may not exists in the database at the end. We cannot
guarantee that the assigned UUID will be stored in the database as it
comes from an unsaved object.
> (I also do not understand your last sentence: I do not have any already
saved reporter in my test case (but in my real case). But as detected now
it even fails without the ignore_conflicts, see above)
My advise is to save `reporter` instances before attaching them to an
`article`.
--
Ticket URL: <https://code.djangoproject.com/ticket/33649#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Markus Friedrich | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: bulk_create | Triage Stage:
ForeignKey ignore_conflicts | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Markus Friedrich):
>> Why? This error message is only raised when a related object **is not
saved**.
> The related object **is saved** since bulk_create() is called on it.
This save is just not recognized by Django either because
> - the used database does not support setting the pk by bulk_create (as
documented) or
> - bulk_create disables the setting of the pk if ignore_conflicts=True is
used (as documented)
> So the error is wrong in this case. And in my view at least for lower
level functions like bulk_create (which are for performance tuning) no
error should be raised if its unclear for Django if there is really an
error. Its up to the caller to ensure that things are right, at least for
lower level functions.
--
Ticket URL: <https://code.djangoproject.com/ticket/33649#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Markus Friedrich | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: bulk_create | Triage Stage:
ForeignKey ignore_conflicts | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
> So the error is wrong in this case.
Rows are saved in the database, but object instances don't have primary
keys, so they are not associated with saved rows. Again, you can fix this
by re-fetching new objects from the database before assigning them to a
related model.
> And in my view at least for lower level functions like bulk_create
(which are for performance tuning) no error should be raised if its
unclear for Django if there is really an error. Its up to the caller to
ensure that things are right, at least for lower level functions.
I don't agree, `bulk_create()` and `bulk_update()` are supported and
documented APIs. I see no reason to leave users on their own in this
particular case.
You can start a discussion on DevelopersMailingList if you don't agree,
where you'll reach a wider audience and see what other think. Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/33649#comment:5>