Skip to content
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.

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

FIX: correctly update replies_count on chat_threads #24711

Merged
merged 4 commits into from Dec 5, 2023

Conversation

jjaffeux
Copy link
Contributor

@jjaffeux jjaffeux commented Dec 5, 2023

The previous query would look at the existing messages, count them, and update the associated thread.

But, if for some reason messages were ALL deleted without updating the replies_count, then the query wouldn't find any message, and wouldn't update any thread's replies_count.

@github-actions github-actions bot added the chat PRs which include a change to Chat plugin label Dec 5, 2023
@jjaffeux
Copy link
Contributor Author

jjaffeux commented Dec 5, 2023

@martin-brennan I pinged you because you wrote this and I wanted your opinion.

The previous query would look at the existing messages, count them, and update the associated thread.

But, if for some reason messages were **ALL** deleted without updating the `replies_count`, then the query wouldn't find any message, and wouldn't update any thread's `replies_count`.
RETURNING ct.id AS thread_id
)
SELECT thread_id
FROM updated_threads;
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe, we don't need WITH here, the inner UPDATE query produces the same output, so it can be just:

UPDATE chat_threads ct
SET replies_count = COALESCE(subquery.new_count, 0)
FROM (
  SELECT cm.thread_id, COUNT(cm.*) - 1 AS new_count
  FROM chat_threads
  LEFT JOIN chat_messages cm ON cm.thread_id = chat_threads.id AND cm.deleted_at IS NULL
  GROUP BY cm.thread_id
) AS subquery
WHERE ct.id = subquery.thread_id AND ct.replies_count IS DISTINCT FROM COALESCE(subquery.new_count, 0)
RETURNING ct.id AS thread_id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I think you are correct

@jjaffeux jjaffeux merged commit c5aa6b5 into discourse:main Dec 5, 2023
19 checks passed
@jjaffeux jjaffeux deleted the chat/replies-count-update branch December 5, 2023 12:47
@martin-brennan
Copy link
Contributor

Change looks good, ty for pinging me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chat PRs which include a change to Chat plugin
3 participants