@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Use cursor-based paging in Differential
Summary:
Ref T603. Ref T2625. Use cursors to page Differential queries, not offsets.
The trick here is that some queries are ordered. In these cases, we either need to pass some kind of tuple or do a cursor lookup. For example, if you are viewing revisions ordered by `dateModified`, we can either have the next page be something like:
?afterDateModified=2398329373&afterID=292&order=modified
...or some magical token:
?afterToken=2398329373:292&order=modified
I think we did this in Conpherence, but one factor there was that paging orders update with some frequency. In most cases, I think it's reasonable to pass just the ID and do a lookup to get the actual clause value (e.g., go look up object ID 292 and see what its dateModified is) and I think this is much simpler in general.
Test Plan: Set page size in Differential to 3, and paged through result lists ordered by date created and date modified.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625
Differential Revision: https://secure.phabricator.com/D6345
+95
-23
+5
-10
src/applications/differential/controller/DifferentialRevisionListController.php
+5
-10
src/applications/differential/controller/DifferentialRevisionListController.php
···
97
97
98
98
$pager = null;
99
99
if ($this->getFilterAllowsPaging($this->filter)) {
100
-
$pager = new AphrontPagerView();
101
-
$pager->setOffset($request->getInt('page'));
102
-
$pager->setPageSize(1000);
103
-
$pager->setURI($uri, 'page');
104
-
105
-
$query->setOffset($pager->getOffset());
106
-
$query->setLimit($pager->getPageSize() + 1);
100
+
$pager = new AphrontCursorPagerView();
101
+
$pager->readFromRequest($request);
107
102
}
108
103
109
104
foreach ($controls as $control) {
110
105
$this->applyControlToQuery($control, $query, $params);
111
106
}
112
107
113
-
$revisions = $query->execute();
114
-
115
108
if ($pager) {
116
-
$revisions = $pager->sliceResults($revisions);
109
+
$revisions = $query->executeWithCursorPager($pager);
110
+
} else {
111
+
$revisions = $query->execute();
117
112
}
118
113
119
114
$views = $this->buildViews(
+89
-12
src/applications/differential/query/DifferentialRevisionQuery.php
+89
-12
src/applications/differential/query/DifferentialRevisionQuery.php
···
57
57
private $needCommitPHIDs = false;
58
58
private $needHashes = false;
59
59
60
+
private $buildingGlobalOrder;
61
+
60
62
61
63
/* -( Query Configuration )------------------------------------------------ */
62
64
···
439
441
}
440
442
441
443
if (count($selects) > 1) {
444
+
$this->buildingGlobalOrder = true;
442
445
$query = qsprintf(
443
446
$conn_r,
444
447
'%Q %Q %Q',
445
448
implode(' UNION DISTINCT ', $selects),
446
-
$this->buildOrderByClause($conn_r, $is_global = true),
449
+
$this->buildOrderClause($conn_r),
447
450
$this->buildLimitClause($conn_r));
448
451
} else {
449
452
$query = head($selects);
···
463
466
$joins = $this->buildJoinsClause($conn_r);
464
467
$where = $this->buildWhereClause($conn_r);
465
468
$group_by = $this->buildGroupByClause($conn_r);
466
-
$order_by = $this->buildOrderByClause($conn_r);
469
+
470
+
$this->buildingGlobalOrder = false;
471
+
$order_by = $this->buildOrderClause($conn_r);
472
+
467
473
$limit = $this->buildLimitClause($conn_r);
468
474
469
475
return qsprintf(
···
675
681
"Unknown revision status filter constant '{$this->status}'!");
676
682
}
677
683
678
-
$where[] = $this->buildPagingCLause($conn_r);
684
+
$where[] = $this->buildPagingClause($conn_r);
679
685
return $this->formatWhereClause($where);
680
686
}
681
687
···
699
705
}
700
706
}
701
707
708
+
private function loadCursorObject($id) {
709
+
$results = id(new DifferentialRevisionQuery())
710
+
->setViewer($this->getViewer())
711
+
->withIDs(array($id))
712
+
->execute();
713
+
return head($results);
714
+
}
702
715
703
-
/**
704
-
* @task internal
705
-
*/
706
-
private function buildOrderByClause($conn_r, $is_global = false) {
716
+
protected function buildPagingClause(AphrontDatabaseConnection $conn_r) {
717
+
$default = parent::buildPagingClause($conn_r);
718
+
719
+
$before_id = $this->getBeforeID();
720
+
$after_id = $this->getAfterID();
721
+
722
+
if (!$before_id && !$after_id) {
723
+
return $default;
724
+
}
725
+
726
+
if ($before_id) {
727
+
$cursor = $this->loadCursorObject($before_id);
728
+
} else {
729
+
$cursor = $this->loadCursorObject($after_id);
730
+
}
731
+
732
+
if (!$cursor) {
733
+
return null;
734
+
}
735
+
736
+
switch ($this->order) {
737
+
case self::ORDER_CREATED:
738
+
return $default;
739
+
case self::ORDER_MODIFIED:
740
+
if ($before_id) {
741
+
return qsprintf(
742
+
$conn_r,
743
+
'(r.dateModified %Q %d OR (r.dateModified = %d AND r.id %Q %d))',
744
+
$this->getReversePaging() ? '<' : '>',
745
+
$cursor->getDateModified(),
746
+
$cursor->getDateModified(),
747
+
$this->getReversePaging() ? '<' : '>',
748
+
$cursor->getID());
749
+
} else {
750
+
return qsprintf(
751
+
$conn_r,
752
+
'(r.dateModified %Q %d OR (r.dateModified = %d AND r.id %Q %d))',
753
+
$this->getReversePaging() ? '>' : '<',
754
+
$cursor->getDateModified(),
755
+
$cursor->getDateModified(),
756
+
$this->getReversePaging() ? '>' : '<',
757
+
$cursor->getID());
758
+
}
759
+
case self::ORDER_PATH_MODIFIED:
760
+
if ($before_id) {
761
+
return qsprintf(
762
+
$conn_r,
763
+
'(p.epoch %Q %d OR (p.epoch = %d AND r.id %Q %d))',
764
+
$this->getReversePaging() ? '<' : '>',
765
+
$cursor->getDateCreated(),
766
+
$cursor->getDateCreated(),
767
+
$this->getReversePaging() ? '<' : '>',
768
+
$cursor->getID());
769
+
} else {
770
+
return qsprintf(
771
+
$conn_r,
772
+
'(p.epoch %Q %d OR (p.epoch = %d AND r.id %Q %d))',
773
+
$this->getReversePaging() ? '>' : '<',
774
+
$cursor->getDateCreated(),
775
+
$cursor->getDateCreated(),
776
+
$this->getReversePaging() ? '>' : '<',
777
+
$cursor->getID());
778
+
}
779
+
}
780
+
}
781
+
782
+
protected function getPagingColumn() {
783
+
$is_global = $this->buildingGlobalOrder;
707
784
switch ($this->order) {
708
785
case self::ORDER_MODIFIED:
709
786
if ($is_global) {
710
-
return 'ORDER BY dateModified DESC';
787
+
return 'dateModified';
711
788
}
712
-
return 'ORDER BY r.dateModified DESC';
789
+
return 'r.dateModified';
713
790
case self::ORDER_CREATED:
714
791
if ($is_global) {
715
-
return 'ORDER BY dateCreated DESC';
792
+
return 'id';
716
793
}
717
-
return 'ORDER BY r.dateCreated DESC';
794
+
return 'r.id';
718
795
case self::ORDER_PATH_MODIFIED:
719
796
if (!$this->pathIDs) {
720
797
throw new Exception(
721
798
"To use ORDER_PATH_MODIFIED, you must specify withPath().");
722
799
}
723
-
return 'ORDER BY p.epoch DESC';
800
+
return 'p.epoch';
724
801
default:
725
802
throw new Exception("Unknown query order constant '{$this->order}'.");
726
803
}