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

Count user's votes by subquery #3477

Merged
merged 6 commits into from
May 6, 2024
Merged

Count user's votes by subquery #3477

merged 6 commits into from
May 6, 2024

Conversation

dartcafe
Copy link
Collaborator

@dartcafe dartcafe commented May 3, 2024

The current approach to count a user's votes by a join statement returns a by the count of options multiplied result

  • Replace join by a subquery
  • correctly count votes of a user and count via subquery
  • count user's yes votes via subquery

This replaces selects on the votes table for every poll

to dos

  • Count orphaned votes via subquery

Signed-off-by: dartcafe <github@dartcafe.de>
@dartcafe

This comment was marked as outdated.

@dartcafe

This comment was marked as outdated.

dartcafe added 2 commits May 4, 2024 00:14
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
@dartcafe
Copy link
Collaborator Author

dartcafe commented May 3, 2024

OK. The IParam has to be defined outside of the subquery method.

Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
@dartcafe
Copy link
Collaborator Author

dartcafe commented May 4, 2024

@come-nc Do you think it is a little bit overengeneered?
Are subqueries a good idea regarding performance?

Signed-off-by: dartcafe <github@dartcafe.de>
@come-nc
Copy link
Contributor

come-nc commented May 6, 2024

Hello,

We looked into it and I think it would be best to avoid subqueries and limit ourselves to counts, sums and joins.
Do you think it would be possible to remove the orphan count information from the polls list? It’s the hardest to get without subquery or sidequery, and it does not look like something which should always be computed for all votes.

Couldn’t the orphaned votes simply be cleared at option deletion, or by a background job?

@dartcafe
Copy link
Collaborator Author

dartcafe commented May 6, 2024

Couldn’t the orphaned votes simply be cleared at option deletion, or by a background job?

This is the problem. Deleted options are kept by intention, in case an option is deleted accitentially or is to be added again after deletion. User's votes should be kept in these cases.

This is why I think a subquery is better than running recursively over the result list.

The other problem is that vote counts get multiplied by the avialable options before changing to the subquery. Sure, this could be solved in a join as well, but for me it works.

Please take also a look into #3479. I tried to solve the frontend rendering problems by chunking the polls list and by reducing the polls in the navigation to a maximum of 6 entries.

Chunking the polls list works by only rendering 20 items and add 20 more by reaching the list bottom by scrolling.

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Works and is better than previous state.
Having two separate subqueries for vote count and yes vote count seems suboptimal but let’s merge that for now it’s better anyway.
Maybe squash commits to help with backporting?

@dartcafe
Copy link
Collaborator Author

dartcafe commented May 6, 2024

Having two separate subqueries for vote count and yes vote count seems suboptimal but let’s merge that for now it’s better anyway.

:-) I went the lazy way, and avoided figuring out how to selct two results from the subquery. Any hint for improvement is welcome.

@dartcafe dartcafe merged commit 4ac30f4 into master May 6, 2024
15 checks passed
@dartcafe
Copy link
Collaborator Author

dartcafe commented May 6, 2024

Ah damn, I forgot to squash. @come-nc

@dartcafe dartcafe deleted the fix/count-votes-in-poll branch November 3, 2024 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants