HomePhabricator

Add GROUP BY to commit query

Description

Add GROUP BY to commit query

Summary:
Ref T4715. Some minor stuff I caught locally while poking around:

  • Since we don't GROUP BY, we can still get duplicate commits. These get silently de-duplicated by loadAllFromArray() because that returns an array keyed by id, but we fetch too much data and this can cause us to execute too many queries to fill pages. Instead, GROUP BY if we joined the audit table.
  • After adding GROUP BY, getting the audit IDs out of the query is no longer reliable. Instead, query audits by the commit PHIDs. This is approximately equiavlent.
  • Since we always JOIN, we currently never return commits that don't have any audits. If we don't know that all results will have an audit, just LEFT JOIN.
  • Add some !== null to catch the withIDs(array()) issue that we hit with Khan Academy a little while ago.

Test Plan:

  • Verified that "All Commits" shows commits with no audits of any kind.
  • Verified that the raw data comes out of the query without duplicates.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5433, T4715

Differential Revision: https://secure.phabricator.com/D8879

Details

Provenance
epriestleyAuthored on
epriestleyPushed on Jul 10 2014, 5:16 PM
Reviewer
btrahan
Differential Revision
D8879: Add GROUP BY to commit query
Parents
rPd83bf5ea06d1: After a file upload, take the user to the info page, not the view page
Branches
Unknown
Tags
Unknown
Tasks
T4715: Move Audit to ApplicationSearch

Event Timeline