Skip to content
This repository was archived by the owner on Jul 22, 2025. It is now read-only.

Conversation

@xfalcox
Copy link
Member

@xfalcox xfalcox commented Nov 21, 2024

This change makes the /filter endpoint use the same criteria we use
in the dashboard table for emotion, so it is not confusing for users.
It means that only posts made in the period with the emotion shall be
shown in the /filter, and the order is simply a count of posts that
match the emotion in the period.

It also uses a trick to extract the filter period, and apply it to
the CTE clause that calculates post emotion count on the period, making
it a bit more efficient. Downside is that /filter filters are evaluated
from left to right, so it will only get the speed-up if the emotion
order is last. As we do this on the dashboard table, it should cover
most uses of the ordering, kicking the need for materialized views
down the road.

This change makes the /filter endpoint use the same criteria we use
in the dashboard table for emotion, so it is not confusing for users.
It means that only posts made in the period with the emotion shall be
shown in the /filter, and the order is simply a count of posts that
match the emotion in the period.

It also uses a trick to extract the filter period, and apply it to
the CTE clause that calculates post emotion count on the period, making
it a bit more efficient. Downside is that /filter filters are evaluated
from left to right, so it will only get the speed-up if the emotion
order is last. As we do this on the dashboard table, it should cover
most uses of the ordering, kicking the need for materialized views
down the road.
Copy link
Contributor

@pmusaraj pmusaraj left a comment

Choose a reason for hiding this comment

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

Looks good to me, CI test failures look relevant, though.

@xfalcox xfalcox merged commit 8e00e03 into main Nov 21, 2024
6 checks passed
@xfalcox xfalcox deleted the emotion-list-stricter branch November 21, 2024 18:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants