Skip to content

Ensure bulk_upsert accepts any iterable for rows #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 27, 2019

Conversation

ericpalakovichcarr
Copy link
Contributor

@ericpalakovichcarr ericpalakovichcarr commented Jun 21, 2019

This updates the bulk_insert and bulk_update methods to allows rows to be any object that's an iterable (i.e. implements __getitem__ and/or __iter__).

Currently, the two methods require rows to be a list (or specifically, an object that supports indexing). I ran into this when I was using a dict to generate the rows to submit. Here's a contrived example:

    rows = {
        1: {pk: 1, val1: False, val2: False},
        4: {pk: 4, val1: False, val2: False},  
    }
    for pk in pks_where_val1_is_true:
        rows[pk]["val1"] = True
    for pk in pks_where_val2_is_true:
        rows[pk]["val2"] = True
    model.objects.bulk_upsert(
        conflict_target=["pk"], rows=rows.values(), return_model=True
    )

Since rows.values() returns a dictionary view object, it doesn't support indexing (e.g. rows[0]). The existing logic that assumes the rows argument supports indexing causes an exception. This PR removes that requirement, allowing any iterable to be passed without first being converted to a list.

I've also included tests 🎉

@Photonios
Copy link
Member

This is excellent work! Many thanks for your contribution 👍

@Photonios Photonios merged commit 08a3d1a into SectorLabs:master Jun 27, 2019
@Photonios
Copy link
Member

@utapyngo
Copy link

This fails when a generator expression is passed as rows. The first is_empty call consumes the generator, and the StopIteration exception is raised when trying to get the first row:

  File "psqlextra/manager/manager.py", line 633, in bulk_upsert
    conflict_target, rows, index_predicate, return_model
  File "psqlextra/manager/manager.py", line 336, in bulk_upsert
    return self.bulk_insert(rows, return_model)
  File "psqlextra/manager/manager.py", line 169, in bulk_insert
    compiler = self._build_insert_compiler(rows)
  File "psqlextra/manager/manager.py", line 357, in _build_insert_compiler
    first_row = next(rows_iter)
StopIteration

@ericpalakovichcarr
Copy link
Contributor Author

Thanks for catching this, @utapyngo. I created PR #135 that'll fix the issue 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants