Page MenuHomePhabricator

Sorting in TablePager doesn't work for aggregates
Open, Needs TriagePublic

Description

In the CampaignEvents extension we have a list of all events and their number of participants, that is built with TablePager. FTR, here is the schema. The query looks like the following:

SELECT event_id,event_name,event_start,event_meeting_type,COUNT(cep_id) AS `num_participants` FROM `campaign_events` LEFT JOIN `ce_participants` ON ((event_id=cep_event_id)) LEFT JOIN `ce_organizers` ON ((event_id=ceo_event_id)) WHERE event_deleted_at IS NULL AND cep_unregistered_at IS NULL AND ceo_user_id = 1 GROUP BY cep_event_id,event_id,event_name,event_start,event_meeting_type ORDER BY event_start LIMIT 51

We would like to make the number of participants sortable (via TablePager::getSortableFields()), however that doesn't work in combinations with offsets. If you order by participants and go to the next page, the query will be like:

SELECT event_id,event_name,event_start,event_meeting_type,COUNT(cep_id) AS `num_participants` FROM `campaign_events` LEFT JOIN `ce_participants` ON ((event_id=cep_event_id)) LEFT JOIN `ce_organizers` ON ((event_id=ceo_event_id)) WHERE event_deleted_at IS NULL AND cep_unregistered_at IS NULL AND ceo_user_id = 1 AND (((num_participants>='2'))) GROUP BY cep_event_id,event_id,event_name,event_start,event_meeting_type ORDER BY num_participants LIMIT 1

However, since num_participants is a COUNT, it should be using HAVING, not WHERE.

Event Timeline

HAVING support exists for the abstract database layer, but not in the pager for pagination.
It also possible that the alias could not be used in the HAVING clause to support postgres (at least the core special pages with HAVING repeating the count(*) part)

Workaround could be a temp table (with IDatabase::buildSelectSubquery)

SELECT event_id,event_name,event_start,event_meeting_type,num_participants FROM ( SELECT event_id,event_name,event_start,event_meeting_type,COUNT(cep_id) AS `num_participants` FROM `campaign_events` LEFT JOIN `ce_participants` ON ((event_id=cep_event_id)) LEFT JOIN `ce_organizers` ON ((event_id=ceo_event_id)) WHERE event_deleted_at IS NULL AND cep_unregistered_at IS NULL and ceo_user_id = 1 GROUP BY cep_event_id,event_id,event_name,event_start,event_meeting_type ) as tmp WHERE (tmp.num_participants>=2) ORDER BY num_participants LIMIT 1

The ceo_user_id could be done outside of the tmp table, not sure what is better for the database and caching

SELECT event_id,event_name,event_start,event_meeting_type,ceo_user_id,num_participants FROM ( SELECT event_id,event_name,event_start,event_meeting_type,ceo_user_id,COUNT(cep_id) AS `num_participants` FROM `campaign_events` LEFT JOIN `ce_participants` ON ((event_id=cep_event_id)) LEFT JOIN `ce_organizers` ON ((event_id=ceo_event_id)) WHERE event_deleted_at IS NULL AND cep_unregistered_at IS NULL GROUP BY cep_event_id,event_id,event_name,event_start,event_meeting_type ) as tmp WHERE ceo_user_id = 1 AND (num_participants>=2) ORDER BY num_participants LIMIT 1

It also possible that the alias could not be used in the HAVING clause to support postgres (at least the core special pages with HAVING repeating the count(*) part)

Indeed, apparently postgres doesn't support HAVING on aliases. Then perhaps using a subquery may be the best solution. However, I still feel that Pager should fail more gracefully, instead of dying with an uncaught DB error...