From 9b5198f463300f41c6e5126326ca6f930c8dd24d Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 11 Apr 2015 20:16:52 -0700 Subject: [PATCH] Remove ORDER_PATH_MODIFIED from Differential Summary: Ref T7803. This is a performance hack, not a real order, and isn't really meaningful or pageable. After D12158, we constraint his query on `dateModified` anyway, which should generally give the database a relatively small result set to examine. Test Plan: Browsed Differential and Diffusion. Checked query plan, it didn't look too crazy. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T7803 Differential Revision: https://secure.phabricator.com/D12361 --- .../DifferentialRevisionViewController.php | 2 +- .../query/DifferentialRevisionQuery.php | 22 ------------------- .../controller/DiffusionBrowseController.php | 2 +- 3 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index fd51b55e39..eadc085ce7 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -786,7 +786,7 @@ final class DifferentialRevisionViewController extends DifferentialController { ->setViewer($this->getRequest()->getUser()) ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) ->withUpdatedEpochBetween($recent, null) - ->setOrder(DifferentialRevisionQuery::ORDER_PATH_MODIFIED) + ->setOrder(DifferentialRevisionQuery::ORDER_MODIFIED) ->setLimit(10) ->needFlags(true) ->needDrafts(true) diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index 58c3815cc4..45359cac1a 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -44,14 +44,6 @@ final class DifferentialRevisionQuery private $order = 'order-modified'; const ORDER_MODIFIED = 'order-modified'; const ORDER_CREATED = 'order-created'; - /** - * This is essentially a denormalized copy of the revision modified time that - * should perform better for path queries with a LIMIT. Critically, when you - * browse "/", every revision in that repository for all time will match so - * the query benefits from being able to stop before fully materializing the - * result set. - */ - const ORDER_PATH_MODIFIED = 'order-path-modified'; private $needRelationships = false; private $needActiveDiffs = false; @@ -894,14 +886,6 @@ final class DifferentialRevisionQuery 'type' => 'int', ); break; - case self::ORDER_PATH_MODIFIED: - $columns[] = array( - 'table' => 'p', - 'column' => 'epoch', - 'value' => $cursor->getDateCreated(), - 'type' => 'int', - ); - break; } $columns[] = array( @@ -932,12 +916,6 @@ final class DifferentialRevisionQuery return 'id'; } return 'r.id'; - case self::ORDER_PATH_MODIFIED: - if (!$this->pathIDs) { - throw new Exception( - 'To use ORDER_PATH_MODIFIED, you must specify withPath().'); - } - return 'p.epoch'; default: throw new Exception("Unknown query order constant '{$this->order}'."); } diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index 7c63437d9e..fa2f0cb031 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -199,7 +199,7 @@ abstract class DiffusionBrowseController extends DiffusionController { ->withPath($repository->getID(), $path_id) ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) ->withUpdatedEpochBetween($recent, null) - ->setOrder(DifferentialRevisionQuery::ORDER_PATH_MODIFIED) + ->setOrder(DifferentialRevisionQuery::ORDER_MODIFIED) ->setLimit(10) ->needRelationships(true) ->needFlags(true)