Skip to content

Optimize Comment queries by replacing fields="*" with explicit field lists#51

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/fix-fetch-all-columns-issue
Draft

Optimize Comment queries by replacing fields="*" with explicit field lists#51
Copilot wants to merge 2 commits intomainfrom
copilot/fix-fetch-all-columns-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 14, 2026

Two Comment table queries were fetching all columns unnecessarily, transferring unused data from the database. This is inefficient when the Comment table contains large text fields (content with HTML/images).

Changes

frappe_comment_xt/helpers/comment.py:31 - add_comments_in_timeline()

  • Replaced fields="*" with explicit list of 12 fields
  • Includes: name, owner, creation, modified, comment_type, comment_email, comment_by, content, reference_doctype, reference_name, custom_visibility, custom_reply_to

frappe_comment_xt/overrides/whitelist/comment.py:130 - get_all_replies()

  • Replaced fields="*" with explicit list of 10 fields
  • Includes: name, owner, creation, modified, comment_type, comment_email, comment_by, content, custom_reply_to, custom_visibility

Example

# Before
comments = frappe.get_all(
    "Comment",
    fields="*",  # Fetches all columns
    filters={...},
)

# After
comments = frappe.get_all(
    "Comment",
    fields=[
        "name", "owner", "creation", "modified",
        "comment_type", "comment_email", "comment_by",
        "content", "reference_doctype", "reference_name",
        "custom_visibility", "custom_reply_to",
    ],
    filters={...},
)

Field lists determined by analyzing code usage patterns and frontend JavaScript dependencies.

Original prompt

This section details on the original issue you should resolve

<issue_title>fields="*" Fetches All Columns Unnecessarily</issue_title>
<issue_description>

Metadata

  • File(s):
    • frappe_comment_xt/helpers/comment.py:31
    • frappe_comment_xt/overrides/whitelist/comment.py:124-132
  • Category: Database
  • Severity: Low
  • Effort to Fix: Low
  • Estimated Performance Gain: 10-20%

Problem Description

Multiple queries use fields="*" to fetch all columns from the Comment table, when only specific fields are actually used. This transfers unnecessary data from the database.

The Comment table may have large text fields (content), and fetching all columns for many comments wastes bandwidth and memory.

Code Location

add_comments_in_timeline() - line 31:

comments = frappe.get_all(
    "Comment",
    fields="*",  # Fetches all columns
    filters={
        "reference_doctype": doc.doctype,
        "reference_name": doc.name,
        "custom_reply_to": "",
    },
)

get_all_replies() - lines 124-132:

replies = frappe.get_all(
    "Comment",
    filters={
        "reference_doctype": reference_doctype,
        "reference_name": reference_name,
    },
    fields="*",  # Fetches all columns
    order_by="creation DESC",
)

Root Cause

Developer convenience - using fields="*" is easier than specifying individual fields, but it's less efficient.

Proposed Solution

Specify only the fields that are actually used:

For add_comments_in_timeline():

comments = frappe.get_all(
    "Comment",
    fields=[
        "name", "owner", "creation", "modified",
        "comment_type", "comment_email", "comment_by",
        "content", "reference_doctype", "reference_name",
        "custom_visibility", "custom_reply_to"
    ],
    filters={
        "reference_doctype": doc.doctype,
        "reference_name": doc.name,
        "custom_reply_to": "",
    },
)

For get_all_replies():

replies = frappe.get_all(
    "Comment",
    filters={
        "reference_doctype": reference_doctype,
        "reference_name": reference_name,
    },
    fields=[
        "name", "owner", "creation", "modified",
        "comment_type", "comment_email", "comment_by", 
        "content", "custom_reply_to", "custom_visibility"
    ],
    order_by="creation DESC",
)

Implementation Steps

  1. Identify which fields are actually used in each function
  2. Replace fields="*" with explicit field lists
  3. Test that all functionality still works correctly
  4. Verify reduced data transfer in database query logs

Additional Notes

  • The content field can be large (HTML with images), so this is important for documents with many comments
  • This is a quick win with minimal risk
  • Same pattern appears in other repos (frappe-chitragupta, frappe-slack-connector)</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…ries

Co-authored-by: mrrobot47 <25586785+mrrobot47@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix fetching all columns unnecessarily in comment queries Optimize Comment queries by replacing fields="*" with explicit field lists Jan 14, 2026
Copilot AI requested a review from mrrobot47 January 14, 2026 04:26
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.

fields="*" Fetches All Columns Unnecessarily

2 participants