HomePhabricator

Simplify construction and execution of Differential queries for "responsible"…

Description

Simplify construction and execution of Differential queries for "responsible" users

Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:

  • If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
  • Otherwise, use a very complicated JOIN/WHERE clause.

This is bad for several reasons:

  • Tons and tons of hard-coding and special casing.
  • The JOIN/WHERE clause performs very poorly for large datasets.
  • (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)

Instead, always use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.

Fixes T3377. Ref T603. Ref T2625. Depends on D6342.

There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:

SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT

...if we find that there's some performance benefit at some point.

Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603, T2625, T3377

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

Details

Provenance
epriestleyAuthored on Jul 3 2013, 12:39 PM
Reviewer
btrahan
Differential Revision
Restricted Differential Revision
Parents
rPe5569bdd3acc: Merge pull request #351 from grpaul/patch-1
Branches
Unknown
Tags
Unknown
Tasks
Restricted Maniphest Task
T603: Support permissions/policies in all Phabricator applications
Restricted Maniphest Task

Event Timeline