Fix a bug where "LIMIT ?" in queries was omitted in the generated query

The LIMIT clause was only honored when followed by a constant integer.  When
followed by a placeholder "?" it wasn't included in the generated database
query.

This could cause performance issues when selecting a few rows from a large
table like, for example, when paging through merged changes in Gerrit UI. In
this case, for every page, most of the closed changes would be selected from
the Changes table in order to display the next 26 changes. On one of our
productive Gerrit servers, running on the 2.7 release, it took often more than
20 seconds to show a next page. With this fix the time is reduced to several
milliseconds.

Change-Id: Ie18898242d9c590eeb4f3eae895d955cd04e1d66
diff --git a/src/main/java/com/google/gwtorm/jdbc/AccessGen.java b/src/main/java/com/google/gwtorm/jdbc/AccessGen.java
index d1e5b5c..7cba532 100644
--- a/src/main/java/com/google/gwtorm/jdbc/AccessGen.java
+++ b/src/main/java/com/google/gwtorm/jdbc/AccessGen.java
@@ -736,6 +736,20 @@
       if (hasLimitParam || !dialect.selectHasLimit()) {
         mv.visitVarInsn(ALOAD, psvar);
         if (hasLimitParam) {
+          CodeGenSupport cgs2 = new CodeGenSupport(mv) {
+            @Override
+            public void pushSqlHandle() {
+              mv.visitVarInsn(ALOAD, psvar);
+            }
+
+            @Override
+            public void pushFieldValue() {
+              final int n = argIdx[0];
+              loadVar(pTypes[n], pVars[n]);
+            }
+          };
+          cgs2.resetColumnIndex(cgs.getColumnIndex() + 1);
+          dialect.getSqlTypeInfo(Integer.TYPE).generatePreparedStatementSet(cgs2);
           mv.visitVarInsn(ILOAD, pVars[pTypes.length - 1]);
         } else {
           cgs.push(info.getStaticLimit());
diff --git a/src/main/java/com/google/gwtorm/schema/QueryModel.java b/src/main/java/com/google/gwtorm/schema/QueryModel.java
index 0730d78..db776a7 100644
--- a/src/main/java/com/google/gwtorm/schema/QueryModel.java
+++ b/src/main/java/com/google/gwtorm/schema/QueryModel.java
@@ -339,7 +339,8 @@
       case QueryParser.LIMIT:
         if (fmt.dialect.selectHasLimit()) {
           final Tree p = node.getChild(0);
-          if (p.getType() == QueryParser.CONSTANT_INTEGER) {
+          if (p.getType() == QueryParser.CONSTANT_INTEGER
+              || p.getType() == QueryParser.PLACEHOLDER) {
             fmt.buf.append(" LIMIT ");
             fmt.buf.append(p.getText());
           }
diff --git a/src/test/java/com/google/gwtorm/schema/QueryModelTest.java b/src/test/java/com/google/gwtorm/schema/QueryModelTest.java
new file mode 100644
index 0000000..13e914c
--- /dev/null
+++ b/src/test/java/com/google/gwtorm/schema/QueryModelTest.java
@@ -0,0 +1,64 @@
+// Copyright 2014 Google Inc.
+//
+// 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;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import com.google.gwtorm.data.PhoneBookDb;
+import com.google.gwtorm.schema.java.JavaSchemaModel;
+import com.google.gwtorm.schema.sql.DialectH2;
+import com.google.gwtorm.server.OrmException;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.List;
+
+public class QueryModelTest {
+
+  private RelationModel people;
+
+  @Before
+  public void setUp() throws OrmException {
+    JavaSchemaModel schema = new JavaSchemaModel(PhoneBookDb.class);
+    people = schema.getRelation("people");
+  }
+
+  @Test
+  public void testLimitWithConstant() throws OrmException {
+    QueryModel qm = new QueryModel(people, null, "LIMIT 5");
+    List<ColumnModel> params = qm.getParameters();
+    assertNotNull(params);
+    assertTrue(params.size() == 0);
+    assertTrue(qm.hasLimit());
+
+    String sql = qm.getSelectSql(new DialectH2(), "T");
+    assertEquals("SELECT T.age,T.registered,T.name FROM people T LIMIT 5", sql);
+  }
+
+  @Test
+  public void testLimitWithPlaceholder() throws OrmException {
+    QueryModel qm = new QueryModel(people, null, "LIMIT ?");
+    List<ColumnModel> params = qm.getParameters();
+    assertNotNull(params);
+    assertTrue(params.size() == 0);
+    assertTrue(qm.hasLimit());
+
+    String sql = qm.getSelectSql(new DialectH2(), "T");
+    assertEquals("SELECT T.age,T.registered,T.name FROM people T LIMIT ?", sql);
+  }
+}
diff --git a/src/test/java/com/google/gwtorm/schema/QueryParserTest.java b/src/test/java/com/google/gwtorm/schema/QueryParserTest.java
index f117549..ada20c9 100644
--- a/src/test/java/com/google/gwtorm/schema/QueryParserTest.java
+++ b/src/test/java/com/google/gwtorm/schema/QueryParserTest.java
@@ -197,4 +197,28 @@
       assertEquals("a", ((QueryParser.Column) aId).getField().getFieldName());
     }
   }
+
+  @Test
+  public void testLimitWithConstant() throws QueryParseException {
+    final Tree t = parse("LIMIT 1");
+    assertNotNull(t);
+    assertEquals(QueryParser.LIMIT, t.getType());
+    assertEquals(1, t.getChildCount());
+    {
+      final Tree c = t.getChild(0);
+      assertEquals(QueryParser.CONSTANT_INTEGER, c.getType());
+    }
+  }
+
+  @Test
+  public void testLimitWithPlaceholder() throws QueryParseException {
+    final Tree t = parse("LIMIT ?");
+    assertNotNull(t);
+    assertEquals(QueryParser.LIMIT, t.getType());
+    assertEquals(1, t.getChildCount());
+    {
+      final Tree c = t.getChild(0);
+      assertEquals(QueryParser.PLACEHOLDER, c.getType());
+    }
+  }
 }