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()); + } + } +}