Fixed multiple bugs in query building

* ORDER BY, LIMIT and OFFSET parameters cannot be bound to the query
as the database surrounds them with quotes, formining invalid SQL.
They are now simply concatenated to the SQL string. The documentation
has been updated to mark these as "unsafe" and not suitable for use with
unfiltered user input.

* ORDER BY should come before LIMIT and OFFSET.

3 files changed, 23 insertions, 20 deletions

diff --git a/README.markdown b/README.markdown
index 1f6ba17..0aefe23 100644
--- a/README.markdown
+++ b/README.markdown
@@ -55,6 +55,8 @@ Idiorm provides a [*fluent interface*](http://en.wikipedia.org/wiki/Fluent_inter
 
 All Idiorm queries start with a call to the `for_table` static method on the ORM class. This tells the ORM which table to use when making the query. Method calls which are then strung together. Finally, the chain is finished by calling either `find_one()` or `find_many()`, which executes the query and returns the result.
 
+*Note that this method **does not** escape its query parameter and so the table name should **not** be passed directly from user input.*
+
 Let's start with a simple example. Say we have a table called `person` which contains the columns `id` (the primary key of the record - Idiorm assumes the primary key column is called `id` but this is configurable, see below), `name`, `age` and `gender`.
 
 #### Single records ####
@@ -107,12 +109,16 @@ Note that this method only supports "question mark placeholder" syntax, and NOT
 
 #### LIMIT and OFFSET ####
 
+*Note that these methods **do not** escape their query parameters and so these should **not** be passed directly from user input.*
+
 The `limit` and `offset` methods map pretty closely to their SQL equivalents.
 
     $people = ORM::for_table('person')->where('gender', 'female')->limit(5)->offset(10)->find_many();
 
 #### ORDER BY ####
 
+*Note that this method **does not** escape its query parameter and so this should **not** be passed directly from user input.*
+
 Two methods are provided to add `ORDER BY` clauses to your query. These are `order_by_desc` and `order_by_asc`, each of which takes a column name to sort by.
 
     $people = ORM::for_table('person')->order_by_asc('gender')->order_by_desc('name')->find_many();
diff --git a/idiorm.php b/idiorm.php
index f3f601a..4db2d67 100644
--- a/idiorm.php
+++ b/idiorm.php
@@ -422,23 +422,10 @@
                 }
             }
 
-            // Add LIMIT if present
-            if (!is_null($this->limit)) {
-                $query[] = "LIMIT ?";
-                $this->values[] = $this->limit;
-            }
-
-            // Add OFFSET if present
-            if (!is_null($this->offset)) {
-                $query[] = "OFFSET ?";
-                $this->values[] = $this->offset;
-            }
-
             // Add ORDER BY clause(s)
             $order_by = array();
             foreach ($this->order_by as $order) {
-                $order_by[] = "? " . $order[self::ORDER_BY_ORDERING];
-                $this->values[] = $order[self::ORDER_BY_COLUMN_NAME];
+                $order_by[] = $order[self::ORDER_BY_COLUMN_NAME] . " " . $order[self::ORDER_BY_ORDERING];
             }
 
             if (count($order_by) != 0) {
@@ -446,6 +433,16 @@
                 $query[] = join(", ", $order_by);
             }
 
+            // Add LIMIT if present
+            if (!is_null($this->limit)) {
+                $query[] = "LIMIT " . $this->limit;
+            }
+
+            // Add OFFSET if present
+            if (!is_null($this->offset)) {
+                $query[] = "OFFSET " . $this->offset;
+            }
+
             return join(" ", $query);
         }
 
diff --git a/test/test_queries.php b/test/test_queries.php
index ac90c5e..249b1df 100644
--- a/test/test_queries.php
+++ b/test/test_queries.php
@@ -35,27 +35,27 @@
     Tester::check_equal("where_like method", $expected);
 
     ORM::for_table('widget')->limit(5)->find_one();
-    $expected = 'SELECT * FROM widget LIMIT "5"';
+    $expected = 'SELECT * FROM widget LIMIT 5';
     Tester::check_equal("LIMIT clause", $expected);
 
     ORM::for_table('widget')->limit(5)->offset(5)->find_one();
-    $expected = 'SELECT * FROM widget LIMIT "5" OFFSET "5"';
+    $expected = 'SELECT * FROM widget LIMIT 5 OFFSET 5';
     Tester::check_equal("LIMIT and OFFSET clause", $expected);
 
     ORM::for_table('widget')->order_by_desc('name')->find_one();
-    $expected = 'SELECT * FROM widget ORDER BY "name" DESC';
+    $expected = 'SELECT * FROM widget ORDER BY name DESC';
     Tester::check_equal("ORDER BY DESC", $expected);
 
     ORM::for_table('widget')->order_by_asc('name')->find_one();
-    $expected = 'SELECT * FROM widget ORDER BY "name" ASC';
+    $expected = 'SELECT * FROM widget ORDER BY name ASC';
     Tester::check_equal("ORDER BY ASC", $expected);
 
     ORM::for_table('widget')->order_by_asc('name')->order_by_desc('age')->find_one();
-    $expected = 'SELECT * FROM widget ORDER BY "name" ASC, "age" DESC';
+    $expected = 'SELECT * FROM widget ORDER BY name ASC, age DESC';
     Tester::check_equal("Multiple ORDER BY", $expected);
 
     ORM::for_table('widget')->where('name', 'Fred')->limit(5)->offset(5)->order_by_asc('name')->find_many();
-    $expected = 'SELECT * FROM widget WHERE name = "Fred" LIMIT "5" OFFSET "5" ORDER BY "name" ASC';
+    $expected = 'SELECT * FROM widget WHERE name = "Fred" ORDER BY name ASC LIMIT 5 OFFSET 5';
     Tester::check_equal("Complex query", $expected);
 
     ORM::for_table('widget')->where_raw('name = ? AND (age = ? OR age = ?)', array('Fred', 5, 10))->find_many();