Fix an issue where paginating notifications could fail a GROUP BY test
ClosedPublic
Actions

Authored by epriestley on Feb 26 2021, 6:45 PM.

Details

Reviewers: None
Maniphest Tasks: T13623: Notifications may fail to paginate with GROUP BY error
Commits: rP0a3093ef9c18: Fix an issue where paginating notifications could fail a GROUP BY test

Summary

Ref T13623. When paginating notifications, we may currently construct a query which:

loads from non-unique rows; and
returns multiple results.

In particular, chronologicalKey isn't unique across the whole table (only for a given viewer). We can get away with this because no user-facing view of notifications is truly "every notification for every viewer" today.

One fix would be to implicitly force the paging query to include withUserPHIDs(viewerPHID), but puruse a slightly more general fix:

Load only unique stories.
Explictly limit the pagination subquery to one result.

Test Plan

Set page size to 1, inserted duplicate notifications of all stories for another user, clicked "Next", got the GROUP BY error.
Applied the "only load unique stories" part of the change, got a "expected one row" error instead.
Applied the "limit 1" part of the change, got a second page of notifications.