Page MenuHomePhabricator

Notifications may fail to paginate with GROUP BY error
Closed, ResolvedPublic

Description

See https://discourse.phabricator-community.org/t/error-viewing-past-notifications/4498/.

When viewing a non-initial notifications page, we build a cursor with a query on chronologicalKey. This query may return more than one row, if that particular feed story notified more than one user. If it does, PhabricatorFeedStory::loadAllFromRows(...) fails with a GROUP BY error:

[Sat Jan 09 21:37:35.340510 2021] [:error] [pid 10222] [client 10.0.64.40:37057] [2021-01-09 21:37:35] EXCEPTION: (Exception) Rows passed to "loadAllFromArray(...)" include two or more rows with the same ID ("1204144"). Rows must have unique IDs. An underlying query may be missing a GROUP BY. at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:659]

In general, we should expect a NotificationQuery may load duplicate stories.

Revisions and Commits

Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision

Related Objects

Event Timeline

epriestley created this task.
epriestley added a revision: Restricted Differential Revision.Feb 26 2021, 6:45 PM
epriestley added a commit: Restricted Diffusion Commit.Feb 26 2021, 11:11 PM

I deployed this and nothing blew up, presuming this is resolved.

See https://discourse.phabricator-community.org/t/error-viewing-notifications/4646.

An issue arose when a user loads a page of notifications which include stories they don't have permission to view.

epriestley added a revision: Restricted Differential Revision.Mar 11 2021, 6:34 PM
epriestley added a commit: Restricted Diffusion Commit.Mar 11 2021, 6:44 PM
epriestley added a revision: Restricted Differential Revision.Mar 12 2021, 5:08 PM
epriestley added a commit: Restricted Diffusion Commit.Mar 12 2021, 5:09 PM

I think this was resolved. I nuked Discourse anyway so it's effectively resolved until another report shows up.