Missing LIMIT if none specified, but OFFSET given / Patch attached

[vifo]

Hey j4mie,

firstable, thanks for this great, simple ORM!

I just encountered a slight bug, when using $sth->offset(), but not specifiying a row limit count with $sth->limit(). The required LIMIT is not emitted in the final SQL (but required for the statement to work).

Following patch fixes the issue:

diff --git a/idiorm.php b/idiorm.php
index 4d454cb..87c8518 100644
--- a/idiorm.php
+++ b/idiorm.php
@@ -856,6 +856,9 @@
             if (!is_null($this->_limit)) {
                 return "LIMIT " . $this->_limit;
             }
+            if (!is_null($this->_offset)) {
+                return "LIMIT 18446744073709551615";
+            }
             return '';
         }

-- 
1.7.3.1

Thanks and regards,

ViFo.

[j4mie]

Hi,

Thanks for the patch - I need to do a bit of research into how portable this is, as hardcoding a value like that seems a bit odd...

Jamie

[vifo]

Hi j4mie,

yes, it also looked a bit odd to me, but since there's no way in standard SQL to ommit LIMIT, when also OFFSET is given, specifiying a LIMIT of 2^64 -1 == 18446744073709551615 (e.g. max value for an unsigned 64-bit integer, ULLONG_MAX from limits.h) seems to be the widely accepted solution (at least in MySQL - it is also shown in MySQL examples).

Postgres, however supports a LIMIT ALL.

Edit: Just checked the PHP docs; there does not seem to be a define for this value. Workaround:

define(ULLONG_MAX, '18446744073709551615');

[j4mie]

It's the portability that worries me. I can't add MySQL-only code to Idiorm, and adding "special cases" for every database is something I'm trying to avoid wherever possible

I'm actually struggling to see exactly why this is needed. Can't you just ensure that limit() is always called when offset() is called in your code? It feels like magically adding a limit to a query would be non-intuitive and surprising to a user.

[vifo]

Hi j4mie,

actually, I agree with your concerns about portability. I can of course ensure, that I'll use limit() always, whenever I also use offset().

Apart from this LIMIT/OFFSET is supported by Postgres, MySQL and SQLite only (as far as I can tell - and LIMIT may be omitted only in Postgres). MSSQL / Oracle do not support it (yet) at all, so paging and limiting must be done there with other statements anyway. For the three mentioned, the above solution is fairly portable and commonly used (and you wouldn't have to distinguish between the underlying RDBMs).

Greetings
ViFo

[treffynnon]

This should be controlled by the application and not at the ORM level in my opinion and I don't like having a hard coded value in the ORM like this.