Merge changes from topic 'kill-patch-set-ancestors'

* changes:
  RelatedChangesSorter: Walk descendants depth-first
  Rename PatchSetAncestorSorter to RelatedChangesSorter
  Remove PatchSetAncestors table
  Remove GetRelatedByAncestors implementation
  EventFactory: Use repo instead of PatchSetAncestors for parents
  EventFactory: Remove unused addPatchSets variant
  EventFactory: Stop using PatchSetAncestors for addDependencies
  EventFactory: Use explicit ReviewDb
  EventFactory: Remove unnecessary finals

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java

@@ -30,11 +30,9 @@ import com.google.gerrit.server.change.GetRelated.RelatedInfo;
 import com.google.gerrit.server.edit.ChangeEditModifier;
 import com.google.gerrit.server.edit.ChangeEditUtil;
 import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.testutil.ConfigSuite;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 
-import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.junit.Test;
@@ -43,20 +41,6 @@ import java.io.IOException;
 import java.util.List;
 
 public class GetRelatedIT extends AbstractDaemonTest {
-  @ConfigSuite.Default
-  public static Config byGroup() {
-    Config cfg = new Config();
-    cfg.setBoolean("change", null, "getRelatedByAncestors", false);
-    return cfg;
-  }
-
-  @ConfigSuite.Config
-  public static Config byAncestors() {
-    Config cfg = new Config();
-    cfg.setBoolean("change", null, "getRelatedByAncestors", true);
-    return cfg;
-  }
-
   @Inject
   private ChangeEditUtil editUtil;
 
@@ -347,8 +331,8 @@ public class GetRelatedIT extends AbstractDaemonTest {
 
     // 1,2 is related directly to 4,1, and the 2-3 parallel branch stays intact.
     assertRelated(ps1_2,
-        changeAndCommit(ps3_2, c3_2, 2),
         changeAndCommit(ps4_1, c4_1, 1),
+        changeAndCommit(ps3_2, c3_2, 2),
         changeAndCommit(ps2_2, c2_2, 2),
         changeAndCommit(ps1_2, c1_2, 2));
 
@@ -432,6 +416,91 @@ public class GetRelatedIT extends AbstractDaemonTest {
     }
   }
 
+  @Test
+  public void getRelatedParallelDescendentBranches() throws Exception {
+    // 1,1---2,1---3,1
+    //   \---4,1---5,1
+    //    \--6,1---7,1
+
+    RevCommit c1_1 = commitBuilder()
+        .add("a.txt", "1")
+        .message("subject: 1")
+        .create();
+    RevCommit c2_1 = commitBuilder()
+        .add("b.txt", "2")
+        .message("subject: 2")
+        .create();
+    RevCommit c3_1 = commitBuilder()
+        .add("c.txt", "3")
+        .message("subject: 3")
+        .create();
+    pushHead(testRepo, "refs/for/master", false);
+    PatchSet.Id ps1_1 = getPatchSetId(c1_1);
+    PatchSet.Id ps2_1 = getPatchSetId(c2_1);
+    PatchSet.Id ps3_1 = getPatchSetId(c3_1);
+
+    testRepo.reset(c1_1);
+    RevCommit c4_1 = commitBuilder()
+        .add("d.txt", "4")
+        .message("subject: 4")
+        .create();
+    RevCommit c5_1 = commitBuilder()
+        .add("e.txt", "5")
+        .message("subject: 5")
+        .create();
+    pushHead(testRepo, "refs/for/master", false);
+    PatchSet.Id ps4_1 = getPatchSetId(c4_1);
+    PatchSet.Id ps5_1 = getPatchSetId(c5_1);
+
+    testRepo.reset(c1_1);
+    RevCommit c6_1 = commitBuilder()
+        .add("f.txt", "6")
+        .message("subject: 6")
+        .create();
+    RevCommit c7_1 = commitBuilder()
+        .add("g.txt", "7")
+        .message("subject: 7")
+        .create();
+    pushHead(testRepo, "refs/for/master", false);
+    PatchSet.Id ps6_1 = getPatchSetId(c6_1);
+    PatchSet.Id ps7_1 = getPatchSetId(c7_1);
+
+    // All changes are related to 1,1, keeping each of the parallel branches
+    // intact.
+    assertRelated(ps1_1,
+        changeAndCommit(ps7_1, c7_1, 1),
+        changeAndCommit(ps6_1, c6_1, 1),
+        changeAndCommit(ps5_1, c5_1, 1),
+        changeAndCommit(ps4_1, c4_1, 1),
+        changeAndCommit(ps3_1, c3_1, 1),
+        changeAndCommit(ps2_1, c2_1, 1),
+        changeAndCommit(ps1_1, c1_1, 1));
+
+    // The 2-3 branch is only related back to 1, not the other branches.
+    for (PatchSet.Id ps : ImmutableList.of(ps2_1, ps3_1)) {
+      assertRelated(ps,
+          changeAndCommit(ps3_1, c3_1, 1),
+          changeAndCommit(ps2_1, c2_1, 1),
+          changeAndCommit(ps1_1, c1_1, 1));
+    }
+
+    // The 4-5 branch is only related back to 1, not the other branches.
+    for (PatchSet.Id ps : ImmutableList.of(ps4_1, ps5_1)) {
+      assertRelated(ps,
+          changeAndCommit(ps5_1, c5_1, 1),
+          changeAndCommit(ps4_1, c4_1, 1),
+          changeAndCommit(ps1_1, c1_1, 1));
+    }
+
+    // The 6-7 branch is only related back to 1, not the other branches.
+    for (PatchSet.Id ps : ImmutableList.of(ps6_1, ps7_1)) {
+      assertRelated(ps,
+          changeAndCommit(ps7_1, c7_1, 1),
+          changeAndCommit(ps6_1, c6_1, 1),
+          changeAndCommit(ps1_1, c1_1, 1));
+    }
+  }
+
   @Test
   public void getRelatedEdit() throws Exception {
     // 1,1---2,1---3,1

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java

@@ -39,8 +39,6 @@ import java.util.Arrays;
  *          |
  *          +- {@link PatchSetApproval}: a +/- vote on the change's current state.
  *          |
- *          +- {@link PatchSetAncestor}: parents of this change's commit.
- *          |
  *          +- {@link PatchLineComment}: comment about a specific line
  * </pre>
  * <p>
@@ -51,11 +49,6 @@ import java.util.Arrays;
  * {@link Account} is usually also listed as the author and committer in the
  * PatchSetInfo.
  * <p>
- * The {@link PatchSetAncestor} entities are a mirror of the Git commit
- * metadata, providing access to the information without needing direct
- * accessing Git. These entities are actually legacy artifacts from Gerrit 1.x
- * and could be removed, replaced by direct RevCommit access.
- * <p>
  * Each PatchSet contains zero or more Patch records, detailing the file paths
  * impacted by the change (otherwise known as, the file paths the author
  * added/deleted/modified). Sometimes a merge commit can contain zero patches,

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetAncestor.java

@@ -1,88 +0,0 @@
-// Copyright (C) 2008 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.reviewdb.client;
-
-import com.google.gwtorm.client.Column;
-import com.google.gwtorm.client.IntKey;
-
-/** Ancestors of a {@link PatchSet} that the PatchSet depends upon. */
-public final class PatchSetAncestor {
-  public static class Id extends IntKey<PatchSet.Id> {
-    private static final long serialVersionUID = 1L;
-
-    @Column(id = 1, name = Column.NONE)
-    protected PatchSet.Id patchSetId;
-
-    @Column(id = 2)
-    protected int position;
-
-    protected Id() {
-      patchSetId = new PatchSet.Id();
-    }
-
-    public Id(final PatchSet.Id psId, final int pos) {
-      this.patchSetId = psId;
-      this.position = pos;
-    }
-
-    @Override
-    public PatchSet.Id getParentKey() {
-      return patchSetId;
-    }
-
-    @Override
-    public int get() {
-      return position;
-    }
-
-    @Override
-    protected void set(int newValue) {
-      position = newValue;
-    }
-  }
-
-  @Column(id = 1, name = Column.NONE)
-  protected Id key;
-
-  @Column(id = 2)
-  protected RevId ancestorRevision;
-
-  protected PatchSetAncestor() {
-  }
-
-  public PatchSetAncestor(final PatchSetAncestor.Id k) {
-    key = k;
-  }
-
-  public PatchSetAncestor.Id getId() {
-    return key;
-  }
-
-  public PatchSet.Id getPatchSet() {
-    return key.patchSetId;
-  }
-
-  public int getPosition() {
-    return key.position;
-  }
-
-  public RevId getAncestorRevision() {
-    return ancestorRevision;
-  }
-
-  public void setAncestorRevision(final RevId id) {
-    ancestorRevision = id;
-  }
-}

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/PatchSetAncestorAccess.java

@@ -1,45 +0,0 @@
-// Copyright (C) 2008 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.reviewdb.server;
-
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetAncestor;
-import com.google.gerrit.reviewdb.client.RevId;
-import com.google.gwtorm.server.Access;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.PrimaryKey;
-import com.google.gwtorm.server.Query;
-import com.google.gwtorm.server.ResultSet;
-
-public interface PatchSetAncestorAccess extends
-    Access<PatchSetAncestor, PatchSetAncestor.Id> {
-  @Override
-  @PrimaryKey("key")
-  PatchSetAncestor get(PatchSetAncestor.Id key) throws OrmException;
-
-  @Query("WHERE key.patchSetId = ? ORDER BY key.position")
-  ResultSet<PatchSetAncestor> ancestorsOf(PatchSet.Id id) throws OrmException;
-
-  @Query("WHERE key.patchSetId.changeId = ?")
-  ResultSet<PatchSetAncestor> byChange(Change.Id id) throws OrmException;
-
-  @Query("WHERE key.patchSetId = ?")
-  ResultSet<PatchSetAncestor> byPatchSet(PatchSet.Id id) throws OrmException;
-
-  @Query("WHERE ancestorRevision = ?")
-  ResultSet<PatchSetAncestor> descendantsOf(RevId revision)
-      throws OrmException;
-}

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java

@@ -92,8 +92,7 @@ public interface ReviewDb extends Schema {
   @Relation(id = 24)
   PatchSetAccess patchSets();
 
-  @Relation(id = 25)
+  // Deleted @Relation(id = 25)
-  PatchSetAncestorAccess patchSetAncestors();
 
   @Relation(id = 26)
   PatchLineCommentAccess patchComments();

gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql

@@ -87,14 +87,6 @@ ON patch_comments (status, author_id);
 CREATE INDEX patch_sets_byRevision
 ON patch_sets (revision);
 
--- *********************************************************************
--- PatchSetAncestorAccess
---    @PrimaryKey covers: ancestorsOf
---    covers:             descendantsOf
-CREATE INDEX patch_set_ancestors_desc
-ON patch_set_ancestors (ancestor_revision);
-
-
 -- *********************************************************************
 -- StarredChangeAccess
 --    @PrimaryKey covers: byAccount

gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql

@@ -96,15 +96,6 @@ CREATE INDEX patch_sets_byRevision
 ON patch_sets (revision)
 #
 
--- *********************************************************************
--- PatchSetAncestorAccess
---    @PrimaryKey covers: ancestorsOf
---    covers:             descendantsOf
-CREATE INDEX patch_set_ancestors_desc
-ON patch_set_ancestors (ancestor_revision)
-#
-
-
 -- *********************************************************************
 -- StarredChangeAccess
 --    @PrimaryKey covers: byAccount

gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql

@@ -136,13 +136,6 @@ WHERE status = 'd';
 CREATE INDEX patch_sets_byRevision
 ON patch_sets (revision);
 
--- *********************************************************************
--- PatchSetAncestorAccess
---    @PrimaryKey covers: ancestorsOf
---    covers:             descendantsOf
-CREATE INDEX patch_set_ancestors_desc
-ON patch_set_ancestors (ancestor_revision);
-
 -- *********************************************************************
 -- StarredChangeAccess
 --    @PrimaryKey covers: byAccount

gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java

@@ -36,6 +36,7 @@ import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.data.ApprovalAttribute;
+import com.google.gerrit.server.data.PatchSetAttribute;
 import com.google.gerrit.server.events.ChangeAbandonedEvent;
 import com.google.gerrit.server.events.ChangeMergedEvent;
 import com.google.gerrit.server.events.ChangeRestoredEvent;
@@ -62,6 +63,7 @@ import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -334,6 +336,16 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
       }
     }
 
+    private PatchSetAttribute asPatchSetAttribute(Change change,
+        PatchSet patchSet, ReviewDb db) throws OrmException {
+      try (Repository repo = repoManager.openRepository(change.getProject());
+          RevWalk revWalk = new RevWalk(repo)) {
+        return eventFactory.asPatchSetAttribute(db, revWalk, patchSet);
+      } catch (IOException e) {
+        throw new OrmException(e);
+      }
+    }
+
     /**
      * Fire the update hook
      *
@@ -380,8 +392,8 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
       AccountState uploader = accountCache.get(patchSet.getUploader());
       AccountState owner = accountCache.get(change.getOwner());
 
-      event.change = eventFactory.asChangeAttribute(change);
+      event.change = eventFactory.asChangeAttribute(db, change);
-      event.patchSet = eventFactory.asPatchSetAttribute(patchSet);
+      event.patchSet = asPatchSetAttribute(change, patchSet, db);
       event.uploader = eventFactory.asAccountAttribute(uploader.getAccount());
       fireEvent(change, event, db);
 
@@ -408,8 +420,8 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
       AccountState uploader = accountCache.get(patchSet.getUploader());
       AccountState owner = accountCache.get(change.getOwner());
 
-      event.change = eventFactory.asChangeAttribute(change);
+      event.change = eventFactory.asChangeAttribute(db, change);
-      event.patchSet = eventFactory.asPatchSetAttribute(patchSet);
+      event.patchSet = asPatchSetAttribute(change, patchSet, db);
       event.uploader = eventFactory.asAccountAttribute(uploader.getAccount());
       fireEvent(change, event, db);
 
@@ -434,9 +446,9 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
       CommentAddedEvent event = new CommentAddedEvent();
       AccountState owner = accountCache.get(change.getOwner());
 
-      event.change = eventFactory.asChangeAttribute(change);
+      event.change = eventFactory.asChangeAttribute(db, change);
       event.author =  eventFactory.asAccountAttribute(account);
-      event.patchSet = eventFactory.asPatchSetAttribute(patchSet);
+      event.patchSet = asPatchSetAttribute(change, patchSet, db);
       event.comment = comment;
 
       LabelTypes labelTypes = projectCache.get(change.getProject()).getLabelTypes();
@@ -478,9 +490,9 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
       ChangeMergedEvent event = new ChangeMergedEvent();
       AccountState owner = accountCache.get(change.getOwner());
 
-      event.change = eventFactory.asChangeAttribute(change);
+      event.change = eventFactory.asChangeAttribute(db, change);
       event.submitter = eventFactory.asAccountAttribute(account);
-      event.patchSet = eventFactory.asPatchSetAttribute(patchSet);
+      event.patchSet = asPatchSetAttribute(change, patchSet, db);
       event.newRev = mergeResultRev;
       fireEvent(change, event, db);
 
@@ -505,9 +517,9 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
       MergeFailedEvent event = new MergeFailedEvent();
       AccountState owner = accountCache.get(change.getOwner());
 
-      event.change = eventFactory.asChangeAttribute(change);
+      event.change = eventFactory.asChangeAttribute(db, change);
       event.submitter = eventFactory.asAccountAttribute(account);
-      event.patchSet = eventFactory.asPatchSetAttribute(patchSet);
+      event.patchSet = asPatchSetAttribute(change, patchSet, db);
       event.reason = reason;
       fireEvent(change, event, db);
 
@@ -532,9 +544,9 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
       ChangeAbandonedEvent event = new ChangeAbandonedEvent();
       AccountState owner = accountCache.get(change.getOwner());
 
-      event.change = eventFactory.asChangeAttribute(change);
+      event.change = eventFactory.asChangeAttribute(db, change);
       event.abandoner = eventFactory.asAccountAttribute(account);
-      event.patchSet = eventFactory.asPatchSetAttribute(patchSet);
+      event.patchSet = asPatchSetAttribute(change, patchSet, db);
       event.reason = reason;
       fireEvent(change, event, db);
 
@@ -559,9 +571,9 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
       ChangeRestoredEvent event = new ChangeRestoredEvent();
       AccountState owner = accountCache.get(change.getOwner());
 
-      event.change = eventFactory.asChangeAttribute(change);
+      event.change = eventFactory.asChangeAttribute(db, change);
       event.restorer = eventFactory.asAccountAttribute(account);
-      event.patchSet = eventFactory.asPatchSetAttribute(patchSet);
+      event.patchSet = asPatchSetAttribute(change, patchSet, db);
       event.reason = reason;
       fireEvent(change, event, db);
 
@@ -615,8 +627,8 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
       ReviewerAddedEvent event = new ReviewerAddedEvent();
       AccountState owner = accountCache.get(change.getOwner());
 
-      event.change = eventFactory.asChangeAttribute(change);
+      event.change = eventFactory.asChangeAttribute(db, change);
-      event.patchSet = eventFactory.asPatchSetAttribute(patchSet);
+      event.patchSet = asPatchSetAttribute(change, patchSet, db);
       event.reviewer = eventFactory.asAccountAttribute(account);
       fireEvent(change, event, db);
 
@@ -638,7 +650,7 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
       TopicChangedEvent event = new TopicChangedEvent();
       AccountState owner = accountCache.get(change.getOwner());
 
-      event.change = eventFactory.asChangeAttribute(change);
+      event.change = eventFactory.asChangeAttribute(db, change);
       event.changer = eventFactory.asAccountAttribute(account);
       event.oldTopic = oldTopic;
       fireEvent(change, event, db);
@@ -670,7 +682,7 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
       HashtagsChangedEvent event = new HashtagsChangedEvent();
       AccountState owner = accountCache.get(change.getOwner());
 
-      event.change = eventFactory.asChangeAttribute(change);
+      event.change = eventFactory.asChangeAttribute(db, change);
       event.editor = eventFactory.asAccountAttribute(account);
       event.hashtags = hashtagArray(hashtags);
       event.added = hashtagArray(added);

gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java

@@ -25,9 +25,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetAncestor;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.change.ChangeInserter;
 import com.google.gerrit.server.change.ChangeMessages;
@@ -72,7 +70,6 @@ import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.text.MessageFormat;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -149,19 +146,6 @@ public class ChangeUtil {
     c.setLastUpdatedOn(TimeUtil.nowTs());
   }
 
-  public static void insertAncestors(ReviewDb db, PatchSet.Id id, RevCommit src)
-      throws OrmException {
-    int cnt = src.getParentCount();
-    List<PatchSetAncestor> toInsert = new ArrayList<>(cnt);
-    for (int p = 0; p < cnt; p++) {
-      PatchSetAncestor a =
-          new PatchSetAncestor(new PatchSetAncestor.Id(id, p + 1));
-      a.setAncestorRevision(new RevId(src.getParent(p).getId().getName()));
-      toInsert.add(a);
-    }
-    db.patchSetAncestors().insert(toInsert);
-  }
-
   public static PatchSet.Id nextPatchSetId(Map<String, Ref> allRefs,
       PatchSet.Id id) {
     PatchSet.Id next = nextPatchSetId(id);
@@ -354,7 +338,6 @@ public class ChangeUtil {
       db.patchComments().delete(db.patchComments().byChange(changeId));
 
       db.patchSetApprovals().delete(db.patchSetApprovals().byChange(changeId));
-      db.patchSetAncestors().delete(db.patchSetAncestors().byChange(changeId));
       db.patchSets().delete(patchSets);
       db.changeMessages().delete(db.changeMessages().byChange(changeId));
       db.starredChanges().delete(db.starredChanges().byChange(changeId));
@@ -461,7 +444,6 @@ public class ChangeUtil {
     // No need to delete from notedb; draft patch sets will be filtered out.
     db.patchComments().delete(db.patchComments().byPatchSet(patchSetId));
     db.patchSetApprovals().delete(db.patchSetApprovals().byPatchSet(patchSetId));
-    db.patchSetAncestors().delete(db.patchSetAncestors().byPatchSet(patchSetId));
 
     db.patchSets().delete(Collections.singleton(patch));
   }

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java

@@ -245,7 +245,6 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
     ReviewDb db = ctx.getDb();
     ChangeControl ctl = ctx.getChangeControl();
     ChangeUpdate update = ctx.getChangeUpdate();
-    ChangeUtil.insertAncestors(db, patchSet.getId(), commit);
     if (patchSet.getGroups() == null) {
       patchSet.setGroups(GroupCollector.getDefaultGroups(patchSet));
     }

gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java

@@ -583,8 +583,6 @@ public class ConsistencyChecker {
         // historical information.
         db.accountPatchReviews().delete(
             db.accountPatchReviews().byPatchSet(psId));
-        db.patchSetAncestors().delete(
-            db.patchSetAncestors().byPatchSet(psId));
         db.patchSetApprovals().delete(
             db.patchSetApprovals().byPatchSet(psId));
         db.patchComments().delete(

gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java

@@ -14,8 +14,6 @@
 
 package com.google.gerrit.server.change;
 
-import static com.google.gerrit.server.index.ChangeField.GROUP;
-
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.Lists;
 import com.google.gerrit.common.Nullable;
@@ -25,10 +23,8 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CommonConverters;
-import com.google.gerrit.server.change.PatchSetAncestorSorter.PatchSetData;
+import com.google.gerrit.server.change.RelatedChangesSorter.PatchSetData;
-import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.git.GroupCollector;
-import com.google.gerrit.server.index.IndexCollection;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
@@ -37,7 +33,6 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.revwalk.RevCommit;
 
 import java.io.IOException;
@@ -50,45 +45,29 @@ import java.util.Set;
 @Singleton
 public class GetRelated implements RestReadView<RevisionResource> {
   private final Provider<ReviewDb> db;
-  private final GetRelatedByAncestors byAncestors;
   private final Provider<InternalChangeQuery> queryProvider;
-  private final PatchSetAncestorSorter sorter;
+  private final RelatedChangesSorter sorter;
-  private final IndexCollection indexes;
-  private final boolean byAncestorsOnly;
 
   @Inject
   GetRelated(Provider<ReviewDb> db,
-      @GerritServerConfig Config cfg,
-      GetRelatedByAncestors byAncestors,
       Provider<InternalChangeQuery> queryProvider,
-      PatchSetAncestorSorter sorter,
+      RelatedChangesSorter sorter) {
-      IndexCollection indexes) {
     this.db = db;
-    this.byAncestors = byAncestors;
     this.queryProvider = queryProvider;
     this.sorter = sorter;
-    this.indexes = indexes;
-    byAncestorsOnly =
-        cfg.getBoolean("change", null, "getRelatedByAncestors", false);
   }
 
   @Override
   public RelatedInfo apply(RevisionResource rsrc)
       throws RepositoryNotFoundException, IOException, OrmException {
-    List<String> thisPatchSetGroups = GroupCollector.getGroups(rsrc);
-    if (byAncestorsOnly
-        || thisPatchSetGroups == null
-        || !indexes.getSearchIndex().getSchema().hasField(GROUP)) {
-      return byAncestors.getRelated(rsrc);
-    }
     RelatedInfo relatedInfo = new RelatedInfo();
-    relatedInfo.changes = getRelated(rsrc, thisPatchSetGroups);
+    relatedInfo.changes = getRelated(rsrc);
     return relatedInfo;
   }
 
-  private List<ChangeAndCommit> getRelated(RevisionResource rsrc,
+  private List<ChangeAndCommit> getRelated(RevisionResource rsrc)
-      List<String> thisPatchSetGroups) throws OrmException, IOException {
+      throws OrmException, IOException {
-    if (thisPatchSetGroups.isEmpty()) {
+    if (GroupCollector.getGroups(rsrc).isEmpty()) {
       return Collections.emptyList();
     }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelatedByAncestors.java

@@ -1,266 +0,0 @@
-// Copyright (C) 2015 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.change;
-
-import com.google.common.collect.ArrayListMultimap;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Multimap;
-import com.google.common.collect.Sets;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetAncestor;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.change.GetRelated.ChangeAndCommit;
-import com.google.gerrit.server.change.GetRelated.RelatedInfo;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.project.ChangeControl;
-import com.google.gerrit.server.project.ProjectControl;
-import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.InternalChangeQuery;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-
-import org.eclipse.jgit.errors.IncorrectObjectTypeException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevFlag;
-import org.eclipse.jgit.revwalk.RevSort;
-import org.eclipse.jgit.revwalk.RevWalk;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.io.IOException;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
-/** Implementation of {@link GetRelated} using {@link PatchSetAncestor}s. */
-class GetRelatedByAncestors {
-  private static final Logger log = LoggerFactory.getLogger(GetRelated.class);
-
-  private final GitRepositoryManager gitMgr;
-  private final Provider<ReviewDb> dbProvider;
-  private final Provider<InternalChangeQuery> queryProvider;
-
-  @Inject
-  GetRelatedByAncestors(GitRepositoryManager gitMgr,
-      Provider<ReviewDb> db,
-      Provider<InternalChangeQuery> queryProvider) {
-    this.gitMgr = gitMgr;
-    this.dbProvider = db;
-    this.queryProvider = queryProvider;
-  }
-
-  public RelatedInfo getRelated(RevisionResource rsrc)
-      throws RepositoryNotFoundException, IOException, OrmException {
-    try (Repository git = gitMgr.openRepository(rsrc.getChange().getProject());
-        RevWalk rw = new RevWalk(git)) {
-      Ref ref = git.getRefDatabase().exactRef(rsrc.getChange().getDest().get());
-      RelatedInfo info = new RelatedInfo();
-      info.changes = walk(rsrc, rw, ref);
-      return info;
-    }
-  }
-
-  private List<ChangeAndCommit> walk(RevisionResource rsrc, RevWalk rw, Ref ref)
-      throws OrmException, IOException {
-    Map<Change.Id, ChangeData> changes = allOpenChanges(rsrc);
-    Map<PatchSet.Id, PatchSet> patchSets = allPatchSets(rsrc, changes.values());
-
-    Map<String, PatchSet> commits = Maps.newHashMap();
-    for (PatchSet p : patchSets.values()) {
-      commits.put(p.getRevision().get(), p);
-    }
-
-    RevCommit rev = rw.parseCommit(ObjectId.fromString(
-        rsrc.getPatchSet().getRevision().get()));
-    rw.sort(RevSort.TOPO);
-    rw.markStart(rev);
-
-    if (ref != null && ref.getObjectId() != null) {
-      try {
-        rw.markUninteresting(rw.parseCommit(ref.getObjectId()));
-      } catch (IncorrectObjectTypeException notCommit) {
-        // Ignore and treat as new branch.
-      }
-    }
-
-    Set<Change.Id> added = Sets.newHashSet();
-    List<ChangeAndCommit> parents = Lists.newArrayList();
-    for (RevCommit c; (c = rw.next()) != null;) {
-      PatchSet p = commits.get(c.name());
-      Change g = null;
-      if (p != null) {
-        g = changes.get(p.getId().getParentKey()).change();
-        added.add(p.getId().getParentKey());
-      }
-      parents.add(new ChangeAndCommit(g, p, c));
-    }
-    List<ChangeAndCommit> list = children(rsrc, rw, changes, patchSets, added);
-    list.addAll(parents);
-
-    if (list.size() == 1) {
-      ChangeAndCommit r = list.get(0);
-      if (r.commit != null && r.commit.commit.equals(rsrc.getPatchSet().getRevision().get())) {
-        return Collections.emptyList();
-      }
-    }
-    return list;
-  }
-
-  private Map<Change.Id, ChangeData> allOpenChanges(RevisionResource rsrc)
-      throws OrmException {
-    return ChangeData.asMap(
-        queryProvider.get().byBranchOpen(rsrc.getChange().getDest()));
-  }
-
-  private Map<PatchSet.Id, PatchSet> allPatchSets(RevisionResource rsrc,
-      Collection<ChangeData> cds) throws OrmException {
-    Map<PatchSet.Id, PatchSet> r =
-        Maps.newHashMapWithExpectedSize(cds.size() * 2);
-    for (ChangeData cd : cds) {
-      for (PatchSet p : cd.patchSets()) {
-        r.put(p.getId(), p);
-      }
-    }
-
-    if (rsrc.getEdit().isPresent()) {
-      r.put(rsrc.getPatchSet().getId(), rsrc.getPatchSet());
-    }
-    return r;
-  }
-
-  private List<ChangeAndCommit> children(RevisionResource rsrc, RevWalk rw,
-      Map<Change.Id, ChangeData> changes, Map<PatchSet.Id, PatchSet> patchSets,
-      Set<Change.Id> added)
-      throws OrmException, IOException {
-    // children is a map of parent commit name to PatchSet built on it.
-    Multimap<String, PatchSet.Id> children = allChildren(changes.keySet());
-
-    RevFlag seenCommit = rw.newFlag("seenCommit");
-    LinkedList<String> q = Lists.newLinkedList();
-    seedQueue(rsrc, rw, seenCommit, patchSets, q);
-
-    ProjectControl projectCtl = rsrc.getControl().getProjectControl();
-    Set<Change.Id> seenChange = Sets.newHashSet();
-    List<ChangeAndCommit> graph = Lists.newArrayList();
-    while (!q.isEmpty()) {
-      String id = q.remove();
-
-      // For every matching change find the most recent patch set.
-      Map<Change.Id, PatchSet.Id> matches = Maps.newHashMap();
-      for (PatchSet.Id psId : children.get(id)) {
-        PatchSet.Id e = matches.get(psId.getParentKey());
-        if ((e == null || e.get() < psId.get())
-            && isVisible(projectCtl, changes, patchSets, psId))  {
-          matches.put(psId.getParentKey(), psId);
-        }
-      }
-
-      for (Map.Entry<Change.Id, PatchSet.Id> e : matches.entrySet()) {
-        ChangeData cd = changes.get(e.getKey());
-        PatchSet ps = patchSets.get(e.getValue());
-        if (cd == null || ps == null || !seenChange.add(e.getKey())) {
-          continue;
-        }
-
-        RevCommit c = rw.parseCommit(ObjectId.fromString(
-            ps.getRevision().get()));
-        if (!c.has(seenCommit)) {
-          c.add(seenCommit);
-          q.addFirst(ps.getRevision().get());
-          if (added.add(ps.getId().getParentKey())) {
-            rw.parseBody(c);
-            graph.add(new ChangeAndCommit(cd.change(), ps, c));
-          }
-        }
-      }
-    }
-    Collections.reverse(graph);
-    return graph;
-  }
-
-  private boolean isVisible(ProjectControl projectCtl,
-      Map<Change.Id, ChangeData> changes,
-      Map<PatchSet.Id, PatchSet> patchSets,
-      PatchSet.Id psId) throws OrmException {
-    ChangeData cd = changes.get(psId.getParentKey());
-    PatchSet ps = patchSets.get(psId);
-    if (cd != null && ps != null) {
-      // Related changes are in the same project, so reuse the existing
-      // ProjectControl.
-      ChangeControl ctl = projectCtl.controlFor(cd.change());
-      return ctl.isVisible(dbProvider.get())
-          && ctl.isPatchVisible(ps, dbProvider.get());
-    }
-    return false;
-  }
-
-  private void seedQueue(RevisionResource rsrc, RevWalk rw,
-      RevFlag seenCommit, Map<PatchSet.Id, PatchSet> patchSets,
-      LinkedList<String> q) throws IOException {
-    RevCommit tip = rw.parseCommit(ObjectId.fromString(
-        rsrc.getPatchSet().getRevision().get()));
-    tip.add(seenCommit);
-    q.add(tip.name());
-
-    Change.Id cId = rsrc.getChange().getId();
-    for (PatchSet p : patchSets.values()) {
-      if (cId.equals(p.getId().getParentKey())) {
-        try {
-          RevCommit c = rw.parseCommit(ObjectId.fromString(
-              p.getRevision().get()));
-          if (!c.has(seenCommit)) {
-            c.add(seenCommit);
-            q.add(c.name());
-          }
-        } catch (IOException e) {
-          log.warn(String.format(
-              "Cannot read patch set %d of %d",
-              p.getPatchSetId(), cId.get()), e);
-        }
-      }
-    }
-  }
-
-  private Multimap<String, PatchSet.Id> allChildren(Collection<Change.Id> ids)
-      throws OrmException {
-    ReviewDb db = dbProvider.get();
-    List<ResultSet<PatchSetAncestor>> t =
-        Lists.newArrayListWithCapacity(ids.size());
-    for (Change.Id id : ids) {
-      t.add(db.patchSetAncestors().byChange(id));
-    }
-
-    Multimap<String, PatchSet.Id> r = ArrayListMultimap.create();
-    for (ResultSet<PatchSetAncestor> rs : t) {
-      for (PatchSetAncestor a : rs) {
-        r.put(a.getAncestorRevision().get(), a.getPatchSet());
-      }
-    }
-    return r;
-  }
-
-
-}

gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java

@@ -221,7 +221,6 @@ public class PatchSetInserter extends BatchUpdate.Op {
     patchSet.setRevision(new RevId(commit.name()));
     patchSet.setDraft(draft);
 
-    ChangeUtil.insertAncestors(db, patchSet.getId(), commit);
     if (groups != null) {
       patchSet.setGroups(groups);
     } else {

gerrit-server/src/main/java/com/google/gerrit/server/change/RelatedChangesSorter.java

@@ -52,11 +52,11 @@ import java.util.Objects;
 import java.util.Set;
 
 @Singleton
-class PatchSetAncestorSorter {
+class RelatedChangesSorter {
   private final GitRepositoryManager repoManager;
 
   @Inject
-  PatchSetAncestorSorter(GitRepositoryManager repoManager) {
+  RelatedChangesSorter(GitRepositoryManager repoManager) {
     this.repoManager = repoManager;
   }
 
@@ -199,10 +199,10 @@ class PatchSetAncestorSorter {
         }
         allPatchSets.add(psd);
       }
-      // Breadth-first search with oldest children first.
+      // Depth-first search with newest children first.
-      // TODO(dborowitz): After killing PatchSetAncestors, consider DFS to keep
+      for (PatchSetData child : children.get(psd)) {
-      // parallel history together.
+        pending.addFirst(child);
-      pending.addAll(Lists.reverse(children.get(psd)));
+      }
     }
 
     // If we saw the same change multiple times, prefer the latest patch set.
@@ -227,7 +227,7 @@ class PatchSetAncestorSorter {
   abstract static class PatchSetData {
     @VisibleForTesting
     static PatchSetData create(ChangeData cd, PatchSet ps, RevCommit commit) {
-      return new AutoValue_PatchSetAncestorSorter_PatchSetData(cd, ps, commit);
+      return new AutoValue_RelatedChangesSorter_PatchSetData(cd, ps, commit);
     }
 
     abstract ChangeData data();

gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java

@@ -14,8 +14,12 @@
 
 package com.google.gerrit.server.events;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.common.base.Function;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Multimap;
+import com.google.common.collect.Ordering;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelTypes;
@@ -27,9 +31,7 @@ import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetAncestor;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.client.UserIdentity;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
@@ -57,56 +59,59 @@ import com.google.gerrit.server.patch.PatchListNotAvailableException;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
 @Singleton
 public class EventFactory {
   private static final Logger log = LoggerFactory.getLogger(EventFactory.class);
+
   private final AccountCache accountCache;
   private final Provider<String> urlProvider;
   private final PatchListCache patchListCache;
-  private final SchemaFactory<ReviewDb> schema;
   private final PatchSetInfoFactory psInfoFactory;
   private final PersonIdent myIdent;
-  private final Provider<ReviewDb> db;
   private final ChangeData.Factory changeDataFactory;
   private final ApprovalsUtil approvalsUtil;
   private final ChangeKindCache changeKindCache;
+  private final Provider<InternalChangeQuery> queryProvider;
 
   @Inject
   EventFactory(AccountCache accountCache,
       @CanonicalWebUrl @Nullable Provider<String> urlProvider,
       PatchSetInfoFactory psif,
-      PatchListCache patchListCache, SchemaFactory<ReviewDb> schema,
+      PatchListCache patchListCache,
       @GerritPersonIdent PersonIdent myIdent,
-      Provider<ReviewDb> db,
       ChangeData.Factory changeDataFactory,
       ApprovalsUtil approvalsUtil,
-      ChangeKindCache changeKindCache) {
+      ChangeKindCache changeKindCache,
+      Provider<InternalChangeQuery> queryProvider) {
     this.accountCache = accountCache;
     this.urlProvider = urlProvider;
     this.patchListCache = patchListCache;
-    this.schema = schema;
     this.psInfoFactory = psif;
     this.myIdent = myIdent;
-    this.db = db;
     this.changeDataFactory = changeDataFactory;
     this.approvalsUtil = approvalsUtil;
     this.changeKindCache = changeKindCache;
+    this.queryProvider = queryProvider;
   }
 
   /**
@@ -116,7 +121,7 @@ public class EventFactory {
    * @param change
    * @return object suitable for serialization to JSON
    */
-  public ChangeAttribute asChangeAttribute(final Change change) {
+  public ChangeAttribute asChangeAttribute(ReviewDb db, Change change) {
     ChangeAttribute a = new ChangeAttribute();
     a.project = change.getProject().get();
     a.branch = change.getDest().getShortName();
@@ -125,8 +130,7 @@ public class EventFactory {
     a.number = change.getId().toString();
     a.subject = change.getSubject();
     try {
-      a.commitMessage =
+      a.commitMessage = changeDataFactory.create(db, change).commitMessage();
-          changeDataFactory.create(db.get(), change).commitMessage();
     } catch (Exception e) {
       log.error("Error while getting full commit message for"
           + " change " + a.number);
@@ -146,7 +150,8 @@ public class EventFactory {
    * @param refName
    * @return object suitable for serialization to JSON
    */
-  public RefUpdateAttribute asRefUpdateAttribute(final ObjectId oldId, final ObjectId newId, final Branch.NameKey refName) {
+  public RefUpdateAttribute asRefUpdateAttribute(ObjectId oldId, ObjectId newId,
+      Branch.NameKey refName) {
     RefUpdateAttribute ru = new RefUpdateAttribute();
     ru.newRev = newId != null ? newId.getName() : ObjectId.zeroId().getName();
     ru.oldRev = oldId != null ? oldId.getName() : ObjectId.zeroId().getName();
@@ -173,10 +178,10 @@ public class EventFactory {
    * @param a
    * @param notes
    */
-  public void addAllReviewers(ChangeAttribute a, ChangeNotes notes)
+  public void addAllReviewers(ReviewDb db, ChangeAttribute a, ChangeNotes notes)
       throws OrmException {
     Collection<Account.Id> reviewers =
-        approvalsUtil.getReviewers(db.get(), notes).values();
+        approvalsUtil.getReviewers(db, notes).values();
     if (!reviewers.isEmpty()) {
       a.allReviewers = Lists.newArrayListWithCapacity(reviewers.size());
       for (Account.Id id : reviewers) {
@@ -226,51 +231,17 @@ public class EventFactory {
     }
   }
 
-  public void addDependencies(ChangeAttribute ca, Change change) {
+  public void addDependencies(RevWalk rw, ChangeAttribute ca, Change change,
+      PatchSet currentPs) {
+    if (change == null || currentPs == null) {
+      return;
+    }
     ca.dependsOn = new ArrayList<>();
     ca.neededBy = new ArrayList<>();
-    try (ReviewDb db = schema.open()) {
+    try {
-      final PatchSet.Id psId = change.currentPatchSetId();
+      addDependsOn(rw, ca, change, currentPs);
-      for (PatchSetAncestor a : db.patchSetAncestors().ancestorsOf(psId)) {
+      addNeededBy(rw, ca, change, currentPs);
-        for (PatchSet p :
+    } catch (OrmException | IOException e) {
-            db.patchSets().byRevision(a.getAncestorRevision())) {
-          Change c = db.changes().get(p.getId().getParentKey());
-          if (c == null) {
-            log.error("Error while generating the ancestor change for"
-                + " revision " + a.getAncestorRevision() + ": Cannot find"
-                + " Change entry in database for " + p.getId().getParentKey());
-            continue;
-          }
-          ca.dependsOn.add(newDependsOn(c, p));
-        }
-      }
-
-      final PatchSet ps = db.patchSets().get(psId);
-      if (ps == null) {
-        log.error("Error while generating the list of descendants for"
-            + " PatchSet " + psId + ": Cannot find PatchSet entry in"
-            + " database.");
-      } else {
-        final RevId revId = ps.getRevision();
-        for (PatchSetAncestor a : db.patchSetAncestors().descendantsOf(revId)) {
-          final PatchSet p = db.patchSets().get(a.getPatchSet());
-          if (p == null) {
-            log.error("Error while generating the list of descendants for"
-                + " revision " + revId.get() + ": Cannot find PatchSet entry in"
-                + " database for " + a.getPatchSet());
-            continue;
-          }
-          final Change c = db.changes().get(p.getId().getParentKey());
-          if (c == null) {
-            log.error("Error while generating the list of descendants for"
-                + " revision " + revId.get() + ": Cannot find Change entry in"
-                + " database for " + p.getId().getParentKey());
-            continue;
-          }
-          ca.neededBy.add(newNeededBy(c, p));
-        }
-      }
-    } catch (OrmException e) {
       // Squash DB exceptions and leave dependency lists partially filled.
     }
     // Remove empty lists so a confusing label won't be displayed in the output.
@@ -282,6 +253,67 @@ public class EventFactory {
     }
   }
 
+  private void addDependsOn(RevWalk rw, ChangeAttribute ca, Change change,
+      PatchSet currentPs) throws OrmException, IOException {
+    RevCommit commit =
+        rw.parseCommit(ObjectId.fromString(currentPs.getRevision().get()));
+    final List<String> parentNames = new ArrayList<>(commit.getParentCount());
+    for (RevCommit p : commit.getParents()) {
+      parentNames.add(p.name());
+    }
+
+    // Find changes in this project having a patch set matching any parent of
+    // this patch set's revision.
+    for (ChangeData cd : queryProvider.get().byProjectCommits(
+        change.getProject(), parentNames)) {
+      for (PatchSet ps : cd.patchSets()) {
+        for (String p : parentNames) {
+          if (!ps.getRevision().get().equals(p)) {
+            continue;
+          }
+          ca.dependsOn.add(newDependsOn(checkNotNull(cd.change()), ps));
+        }
+      }
+    }
+    // Sort by original parent order.
+    Collections.sort(ca.dependsOn, Ordering.natural().onResultOf(
+        new Function<DependencyAttribute, Integer>() {
+          @Override
+          public Integer apply(DependencyAttribute d) {
+            for (int i = 0; i < parentNames.size(); i++) {
+              if (parentNames.get(i).equals(d.revision)) {
+                return i;
+              }
+            }
+            return parentNames.size() + 1;
+          }
+        }));
+  }
+
+  private void addNeededBy(RevWalk rw, ChangeAttribute ca, Change change,
+      PatchSet currentPs) throws OrmException, IOException {
+    if (currentPs.getGroups() == null || currentPs.getGroups().isEmpty()) {
+      return;
+    }
+    String rev = currentPs.getRevision().get();
+    // Find changes in the same related group as this patch set, having a patch
+    // set whose parent matches this patch set's revision.
+    for (ChangeData cd : queryProvider.get().byProjectGroups(
+        change.getProject(), currentPs.getGroups())) {
+      patchSets: for (PatchSet ps : cd.patchSets()) {
+        RevCommit commit =
+            rw.parseCommit(ObjectId.fromString(ps.getRevision().get()));
+        for (RevCommit p : commit.getParents()) {
+          if (!p.name().equals(rev)) {
+            continue;
+          }
+          ca.neededBy.add(newNeededBy(checkNotNull(cd.change()), ps));
+          continue patchSets;
+        }
+      }
+    }
+  }
+
   private DependencyAttribute newDependsOn(Change c, PatchSet ps) {
     DependencyAttribute d = newDependencyAttribute(c, ps);
     d.isCurrentPatchSet = ps.getId().equals(c.currentPatchSetId());
@@ -319,24 +351,21 @@ public class EventFactory {
     a.commitMessage = commitMessage;
   }
 
-  public void addPatchSets(ChangeAttribute a, Collection<PatchSet> ps,
+  public void addPatchSets(ReviewDb db, RevWalk revWalk, ChangeAttribute ca,
-      LabelTypes labelTypes) {
+      Collection<PatchSet> ps,
-    addPatchSets(a, ps, null, false, null, labelTypes);
-  }
-
-  public void addPatchSets(ChangeAttribute ca, Collection<PatchSet> ps,
       Map<PatchSet.Id, Collection<PatchSetApproval>> approvals,
       LabelTypes labelTypes) {
-    addPatchSets(ca, ps, approvals, false, null, labelTypes);
+    addPatchSets(db, revWalk, ca, ps, approvals, false, null, labelTypes);
   }
 
-  public void addPatchSets(ChangeAttribute ca, Collection<PatchSet> ps,
+  public void addPatchSets(ReviewDb db, RevWalk revWalk, ChangeAttribute ca,
+      Collection<PatchSet> ps,
       Map<PatchSet.Id, Collection<PatchSetApproval>> approvals,
       boolean includeFiles, Change change, LabelTypes labelTypes) {
     if (!ps.isEmpty()) {
       ca.patchSets = new ArrayList<>(ps.size());
       for (PatchSet p : ps) {
-        PatchSetAttribute psa = asPatchSetAttribute(p);
+        PatchSetAttribute psa = asPatchSetAttribute(db, revWalk, p);
         if (approvals != null) {
           addApprovals(psa, p.getId(), approvals, labelTypes);
         }
@@ -400,7 +429,8 @@ public class EventFactory {
    * @param patchSet
    * @return object suitable for serialization to JSON
    */
-  public PatchSetAttribute asPatchSetAttribute(final PatchSet patchSet) {
+  public PatchSetAttribute asPatchSetAttribute(ReviewDb db, RevWalk revWalk,
+      PatchSet patchSet) {
     PatchSetAttribute p = new PatchSetAttribute();
     p.revision = patchSet.getRevision().get();
     p.number = Integer.toString(patchSet.getPatchSetId());
@@ -408,12 +438,12 @@ public class EventFactory {
     p.uploader = asAccountAttribute(patchSet.getUploader());
     p.createdOn = patchSet.getCreatedOn().getTime() / 1000L;
     p.isDraft = patchSet.isDraft();
-    final PatchSet.Id pId = patchSet.getId();
+    PatchSet.Id pId = patchSet.getId();
-    try (ReviewDb db = schema.open()) {
+    try {
       p.parents = new ArrayList<>();
-      for (PatchSetAncestor a : db.patchSetAncestors().ancestorsOf(
+      RevCommit c = revWalk.parseCommit(ObjectId.fromString(p.revision));
-          patchSet.getId())) {
+      for (RevCommit parent : c.getParents()) {
-        p.parents.add(a.getAncestorRevision().get());
+        p.parents.add(parent.name());
       }
 
       UserIdentity author = psInfoFactory.get(db, pId).getAuthor();
@@ -436,7 +466,7 @@ public class EventFactory {
         }
       }
       p.kind = changeKindCache.getChangeKind(db, change, patchSet);
-    } catch (OrmException e) {
+    } catch (OrmException | IOException e) {
       log.error("Cannot load patch set data for " + patchSet.getId(), e);
     } catch (PatchSetInfoNotAvailableException e) {
       log.error(String.format("Cannot get authorEmail for %s.", pId), e);
@@ -491,7 +521,7 @@ public class EventFactory {
    * @param account
    * @return object suitable for serialization to JSON
    */
-  public AccountAttribute asAccountAttribute(final Account account) {
+  public AccountAttribute asAccountAttribute(Account account) {
     if (account == null) {
       return null;
     }
@@ -560,9 +590,9 @@ public class EventFactory {
   }
 
   /** Get a link to the change; null if the server doesn't know its own address. */
-  private String getChangeUrl(final Change change) {
+  private String getChangeUrl(Change change) {
     if (change != null && urlProvider.get() != null) {
-      final StringBuilder r = new StringBuilder();
+      StringBuilder r = new StringBuilder();
       r.append(urlProvider.get());
       r.append(change.getChangeId());
       return r.toString();

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -2252,7 +2252,6 @@ public class ReceiveCommits {
           return null;
         }
 
-        ChangeUtil.insertAncestors(db, newPatchSet.getId(), newCommit);
         if (newPatchSet.getGroups() == null) {
           newPatchSet.setGroups(GroupCollector.getCurrentGroups(db, change));
         }

gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java

@@ -19,11 +19,9 @@ import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetAncestor;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.PatchSetInfo;
 import com.google.gerrit.reviewdb.client.RevId;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.git.BatchUpdate;
 import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
@@ -42,11 +40,9 @@ import com.google.gwtorm.server.OrmException;
 
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.transport.ReceiveCommand;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -187,7 +183,6 @@ public class CherryPick extends SubmitStrategy {
       Change c = toMerge.change();
       ps.setGroups(GroupCollector.getCurrentGroups(args.db, c));
       args.db.patchSets().insert(Collections.singleton(ps));
-      insertAncestors(args.db, ps.getId(), newCommit);
       c.setCurrentPatchSet(patchSetInfo);
       args.db.changes().update(Collections.singletonList(c));
 
@@ -247,20 +242,6 @@ public class CherryPick extends SubmitStrategy {
     }
   }
 
-  private static void insertAncestors(ReviewDb db, PatchSet.Id id,
-      RevCommit src) throws OrmException {
-    int cnt = src.getParentCount();
-    List<PatchSetAncestor> toInsert = new ArrayList<>(cnt);
-    for (int p = 0; p < cnt; p++) {
-      PatchSetAncestor a;
-
-      a = new PatchSetAncestor(new PatchSetAncestor.Id(id, p + 1));
-      a.setAncestorRevision(new RevId(src.getParent(p).getId().name()));
-      toInsert.add(a);
-    }
-    db.patchSetAncestors().insert(toInsert);
-  }
-
   @Override
   public Map<Change.Id, CodeReviewCommit> getNewCommits() {
     return newCommits;

gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.query.change;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.gerrit.server.index.ChangeField.SUBMISSIONID;
 import static com.google.gerrit.server.query.Predicate.and;
 import static com.google.gerrit.server.query.Predicate.not;
@@ -215,6 +216,13 @@ public class InternalChangeQuery {
     return query(commit(schema(indexes), id.name()));
   }
 
+  public List<ChangeData> byProjectCommits(Project.NameKey project,
+      List<String> hashes) throws OrmException {
+    int n = indexConfig.maxTerms() - 1;
+    checkArgument(hashes.size() <= n, "cannot exceed %s commits", n);
+    return query(and(project(project), or(commits(schema(indexes), hashes))));
+  }
+
   public List<ChangeData> bySubmissionId(String cs) throws OrmException {
     if (Strings.isNullOrEmpty(cs) || !schema(indexes).hasField(SUBMISSIONID)) {
       return Collections.emptyList();

gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java

@@ -14,24 +14,31 @@
 
 package com.google.gerrit.server.query.change;
 
+import static com.google.common.base.Preconditions.checkState;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.config.TrackingFooters;
 import com.google.gerrit.server.data.ChangeAttribute;
 import com.google.gerrit.server.data.PatchSetAttribute;
 import com.google.gerrit.server.data.QueryStatsAttribute;
 import com.google.gerrit.server.events.EventFactory;
+import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.QueryParseException;
 import com.google.gson.Gson;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.util.io.DisabledOutputStream;
 import org.joda.time.format.DateTimeFormat;
 import org.joda.time.format.DateTimeFormatter;
@@ -47,7 +54,9 @@ import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 /**
  * Change query implementation that outputs to a stream in the style of an SSH
@@ -64,6 +73,8 @@ public class OutputStreamQuery {
     TEXT, JSON
   }
 
+  private final Provider<ReviewDb> db;
+  private final GitRepositoryManager repoManager;
   private final ChangeQueryBuilder queryBuilder;
   private final QueryProcessor queryProcessor;
   private final EventFactory eventFactory;
@@ -86,11 +97,15 @@ public class OutputStreamQuery {
 
   @Inject
   OutputStreamQuery(
+      Provider<ReviewDb> db,
+      GitRepositoryManager repoManager,
       ChangeQueryBuilder queryBuilder,
       QueryProcessor queryProcessor,
       EventFactory eventFactory,
       TrackingFooters trackingFooters,
       CurrentUser user) {
+    this.db = db;
+    this.repoManager = repoManager;
     this.queryBuilder = queryBuilder;
     this.queryProcessor = queryProcessor;
     this.eventFactory = eventFactory;
@@ -179,76 +194,16 @@ public class OutputStreamQuery {
         final QueryStatsAttribute stats = new QueryStatsAttribute();
         stats.runTimeMilliseconds = TimeUtil.nowMs();
 
+        Map<Project.NameKey, Repository> repos = new HashMap<>();
+        Map<Project.NameKey, RevWalk> revWalks = new HashMap<>();
         QueryResult results =
             queryProcessor.queryChanges(queryBuilder.parse(queryString));
-        ChangeAttribute c = null;
+        try {
           for (ChangeData d : results.changes()) {
-          ChangeControl cc = d.changeControl().forUser(user);
+            show(buildChangeAttribute(d, repos, revWalks));
-
-          LabelTypes labelTypes = cc.getLabelTypes();
-          c = eventFactory.asChangeAttribute(d.change());
-          eventFactory.extend(c, d.change());
-
-          if (!trackingFooters.isEmpty()) {
-            eventFactory.addTrackingIds(c,
-                trackingFooters.extract(d.commitFooters()));
           }
-
+        } finally {
-          if (includeAllReviewers) {
+          closeAll(revWalks.values(), repos.values());
-            eventFactory.addAllReviewers(c, d.notes());
-          }
-
-          if (includeSubmitRecords) {
-            eventFactory.addSubmitRecords(c, new SubmitRuleEvaluator(d)
-                .setAllowClosed(true)
-                .setAllowDraft(true)
-                .evaluate());
-          }
-
-          if (includeCommitMessage) {
-            eventFactory.addCommitMessage(c, d.commitMessage());
-          }
-
-          if (includePatchSets) {
-            if (includeFiles) {
-              eventFactory.addPatchSets(c, d.patchSets(),
-                includeApprovals ? d.approvals().asMap() : null,
-                includeFiles, d.change(), labelTypes);
-            } else {
-              eventFactory.addPatchSets(c, d.patchSets(),
-                  includeApprovals ? d.approvals().asMap() : null,
-                  labelTypes);
-            }
-          }
-
-          if (includeCurrentPatchSet) {
-            PatchSet current = d.currentPatchSet();
-            if (current != null) {
-              c.currentPatchSet = eventFactory.asPatchSetAttribute(current);
-              eventFactory.addApprovals(c.currentPatchSet,
-                  d.currentApprovals(), labelTypes);
-
-              if (includeFiles) {
-                eventFactory.addPatchSetFileNames(c.currentPatchSet,
-                    d.change(), d.currentPatchSet());
-              }
-            }
-          }
-
-          if (includeComments) {
-            eventFactory.addComments(c, d.messages());
-            if (includePatchSets) {
-              for (PatchSetAttribute attribute : c.patchSets) {
-                eventFactory.addPatchSetComments(attribute,  d.publishedComments());
-              }
-            }
-          }
-
-          if (includeDependencies) {
-            eventFactory.addDependencies(c, d.change());
-          }
-
-          show(c);
         }
 
         stats.rowCount = results.changes().size();
@@ -277,6 +232,107 @@ public class OutputStreamQuery {
     }
   }
 
+  private ChangeAttribute buildChangeAttribute(ChangeData d,
+      Map<Project.NameKey, Repository> repos,
+      Map<Project.NameKey, RevWalk> revWalks)
+      throws OrmException, IOException {
+    ChangeControl cc = d.changeControl().forUser(user);
+
+    LabelTypes labelTypes = cc.getLabelTypes();
+    ChangeAttribute c = eventFactory.asChangeAttribute(db.get(), d.change());
+    eventFactory.extend(c, d.change());
+
+    if (!trackingFooters.isEmpty()) {
+      eventFactory.addTrackingIds(c,
+          trackingFooters.extract(d.commitFooters()));
+    }
+
+    if (includeAllReviewers) {
+      eventFactory.addAllReviewers(db.get(), c, d.notes());
+    }
+
+    if (includeSubmitRecords) {
+      eventFactory.addSubmitRecords(c, new SubmitRuleEvaluator(d)
+          .setAllowClosed(true)
+          .setAllowDraft(true)
+          .evaluate());
+    }
+
+    if (includeCommitMessage) {
+      eventFactory.addCommitMessage(c, d.commitMessage());
+    }
+
+    RevWalk rw = null;
+    if (includePatchSets || includeCurrentPatchSet || includeDependencies) {
+      Project.NameKey p = d.change().getProject();
+      rw = revWalks.get(p);
+      // Cache and reuse repos and revwalks.
+      if (rw == null) {
+        Repository repo = repoManager.openRepository(p);
+        checkState(repos.put(p, repo) == null);
+        rw = new RevWalk(repo);
+        revWalks.put(p, rw);
+      }
+    }
+
+    if (includePatchSets) {
+      if (includeFiles) {
+        eventFactory.addPatchSets(db.get(), rw, c, d.patchSets(),
+          includeApprovals ? d.approvals().asMap() : null,
+          includeFiles, d.change(), labelTypes);
+      } else {
+        eventFactory.addPatchSets(db.get(), rw, c, d.patchSets(),
+            includeApprovals ? d.approvals().asMap() : null,
+            labelTypes);
+      }
+    }
+
+    if (includeCurrentPatchSet) {
+      PatchSet current = d.currentPatchSet();
+      if (current != null) {
+        c.currentPatchSet =
+            eventFactory.asPatchSetAttribute(db.get(), rw, current);
+        eventFactory.addApprovals(c.currentPatchSet,
+            d.currentApprovals(), labelTypes);
+
+        if (includeFiles) {
+          eventFactory.addPatchSetFileNames(c.currentPatchSet,
+              d.change(), d.currentPatchSet());
+        }
+      }
+    }
+
+    if (includeComments) {
+      eventFactory.addComments(c, d.messages());
+      if (includePatchSets) {
+        for (PatchSetAttribute attribute : c.patchSets) {
+          eventFactory.addPatchSetComments(
+              attribute, d.publishedComments());
+        }
+      }
+    }
+
+    if (includeDependencies) {
+      eventFactory.addDependencies(rw, c, d.change(), d.currentPatchSet());
+    }
+
+    return c;
+  }
+
+  private static void closeAll(Iterable<RevWalk> revWalks,
+      Iterable<Repository> repos) {
+    if (repos != null) {
+      for (Repository repo : repos) {
+        repo.close();
+      }
+    }
+    if (revWalks != null) {
+      for (RevWalk revWalk : revWalks) {
+        revWalk.close();
+      }
+    }
+  }
+
   private void show(Object data) {
     switch (outputFormat) {
       default:

gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java

@@ -32,7 +32,7 @@ import java.util.List;
 /** A version of the database schema. */
 public abstract class SchemaVersion {
   /** The current schema version. */
-  public static final Class<Schema_113> C = Schema_113.class;
+  public static final Class<Schema_114> C = Schema_114.class;
 
   public static int getBinaryVersion() {
     return guessVersion(C);

gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_114.java

@@ -0,0 +1,25 @@
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.schema;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+public class Schema_114 extends SchemaVersion {
+  @Inject
+  Schema_114(Provider<Schema_113> prior) {
+    super(prior);
+  }
+}

gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java

@@ -30,7 +30,6 @@ import com.google.gerrit.reviewdb.server.ChangeAccess;
 import com.google.gerrit.reviewdb.server.ChangeMessageAccess;
 import com.google.gerrit.reviewdb.server.PatchLineCommentAccess;
 import com.google.gerrit.reviewdb.server.PatchSetAccess;
-import com.google.gerrit.reviewdb.server.PatchSetAncestorAccess;
 import com.google.gerrit.reviewdb.server.PatchSetApprovalAccess;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.reviewdb.server.SchemaVersionAccess;
@@ -159,11 +158,6 @@ public class DisabledReviewDb implements ReviewDb {
     throw new AssertionError(MESSAGE);
   }
 
-  @Override
-  public PatchSetAncestorAccess patchSetAncestors() {
-    throw new AssertionError(MESSAGE);
-  }
-
   @Override
   public PatchLineCommentAccess patchComments() {
     throw new AssertionError(MESSAGE);