0

So I have this django query that I am making on one of my models and I have some extra python code that is used to do additional work in between each query. I am looking for some assistance in turning this chunk of code into a single django query as I know you should be making the database do most of the work.

The query code is below.

keywords = []
if query:
   results = []
   keywords = query.split()
   for x in keywords:
      res = Textbook.objects.filter(Q(class_name__icontains=x) |
                                    Q(textbook_name__icontains=x) |
                                    Q(author__icontains=x) |
                                    Q(isbn__icontains=x))
      if len(results) == 0:
         results = res
      else:
         results = set(results) & set(res)

numresults = len(results)

So query is the information I take from the user. I split up this information into keywords and make a query for each keyword. They are searching for textbooks so if any word they enter matches the isbn, title, class name, or author of a book it is added to the results. However, if they used multiple words I only add it to the results if both words return the book as a query. That's what results = set(results) & set(res) does. If the current results and the new query return the same book keep it in the results.

I know this is bad so I am looking for a way to improve it.

1 Answer 1

8

You can do additional filters in the loop instead of set-intersection:

keywords = []
if query:
    results = []
    keywords = query.split()
    query = Textbook.objects.all()
    for x in keywords:
        query = query.filter(Q(class_name__icontains = x) | Q(textbook_name__icontains = x) | Q(author__icontains = x) | Q(isbn__icontains = x))

numresults = len(query)

Since Django's QuerySets are lazy, SQL call should be deffered until numresults = len(query) line

8
  • Is not better to use query.count()?
    – Gocht
    Oct 6, 2015 at 21:29
  • This will filter results in a different way than Joe does. If you query 'foo bar' you get only results that contain 'foo' AND 'bar'. Joe will have results that have 'foo' OR 'bar'.
    – allcaps
    Oct 6, 2015 at 21:32
  • 1
    @Gocht: I asumed that numresults is just a side effect, not the main reason this query is called and thus according to Django docs: "Note that if you want the number of items in a QuerySet and are also retrieving model instances from it (for example, by iterating over it), it’s probably more efficient to use len(queryset) which won’t cause an extra database query like count() would."
    – Ritave
    Oct 6, 2015 at 21:34
  • 1
    @OlafTomalka: I accidentally down voted your answer, can you edit your answer so I can cancel my action? Stackoverflow doesn't allow me to undo the vote if it's made 5 minutes ago and the answer is not edited. Also, you might call & intersection instead of union.
    – Shang Wang
    Oct 6, 2015 at 21:46
  • 1
    @ShangWang You're right - I changed union to correct one: intersection. Thanks.
    – Ritave
    Oct 6, 2015 at 21:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.