Skip to content
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

initial prototype for returning clause #203

Closed
wants to merge 1 commit into from
Closed

Conversation

dantownsend
Copy link
Member

@dantownsend dantownsend commented Aug 28, 2021

Fixes #171

As pointed out in the above issue, there's no way to customise what the insert method returns.

We currently just return the primary key for the inserted rows.

>>> Manager.insert(Manager(name="Bob"), Manager(name="Sally")).run_sync()
[{'id': 3}, {'id': 4}]

Being able to customise what gets returned is a nice usability improvement, and in some cases could save another SELECT query.

For example:

>>> Manager.insert(Manager(name="Bob"), Manager(name="Sally"), returning=[Manager.id, Manager.name]).run_sync()
[{'id': 3, 'name': 'Bob'}, {'id': 4, 'name': 'Sally'}]

The implementation is fairly straightforward with Postgres, but SQLite only supports the RETURNING clause in fairly recent versions (>= 3.35.0), so needs more consideration:

https://www.sqlite.org/lang_returning.html

We could also potentially add a returns_objects argument, which would returns the new rows as Table instances instead of a list of dictionaries.

Verified

This commit was signed with the committer’s verified signature.
sagikazarmark Márk Sági-Kazár
@codecov-commenter
Copy link

codecov-commenter commented Aug 28, 2021

Codecov Report

Merging #203 (1092474) into master (ceb12d9) will decrease coverage by 0.03%.
The diff coverage is 81.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #203      +/-   ##
==========================================
- Coverage   90.48%   90.44%   -0.04%     
==========================================
  Files         114      114              
  Lines        5358     5367       +9     
==========================================
+ Hits         4848     4854       +6     
- Misses        510      513       +3     
Impacted Files Coverage Δ
piccolo/query/mixins.py 93.95% <75.00%> (-0.53%) ⬇️
piccolo/query/methods/insert.py 94.73% <80.00%> (-5.27%) ⬇️
piccolo/table.py 94.38% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ceb12d9...1092474. Read the comment docs.

@dantownsend
Copy link
Member Author

Replaced by #564

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.

Insert & get
2 participants