Order the primary key columns by the @Column.id()
Since 1.6 we created compound primary key indexes ordered by
depth-first traversal of the primary key using the alphabetical order
of fields. This created many compound primary key indexes with a
wrong column order which lead to poor performance in some Gerrit
database queries which relied on using a prefix of the primary key
index columns.
Up to and including 1.4 we use the depth first traversal of the
primary key, using field declaration order. This worked well with
Java 6 but had to be fixed when switching to Java 7 and a new ordering
was imposed (between 1.4 and 1.6).
To fix the issue of the column order order the primary key fields
by:
- depth first traversal
- @Column annotated fields before non-annotated
- @Column annotated fields ordered by the @Column.id()
- non-annotated fields ordered alphabetically
Change-Id: Icd5a644587c6af442172797e0a52a7eea4512e79
diff --git a/src/main/java/com/google/gwtorm/schema/java/JavaColumnModel.java b/src/main/java/com/google/gwtorm/schema/java/JavaColumnModel.java
index 6b7680b..683e5a5 100644
--- a/src/main/java/com/google/gwtorm/schema/java/JavaColumnModel.java
+++ b/src/main/java/com/google/gwtorm/schema/java/JavaColumnModel.java
@@ -33,12 +33,27 @@
public class JavaColumnModel extends ColumnModel {
public static List<Field> getDeclaredFields(Class<?> in) {
- return Ordering.natural().onResultOf(new Function<Field, String>() {
+ return sort(Arrays.asList(in.getDeclaredFields()));
+ }
+
+ static List<Field> sort(List<Field> fields) {
+ return new Ordering<Field>() {
@Override
- public String apply(Field f) {
- return f.getName();
+ public int compare(Field f1, Field f2) {
+ Column a1 = f1.getAnnotation(Column.class);
+ Column a2 = f2.getAnnotation(Column.class);
+ if (a1 == null && a2 == null) {
+ return f1.getName().compareTo(f2.getName());
+ }
+ if (a1 == null) {
+ return 1;
+ }
+ if (a2 == null) {
+ return -1;
+ }
+ return a1.id() - a2.id();
}
- }).sortedCopy(Arrays.asList(in.getDeclaredFields()));
+ }.sortedCopy(fields);
}
private final Field field;
diff --git a/src/test/java/com/google/gwtorm/schema/java/DeclaredFieldsOrderTest.java b/src/test/java/com/google/gwtorm/schema/java/DeclaredFieldsOrderTest.java
new file mode 100644
index 0000000..adae52b
--- /dev/null
+++ b/src/test/java/com/google/gwtorm/schema/java/DeclaredFieldsOrderTest.java
@@ -0,0 +1,58 @@
+// Copyright (C) 2014 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.gwtorm.schema.java;
+
+import static com.google.gwtorm.schema.java.JavaColumnModel.sort;
+import static com.google.common.collect.Collections2.permutations;
+
+import static org.junit.Assert.assertEquals;
+
+import com.google.gwtorm.client.Column;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.Test;
+
+public class DeclaredFieldsOrderTest {
+
+ private static class C {
+ @Column(id = 1)
+ private String c;
+ @Column(id = 2)
+ private String b;
+ @Column(id = 3)
+ private String a;
+
+ private String d;
+ private String e;
+ private String f;
+ }
+
+ @Test
+ public void testFieldSorting() {
+ List<Field> fields = Arrays.asList(C.class.getDeclaredFields());
+ for (List<Field> p : permutations(fields)) {
+ List<Field> sorted = sort(p);
+ assertEquals("c", sorted.get(0).getName());
+ assertEquals("b", sorted.get(1).getName());
+ assertEquals("a", sorted.get(2).getName());
+ assertEquals("d", sorted.get(3).getName());
+ assertEquals("e", sorted.get(4).getName());
+ assertEquals("f", sorted.get(5).getName());
+ }
+ }
+}
