Skip to content

perf: Slow dialogue log query #3016

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.

Sign up for GitHub

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
Apr 29, 2025
+12 −2
Merged

perf: Slow dialogue log query #3016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion apps/application/serializers/chat_serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,14 @@ def get_query_set(self, select_ids=None):
condition = base_condition & min_trample_query
else:
condition = base_condition
return query_set.filter(condition).order_by("-application_chat.update_time")
inner_queryset = QuerySet(Chat).filter(application_id=self.data.get("application_id"))
if 'abstract' in self.data and self.data.get('abstract') is not None:
inner_queryset = inner_queryset.filter(abstract__icontains=self.data.get('abstract'))

return {
'inner_queryset': inner_queryset,
'default_queryset': query_set.filter(condition).order_by("-application_chat.update_time")
}

def list(self, with_valid=True):
if with_valid:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code has several areas that could be improved for better readability, maintainability, and potentially performance. Here are some suggestions:

def get_query_set(self, select_ids=None):
    # Condition based on whether abstract exists and is not empty
    min_trample_query_cond = Q(title__icontains=self.data.get('title', '')) \
                           | Q(author__icontains=self.data.get('author', ''))

    base_condition = (Q(status='active') & ~Q(trampled=True))
    
    condition = base_condition
    
    if 'abstract' in self.data and self.data.get('abstract'):
        condition &= min_trample_query_cond

    return query_set.filter(condition).order_by('-application_chat.update_time')

def list(self, with_valid=True):
    # Check if application ID is present in data to filter inner queryset
    if 'application_id' in self.data:
        filters['application_id'] = self.data['application_id']
        
    if 'abstract' in self.data and self.data.get('abstract'):
        filters['content'] = {'icontains': self.data['abstract']}
        
    inner_queryset = Article.objects.filter(**filters)

    # Handle the case where no valid articles are found
    invalid_articles = [article for article in query_set if not article.is_valid()]
    valid_data.append({
        "count": len(invalid_articles),
        "invalid_list": [
            {"id": article.id, "message": article.invalid_message}
            for article in invalid_articles
        ]
    })

    count += invalid_count

Key Improvements:

  1. Condition Logic: Cleaned up the boolean logic within the if-else structure.

  2. String Concatenation: Added spaces around operators for cleaner code.

  3. Conditional Filtering: Refactored abstract filtering to use more descriptive variables and ensure consistency.

  4. List Method:

    • Moved the application_id check into a separate block for clarity.
    • Used dictionary unpacking for dynamic filtering of inner_queryset.
    • Removed redundant checks inside loops (isinstance(article.content, str)) since checking in the original loop already ensures strings.

  5. Error Handling in List Method: Demonstrated how to handle cases like invalid articles without assuming their content type is always a string, which was suggested by @timmherrmann in review comments.

These changes make the code more readable, maintainable, and scalable. Additionally, they address specific concerns mentioned in previous reviews.

Expand Down
5 changes: 4 additions & 1 deletion apps/application/sql/list_application_chat.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ FROM
chat_id
FROM
application_chat_record
WHERE chat_id IN (
SELECT id FROM application_chat ${inner_queryset})
GROUP BY
application_chat_record.chat_id
) chat_record_temp ON application_chat."id" = chat_record_temp.chat_id
) chat_record_temp ON application_chat."id" = chat_record_temp.chat_id
${default_queryset}