From 258c6412264edc97209210f9f6da0ef04a399980 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 24 Sep 2013 15:39:08 -0700 Subject: [PATCH] Rename new sortkey index field to sortkey2 For technical reasons googlesource.com's secondary index implementation needs to construct a document containing the union of all fields from all active write schema versions; for this reason we don't allow distinct fields in different versions to have the same name. This fact was missed in Icb760dba. Don't bump the schema version since that change has only been submitted for a few hours. Change-Id: I4f8a2e515372be3a4b4a145a0b54eef864dcea00 --- .../gerrit/server/index/ChangeField.java | 2 +- .../gerrit/server/index/IndexRewriteImpl.java | 4 +- .../query/change/BasicChangeRewrites.java | 15 ++++++- .../query/change/ChangeQueryBuilder.java | 6 ++- .../server/query/change/SortKeyPredicate.java | 43 ++++++++++++++----- .../server/query/change/SqlRewriterImpl.java | 2 +- .../gerrit/server/index/IndexRewriteTest.java | 2 +- 7 files changed, 55 insertions(+), 19 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java index cfa4644a64..0ee43e9060 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java @@ -153,7 +153,7 @@ public class ChangeField { */ public static final FieldDef SORTKEY = new FieldDef.Single( - "sortkey", FieldType.LONG, true) { + "sortkey2", FieldType.LONG, true) { @Override public Long get(ChangeData input, FillArgs args) throws OrmException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java index bbc4773db1..4d8c95153c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java @@ -278,8 +278,8 @@ public class IndexRewriteImpl implements ChangeQueryRewriter { new QueryRewriter.Definition( BasicRewritesImpl.class, SqlRewriterImpl.BUILDER); @Inject - BasicRewritesImpl(Provider db) { - super(mydef, db); + BasicRewritesImpl(Provider db, IndexCollection indexes) { + super(mydef, db, indexes); } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java index 1d2c9d2766..f81b0ca82f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java @@ -14,9 +14,13 @@ package com.google.gerrit.server.query.change; +import com.google.gerrit.common.Nullable; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.index.ChangeIndex; +import com.google.gerrit.server.index.IndexCollection; +import com.google.gerrit.server.index.Schema; import com.google.gerrit.server.query.IntPredicate; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryRewriter; @@ -32,13 +36,20 @@ public abstract class BasicChangeRewrites extends QueryRewriter { null, null, null, null, null, // null, null, null, null, null), null); + static Schema schema(@Nullable IndexCollection indexes) { + ChangeIndex index = indexes != null ? indexes.getSearchIndex() : null; + return index != null ? index.getSchema() : null; + } + protected final Provider dbProvider; + private final IndexCollection indexes; protected BasicChangeRewrites( Definition> def, - Provider dbProvider) { + Provider dbProvider, IndexCollection indexes) { super(def); this.dbProvider = dbProvider; + this.indexes = indexes; } @Rewrite("-status:open") @@ -74,7 +85,7 @@ public abstract class BasicChangeRewrites extends QueryRewriter { @Rewrite("sortkey_before:z A=(age:*)") public Predicate r00_ageToSortKey(@Named("A") AgePredicate a) { String cut = ChangeUtil.sortKey(a.getCut(), Integer.MAX_VALUE); - return and(new SortKeyPredicate.Before(dbProvider, cut), a); + return and(new SortKeyPredicate.Before(schema(indexes), dbProvider, cut), a); } @NoCostComputation diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index 39abb5947f..760287c5b3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -579,12 +579,14 @@ public class ChangeQueryBuilder extends QueryBuilder { @Operator public Predicate sortkey_after(String sortKey) { - return new SortKeyPredicate.After(args.dbProvider, sortKey); + return new SortKeyPredicate.After( + BasicChangeRewrites.schema(args.indexes), args.dbProvider, sortKey); } @Operator public Predicate sortkey_before(String sortKey) { - return new SortKeyPredicate.Before(args.dbProvider, sortKey); + return new SortKeyPredicate.Before( + BasicChangeRewrites.schema(args.indexes), args.dbProvider, sortKey); } @Operator diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SortKeyPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SortKeyPredicate.java index a502746ca3..c61294501a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SortKeyPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SortKeyPredicate.java @@ -14,6 +14,10 @@ package com.google.gerrit.server.query.change; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.gerrit.server.index.ChangeField.SORTKEY; + +import com.google.gerrit.common.Nullable; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ChangeUtil; @@ -27,18 +31,35 @@ import com.google.inject.Provider; public abstract class SortKeyPredicate extends IndexPredicate { @SuppressWarnings("deprecation") private static long parseSortKey(Schema schema, String value) { - FieldDef field = schema.getFields().get(ChangeField.SORTKEY.getName()); - if (field == ChangeField.SORTKEY) { + FieldDef field = schema.getFields().get(SORTKEY.getName()); + if (field == SORTKEY) { return ChangeUtil.parseSortKey(value); } else { return ChangeField.legacyParseSortKey(value); } } + @SuppressWarnings("deprecation") + private static FieldDef sortkeyField(Schema schema) { + if (schema == null) { + return ChangeField.LEGACY_SORTKEY; + } + FieldDef f = schema.getFields().get(SORTKEY.getName()); + if (f != null) { + return f; + } + return checkNotNull( + schema.getFields().get(ChangeField.LEGACY_SORTKEY.getName()), + "schema missing sortkey field, found: %s", schema.getFields().keySet()); + } + + protected final Schema schema; protected final Provider dbProvider; - SortKeyPredicate(Provider dbProvider, String name, String value) { - super(ChangeField.SORTKEY, name, value); + SortKeyPredicate(Schema schema, Provider dbProvider, + String name, String value) { + super(sortkeyField(schema), name, value); + this.schema = schema; this.dbProvider = dbProvider; } @@ -52,8 +73,9 @@ public abstract class SortKeyPredicate extends IndexPredicate { public abstract SortKeyPredicate copy(String newValue); public static class Before extends SortKeyPredicate { - Before(Provider dbProvider, String value) { - super(dbProvider, "sortkey_before", value); + Before(@Nullable Schema schema, Provider dbProvider, + String value) { + super(schema, dbProvider, "sortkey_before", value); } @Override @@ -74,13 +96,14 @@ public abstract class SortKeyPredicate extends IndexPredicate { @Override public Before copy(String newValue) { - return new Before(dbProvider, newValue); + return new Before(schema, dbProvider, newValue); } } public static class After extends SortKeyPredicate { - After(Provider dbProvider, String value) { - super(dbProvider, "sortkey_after", value); + After(@Nullable Schema schema, Provider dbProvider, + String value) { + super(schema, dbProvider, "sortkey_after", value); } @Override @@ -101,7 +124,7 @@ public abstract class SortKeyPredicate extends IndexPredicate { @Override public After copy(String newValue) { - return new After(dbProvider, newValue); + return new After(schema, dbProvider, newValue); } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SqlRewriterImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SqlRewriterImpl.java index 71daa9cc18..34fe3fb102 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SqlRewriterImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SqlRewriterImpl.java @@ -40,7 +40,7 @@ public class SqlRewriterImpl extends BasicChangeRewrites @Inject @VisibleForTesting public SqlRewriterImpl(Provider dbProvider) { - super(mydef, dbProvider); + super(mydef, dbProvider, null); } @Override diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java index 08617b6850..f9bb4f3a30 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java @@ -54,7 +54,7 @@ public class IndexRewriteTest extends TestCase { rewrite = new IndexRewriteImpl( indexes, null, - new IndexRewriteImpl.BasicRewritesImpl(null), + new IndexRewriteImpl.BasicRewritesImpl(null, indexes), new SqlRewriterImpl(null)); }