piccolo icon indicating copy to clipboard operation
piccolo copied to clipboard

Sub queries for `is_in` and `not_in`

Open dantownsend opened this issue 2 years ago • 6 comments

Resolves https://github.com/piccolo-orm/piccolo/issues/785

dantownsend avatar Mar 06 '23 18:03 dantownsend

@dantownsend Sorry to bother, but I wonder why you didn't merge this (I know the tests are missing) because I think it's really useful and convenient, and the performance is approx. 50% better on a GET request because we don't have to make two separate queries but just one?

sinisaos avatar Jun 20 '23 08:06 sinisaos

@sinisaos Yeah, it's a good point. Like you say, it just needs test and docs, and they shouldn't take too long to add. I'll try and finish it up and get it merged.

dantownsend avatar Jun 20 '23 09:06 dantownsend

@dantownsend Great. Thanks.

sinisaos avatar Jun 20 '23 09:06 sinisaos

Codecov Report

Merging #786 (4643993) into master (29f4c2f) will decrease coverage by 0.07%. The diff coverage is 60.00%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #786      +/-   ##
==========================================
- Coverage   91.62%   91.56%   -0.07%     
==========================================
  Files         109      109              
  Lines        7885     7897      +12     
==========================================
+ Hits         7225     7231       +6     
- Misses        660      666       +6     
Impacted Files Coverage Δ
piccolo/columns/combination.py 90.90% <57.14%> (-2.57%) :arrow_down:
piccolo/columns/base.py 94.04% <62.50%> (-0.82%) :arrow_down:

codecov-commenter avatar Jun 22 '23 20:06 codecov-commenter

Can we perhaps make it so that piccolo handles the case where we put in an empty list? It can be useful when the list is dynamically generated, and there are different conditions in the where clause. Maybe this would work?

powellnorma avatar Jul 20 '23 20:07 powellnorma

@powellnorma I usually have to add a check before any query with an is_in to make sure the list isn't empty. It has tripped me up a few times.

dantownsend avatar Jul 21 '23 11:07 dantownsend