Skip to content

Fix invalid SQL syntax in clause_discussions_commented_by_user for empty result sets#459

Merged
netchampfaris merged 3 commits intodevelopfrom
copilot/fix-invalid-sql-syntax
Apr 22, 2026
Merged

Fix invalid SQL syntax in clause_discussions_commented_by_user for empty result sets#459
netchampfaris merged 3 commits intodevelopfrom
copilot/fix-invalid-sql-syntax

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 1, 2026

clause_discussions_commented_by_user() materializes comment lookups into a Python list, then passes it to Discussion.name.isin(list). When a user has no comments, this generates WHERE name IN () — invalid SQL that triggers MariaDB error 1064.

Changes

  • Replace materialized list with query builder subquery in clause_discussions_commented_by_user()
  • Add test coverage for users with no comments

Before

commented_in = list(
    set(
        frappe.db.get_all(
            "GP Comment",
            fields=["reference_name"],
            filters={"reference_doctype": "GP Discussion", "owner": user},
            pluck="reference_name",
        )
    )
)
return Discussion.name.isin(commented_in)  # IN () when empty

After

Comment = frappe.qb.DocType("GP Comment")
commented_in = (
    frappe.qb.from_(Comment)
    .select(Comment.reference_name)
    .where((Comment.reference_doctype == "GP Discussion") & (Comment.owner == user))
)
return Discussion.name.isin(commented_in)  # IN (SELECT ...) — always valid

This matches the pattern already used in clause_discussions_bookmarked_by_user().

Original prompt

This section details on the original issue you should resolve

<issue_title>GP DIscussions API clause_discussions_commented_by_user returns Invalid SQL Syntax on no Comments by User</issue_title>
<issue_description>Error: When building the “Participating” feed (or when a participator filter is used), clause_discussions_commented_by_user() materializes a Python list of discussion names and returns Discussion.name.isin(list). For users with no comments, this produced an empty IN clause (IN ()), which renders invalid SQL and causes MariaDB error 1064.

Suggested Solution (Worked fine for me):

def clause_discussions_commented_by_user(user):
	Discussion = frappe.qb.DocType("GP Discussion")
	commented_in = list(
-	set(
-		frappe.db.get_all(
-			"GP Comment",
-			fields=["reference_name"],
-			filters={"reference_doctype": "GP Discussion", "owner": user},
-			pluck="reference_name",
-		)
+	Comment = frappe.qb.DocType("GP Comment")
+	commented_in = (
+		frappe.qb.from_(Comment)
+		.select(Comment.reference_name)
+		.where(
+			(Comment.reference_doctype == "GP Discussion")
+			& (Comment.owner == user)
		)
	)
	return Discussion.name.isin(commented_in)
```</issue_description>

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

<comments>
</comments>


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix invalid SQL syntax in clause_discussions_commented_by_user Fix invalid SQL syntax in clause_discussions_commented_by_user for empty result sets Feb 1, 2026
Copilot AI requested a review from netchampfaris February 1, 2026 07:55
Copilot AI and others added 3 commits April 22, 2026 14:11
…s with no comments

Co-authored-by: netchampfaris <9355208+netchampfaris@users.noreply.github.com>
Co-authored-by: netchampfaris <9355208+netchampfaris@users.noreply.github.com>
Co-authored-by: netchampfaris <9355208+netchampfaris@users.noreply.github.com>
@netchampfaris netchampfaris force-pushed the copilot/fix-invalid-sql-syntax branch from a36476b to cf3648a Compare April 22, 2026 08:41
@cypress
Copy link
Copy Markdown

cypress Bot commented Apr 22, 2026

gameplan    Run #422

Run Properties:  status check passed Passed #422  •  git commit aa6ff7f9aa ℹ️: Merge cf3648ab9a76ef799d1cc2e817132e126bfd4c4b into ed54ca3e6742086f72c85bd6ca89...
Project gameplan
Branch Review copilot/fix-invalid-sql-syntax
Run status status check passed Passed #422
Run duration 00m 54s
Commit git commit aa6ff7f9aa ℹ️: Merge cf3648ab9a76ef799d1cc2e817132e126bfd4c4b into ed54ca3e6742086f72c85bd6ca89...
Committer Copilot
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 7
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

@netchampfaris netchampfaris marked this pull request as ready for review April 22, 2026 08:52
@netchampfaris netchampfaris merged commit 09b893c into develop Apr 22, 2026
3 checks passed
@netchampfaris netchampfaris deleted the copilot/fix-invalid-sql-syntax branch April 22, 2026 08:52
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.

GP DIscussions API clause_discussions_commented_by_user returns Invalid SQL Syntax on no Comments by User

2 participants