bulk_insert() wasn't returning any objects as documented.

[lairdm]

bulk_insert() in the documentation has:
obj = (
MyModel.objects
.on_conflict(['name'], ConflictAction.UPDATE)
.bulk_insert([
...

However bulk_insert() wasn't actually returning anything. What would be useful would be to utilize the 'returning' clause already being added to the Postgres query to send back any auto inc indexes the table might have. This makes the functionality better mirror bulk_create()

In addition, when using the on_conflict() this would allow the user to see the indexes for any entry that might be updated.

As well, to make this work there was a bug in how 'returning' clause rows were being retrieved in the compiler, it was only grabbing the first entry from the cursor, not all the returned records.

Tests have been added to this patch as well as documentation.

[lairdm]

And I just noticed the other pull request, doh, sorry I didn't notice someone else already made a similar proposal. Anyhow, if there's anything I can do to help either of these solutions from being merged, please let me know. Personally, this is a very important feature for my current project, but also I can see it making the whole package more useful. Thanks again for building library.

[Photonios]

I've released this feature in Alpha 6: https://github.com/SectorLabs/django-postgres-extra/releases/tag/v21a6

[timworx]

Just saw this feature used in a PR - love it @lairdm

However, digging through the implementation I'd like to understand some of the decisions.

I want to set the stage by bringing up your first point: The docs mentioned bulk_insert returning obj when it was actually returning nothing.

However, in this implementation, instead of returning an instance of the object by default, it returns a dict. I could see an argument for that; you are passing it a list of dicts in the first place; albeit only because its a convention required by django-postgres-extra.

What makes this a bit hairier is that it falls through to Django's bulk_create which will always return a list of object instances. Arguably in Django you want the model much more often than you want a dict representation of it.

As a result, I think that not only should the model being returned be the default, I don't really think there should be another option, since this is going to return two types:

• A list of dicts if there is a conflict target or action
• An instance of the model if there is no conflict target or action

In the least, if you have to use a kwarg in order to get a dict back (rather than model), then there can also be a warning by it in the docs that you will see two different returned types, based on whether or not there is a conflict action/target.