Thread: Negative LIMIT and OFFSET?
I'm fairly surprised these queries work. Is there some reason why we support this? April Fools Day? Jules Verne? I'm all for fast queries, but zero seems like the lowest value we should support... postgres=# select * from accounts limit -9;aid | bid | abalance | filler -----+-----+----------+-------- (0 rows) Time: 0.330 ms postgres=# select * from accounts limit -9 offset 45;aid | bid | abalance | filler -----+-----+----------+-------- (0 rows) Time: 0.268 ms postgres=# select * from accounts limit -9 offset -100000;aid | bid | abalance | filler -----+-----+----------+-------- (0 rows) Time: 0.287 ms postgres=# select * from accounts limit 0 offset -100000;aid | bid | abalance | filler -----+-----+----------+-------- (0 rows) Time: 0.289 ms -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com
"Simon Riggs" <simon@2ndquadrant.com> writes: > I'm fairly surprised these queries work. Is there some reason why we > support this? April Fools Day? Jules Verne? I'm all for fast queries, > but zero seems like the lowest value we should support... Huh, I was all set to post an example of a useful application of it but then apparently I'm wrong and it doesn't work: postgres=# select * from generate_series(1,10) offset -1 limit 2;generate_series ----------------- 1 2 (2 rows) I'll leave it as an exercise for the reader to guess what I was expecting. So given that that doesn't work I don't see any particular reason to accept negative offsets or limits in 8.4 and on. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning
On Fri, Dec 14, 2007 at 01:47:23AM +0000, Gregory Stark wrote: > Huh, I was all set to post an example of a useful application of it but then > apparently I'm wrong and it doesn't work: I dimly remember some discussion of this issue once before, maybe a year ago. My memory isn't what it was, and I can't find it by trolling archives, but I recall Tom saying that it was dumb, yes, but don't do that, because there's some reason not to change it. I know, helpful search terms R me. A
On Dec 13, 2007 9:43 PM, Andrew Sullivan <ajs@crankycanuck.ca> wrote: > I dimly remember some discussion of this issue once before, maybe a year > ago. My memory isn't what it was, and I can't find it by trolling archives, > but I recall Tom saying that it was dumb, yes, but don't do that, because > there's some reason not to change it. I know, helpful search terms R me. Man, maybe my mad Google skillz are not as mad as I thought :( The best I could come up with was on my first try, though as it's just a reply to a user asking for a different behavior of it, I doubt this is it: http://archives.postgresql.org/pgsql-general/2002-10/msg01293.php Google search terms: site:archives.postgresql.org "tom lane" "limit -1" "negative" While I haven't looked at the code myself, I tend to agree with Simon and Greg... I know of no reason to allow a negative limit/offset. -- Jonah H. Harris, Sr. Software Architect | phone: 732.331.1324 EnterpriseDB Corporation | fax: 732.331.1301 499 Thornall Street, 2nd Floor | jonah.harris@enterprisedb.com Edison, NJ 08837 | http://www.enterprisedb.com/
Andrew Sullivan <ajs@crankycanuck.ca> writes:
> On Fri, Dec 14, 2007 at 01:47:23AM +0000, Gregory Stark wrote:
>> Huh, I was all set to post an example of a useful application of it but then
>> apparently I'm wrong and it doesn't work:
> I dimly remember some discussion of this issue once before, maybe a year
> ago. My memory isn't what it was, and I can't find it by trolling archives,
> but I recall Tom saying that it was dumb, yes, but don't do that, because
> there's some reason not to change it. I know, helpful search terms R me.
Hmm ... I don't recall much either. The code in nodeLimit.c just
silently replaces a negative input value by zero. It'd certainly be
possible to make it throw an error instead, but what the downsides of
that might be aren't clear.
I guess that on purely philosophical grounds, it's not an unreasonable
behavior. For example, "LIMIT n" means "output at most n tuples",
not "output exactly n tuples". So when it outputs no tuples in the face
of a negative limit, it's meeting its spec. If you want to throw an
error for negative limit, shouldn't you logically also throw an error
for limit larger than the actual number of rows produced by the subplan?
regards, tom lane
On Thu, Dec 13, 2007 at 10:01:43PM -0500, Jonah H. Harris wrote: > Man, maybe my mad Google skillz are not as mad as I thought :( Hey, I worked in a library some years ago, when Google was just a googlet, and I couldn't find it either. It's a dim memory, note. Which could mean "artifact". I'm old! I'm probably delusional too. A
On Dec 13, 2007 10:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I guess that on purely philosophical grounds, it's not an unreasonable > behavior. For example, "LIMIT n" means "output at most n tuples", > not "output exactly n tuples". So when it outputs no tuples in the face > of a negative limit, it's meeting its spec. If you want to throw an > error for negative limit, shouldn't you logically also throw an error > for limit larger than the actual number of rows produced by the subplan? Hmm, good point. It does seem like if you're going to be pedantic, you should be pedantic on both counts. Though, I could understand throwing an error on a negative, because that's likely a bug in the user's code and would enable them to find out what's wrong. On the limit-larger-than-tuples-returned case, I don't think it should throw an error because it's generally considered as, "at most this many". I don't see a case where any user would think that a negative limit *should* be allowed. Don't we have any similar usability cases in the system like this, where negatives are not allowed only for the sake of it being an insane setting? I'm tired, but I thought we did. -- Jonah H. Harris, Sr. Software Architect | phone: 732.331.1324 EnterpriseDB Corporation | fax: 732.331.1301 499 Thornall Street, 2nd Floor | jonah.harris@enterprisedb.com Edison, NJ 08837 | http://www.enterprisedb.com/
On Thu, Dec 13, 2007 at 10:06:35PM -0500, Tom Lane wrote: > of a negative limit, it's meeting its spec. If you want to throw an > error for negative limit, shouldn't you logically also throw an error Should it be a WARNING? A
"Jonah H. Harris" <jonah.harris@gmail.com> writes:
> Don't we have any similar usability cases in the system like this,
> where negatives are not allowed only for the sake of it being an
> insane setting? I'm tired, but I thought we did.
Yeah, probably. It's the kind of thing where the call is close enough
that it might be made differently by different people.
After thinking about it for a bit, the only downside I can think of is
that throwing an error might create an unexpected corner case for code
that computes a LIMIT value on-the-fly and might sometimes come up
with a slightly negative value. But you could always doLIMIT greatest(whatever, 0)
so that seems like a weak argument.
regards, tom lane
On Dec 13, 2007 10:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andrew Sullivan <ajs@crankycanuck.ca> writes: > > On Fri, Dec 14, 2007 at 01:47:23AM +0000, Gregory Stark wrote: > >> Huh, I was all set to post an example of a useful application of it but then > >> apparently I'm wrong and it doesn't work: > > > I dimly remember some discussion of this issue once before, maybe a year > > ago. My memory isn't what it was, and I can't find it by trolling archives, > > but I recall Tom saying that it was dumb, yes, but don't do that, because > > there's some reason not to change it. I know, helpful search terms R me. > > Hmm ... I don't recall much either. The code in nodeLimit.c just > silently replaces a negative input value by zero. It'd certainly be > possible to make it throw an error instead, but what the downsides of > that might be aren't clear. > > I guess that on purely philosophical grounds, it's not an unreasonable > behavior. For example, "LIMIT n" means "output at most n tuples", > not "output exactly n tuples". So when it outputs no tuples in the face > of a negative limit, it's meeting its spec. If you want to throw an > error for negative limit, shouldn't you logically also throw an error > for limit larger than the actual number of rows produced by the subplan? for historical record, this comment (subject not directly related to the OP) was probably this: http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg62562.html at least if it happened since 10/2004, which is when i started tracking -hackers in my gmail account (an amazing search tool, btw). merlin
On Thu, 2007-12-13 at 22:06 -0500, Tom Lane wrote: > I guess that on purely philosophical grounds, it's not an unreasonable > behavior. For example, "LIMIT n" means "output at most n tuples", > not "output exactly n tuples". So when it outputs no tuples in the face > of a negative limit, it's meeting its spec. If "LIMIT n" means "emit at most n tuples", then a query that produces 0 rows with n < 0 is arguably violating its spec, since it has produced more tuples than the LIMIT specified (0 > n). Interpreted this way, no result set can be consistent with a negative limit, so I'd vote for throwing an error. -Neil
"Tom Lane" <tgl@sss.pgh.pa.us> writes: > "Jonah H. Harris" <jonah.harris@gmail.com> writes: >> Don't we have any similar usability cases in the system like this, >> where negatives are not allowed only for the sake of it being an >> insane setting? I'm tired, but I thought we did. > > Yeah, probably. It's the kind of thing where the call is close enough > that it might be made differently by different people. > > After thinking about it for a bit, the only downside I can think of is > that throwing an error might create an unexpected corner case for code > that computes a LIMIT value on-the-fly and might sometimes come up > with a slightly negative value. See, that's what I was thinking when I wrote the OFFSET -1 LIMIT 2 test. If that produced 1 record then I would say it avoids a corner case for user code which would have to special case windows which hit the beginning or end of the data set. But given that it doesn't work that way anyways user code already has a big corner case. The argument for errors that I see is: Having a useless non-error behaviour locks us into keeping that. If we one day think of a useful set of semantics we can't just silently change the meaning on users without a transition period of generating errors. For example we might want to implement negative offsets like above, or if we want to implement negative limits as meaning some number of rows *before* the offset or end of relation, or something like that... The flip side of all this is that it's hard to get too excited about it. It's just OFFSET / LIMIT. If it was a big deal we would have noticed years ago anyways. It might not be worth the effort to change and introduce behaviour changes for users at all. Oh, and incidentally the problem with WARNING is that this is DML which could potentially be executing hundreds or thousands of times per minute. A WARNING is effectively an ERROR. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support!
On Thu, 2007-12-13 at 22:06 -0500, Tom Lane wrote: > Hmm ... I don't recall much either. The code in nodeLimit.c just > silently replaces a negative input value by zero. It'd certainly be > possible to make it throw an error instead, but what the downsides of > that might be aren't clear. > > I guess that on purely philosophical grounds, it's not an unreasonable > behavior. For example, "LIMIT n" means "output at most n tuples", > not "output exactly n tuples". So when it outputs no tuples in the face > of a negative limit, it's meeting its spec. Not bothered either way, really, just reporting weirdness as it comes. A calculated LIMIT value that was negative probably indicates an error in the calculation that the SQL programmer may wish to know about. I had previously assumed that we would report such errors. > If you want to throw an > error for negative limit, shouldn't you logically also throw an error > for limit larger than the actual number of rows produced by the subplan? No, thats another feature AT LEAST n, which could also be a useful thing, like an Assert. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com
On Thu, 2007-12-13 at 22:23 -0800, Neil Conway wrote: > On Thu, 2007-12-13 at 22:06 -0500, Tom Lane wrote: > > I guess that on purely philosophical grounds, it's not an unreasonable > > behavior. For example, "LIMIT n" means "output at most n tuples", > > not "output exactly n tuples". So when it outputs no tuples in the face > > of a negative limit, it's meeting its spec. > > If "LIMIT n" means "emit at most n tuples", then a query that produces 0 > rows with n < 0 is arguably violating its spec, since it has produced > more tuples than the LIMIT specified (0 > n). Interpreted this way, no > result set can be consistent with a negative limit, so I'd vote for > throwing an error. I even found an existing, unused error message called ERRCODE_INVALID_LIMIT_VALUE so here's a patch. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com
Attachment
On Fri, 2007-12-14 at 14:41 +0000, Simon Riggs wrote: > On Thu, 2007-12-13 at 22:23 -0800, Neil Conway wrote: > so here's a patch. minor correction -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com
Attachment
On Fri, Dec 14, 2007 at 09:02:04AM +0000, Gregory Stark wrote: > > Oh, and incidentally the problem with WARNING is that this is DML which could > potentially be executing hundreds or thousands of times per minute. A WARNING > is effectively an ERROR. Good point. Also, the sort of case where you're likely to be automatically generating these negative values is also the sort of case where you have various nice programmatic interfaces, many of which store up all the warnings. The warnings then have to be freed explicitly, which of course means that by adding a warning, clients would suddenly start to chew through piles of memory. A
On Thu, Dec 13, 2007 at 11:31:17PM -0500, Merlin Moncure wrote: > > for historical record, this comment (subject not directly related to > the OP) was probably this: > http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg62562.html Bingo. Thanks! A
Simon Riggs <simon@2ndquadrant.com> writes:
> I even found an existing, unused error message called
> ERRCODE_INVALID_LIMIT_VALUE
That's a bad idea I think. That code is defined by SQL99. I can't find
anyplace that they specify what it should be raised for, but we can be
pretty confident that it's not meant for LIMIT. I think we should
just use INVALID_PARAMETER_VALUE.
How do people feel about applying this to 8.3, rather than holding it?
One possible objection is that we're past string freeze, but I noted
Peter doing some message editorializing as recently as today, so it
would seem a slushy freeze at best.
regards, tom lane
On Dec 14, 2007 6:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > How do people feel about applying this to 8.3, rather than holding it? > One possible objection is that we're past string freeze, but I noted > Peter doing some message editorializing as recently as today, so it > would seem a slushy freeze at best. FWIW, I'm good with applying it to 8.3. -- Jonah H. Harris, Sr. Software Architect | phone: 732.331.1324 EnterpriseDB Corporation | fax: 732.331.1301 499 Thornall Street, 2nd Floor | jonah.harris@enterprisedb.com Edison, NJ 08837 | http://www.enterprisedb.com/
"Jonah H. Harris" <jonah.harris@gmail.com> writes: > On Dec 14, 2007 6:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> How do people feel about applying this to 8.3, rather than holding it? >> One possible objection is that we're past string freeze, but I noted >> Peter doing some message editorializing as recently as today, so it >> would seem a slushy freeze at best. > > FWIW, I'm good with applying it to 8.3. I think it would have been better to apply before beta. We would have found out if users were going to complain about it. Perhaps we should do it for 8.4 instead -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB'sPostgreSQL training!
Gregory Stark <stark@enterprisedb.com> writes:
>> On Dec 14, 2007 6:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> How do people feel about applying this to 8.3, rather than holding it?
> I think it would have been better to apply before beta. We would have found
> out if users were going to complain about it. Perhaps we should do it for 8.4
> instead
Um ... what's your point? Are you suggesting we might've backed it out
if a bunch of people complained? Perhaps, but given that no one's even
noticed the detail before, it seems pretty unlikely there would have
been any complaints during beta.
regards, tom lane
On Fri, 2007-12-14 at 18:42 -0500, Tom Lane wrote: > Simon Riggs <simon@2ndquadrant.com> writes: > How do people feel about applying this to 8.3, rather than holding it? > One possible objection is that we're past string freeze, but I noted > Peter doing some message editorializing as recently as today, so it > would seem a slushy freeze at best. No opinion either way on this one. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com
"Tom Lane" <tgl@sss.pgh.pa.us> writes: > Gregory Stark <stark@enterprisedb.com> writes: >>> On Dec 14, 2007 6:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>>> How do people feel about applying this to 8.3, rather than holding it? > >> I think it would have been better to apply before beta. We would have found >> out if users were going to complain about it. Perhaps we should do it for 8.4 >> instead > > Um ... what's your point? Are you suggesting we might've backed it out > if a bunch of people complained? Perhaps, but given that no one's even > noticed the detail before, it seems pretty unlikely there would have > been any complaints during beta. I suppose that's what I'm saying. We've certainly been surprised before by user reaction. But if you think we wouldn't back it out in that case that kind of takes the wind out of that concern. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support!
On Fri, Dec 14, 2007 at 06:42:24PM -0500, Tom Lane wrote: > > How do people feel about applying this to 8.3, rather than holding it? To me, this is a feature change, and therefore should be held. A
Andrew Sullivan <ajs@crankycanuck.ca> writes:
> On Fri, Dec 14, 2007 at 06:42:24PM -0500, Tom Lane wrote:
>> How do people feel about applying this to 8.3, rather than holding it?
> To me, this is a feature change, and therefore should be held.
Well, I wouldn't advocate making it in a minor release, but it's not
clear how that translates into saying it can't go into 8.3.
regards, tom lane
Gregory Stark wrote: > > "Simon Riggs" <simon@2ndquadrant.com> writes: > > > I'm fairly surprised these queries work. Is there some reason why we > > support this? April Fools Day? Jules Verne? I'm all for fast queries, > > but zero seems like the lowest value we should support... > > Huh, I was all set to post an example of a useful application of it but then > apparently I'm wrong and it doesn't work: > > postgres=# select * from generate_series(1,10) offset -1 limit 2; > generate_series > ----------------- > 1 > 2 > (2 rows) > > I'll leave it as an exercise for the reader to guess what I was expecting. > > So given that that doesn't work I don't see any particular reason to accept > negative offsets or limits in 8.4 and on. Since we got LIMIT/OFFSET from MySQL, would someone tell us how MySQL behaves in these cases? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Since we got LIMIT/OFFSET from MySQL, would someone tell us how MySQL
> behaves in these cases?
Not very well, at least not in mysql 5.0.45:
mysql> select * from t limit -2;
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server
versionfor the right syntax to use near '-2' at line 1
mysql> select * from t limit 2 offset -2;
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server
versionfor the right syntax to use near '-2' at line 1
This behavior suggests that they can't even deal with LIMIT/OFFSET
values that aren't simple integer literals ...
regards, tom lane
"Tom Lane" <tgl@sss.pgh.pa.us> writes: > mysql> select * from t limit 2 offset -2; > ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server versionfor the right syntax to use near '-2' at line 1 > > This behavior suggests that they can't even deal with LIMIT/OFFSET > values that aren't simple integer literals ... I suppose when they added these features I think they didn't have subqueries, so there wasn't really much useful that could be done with arbitrary expressions here. Being able to do "LIMIT 1+1" doesn't actually add anything. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support!
Gregory Stark <stark@enterprisedb.com> writes:
> "Tom Lane" <tgl@sss.pgh.pa.us> writes:
>> This behavior suggests that they can't even deal with LIMIT/OFFSET
>> values that aren't simple integer literals ...
> I suppose when they added these features I think they didn't have subqueries,
> so there wasn't really much useful that could be done with arbitrary
> expressions here. Being able to do "LIMIT 1+1" doesn't actually add anything.
Sure. I think our first implementation of LIMIT was similarly
constrained. It's just amusing that they haven't moved past that,
despite having had the feature first ...
regards, tom lane
On Sun, Dec 16, 2007 at 12:31:11PM -0500, Tom Lane wrote: > > Well, I wouldn't advocate making it in a minor release, but it's not > clear how that translates into saying it can't go into 8.3. Just because we're well past feature freeze, in beta. I realise this seems like a corner case, but the whole point of having betas where functionality is more or less frozen is to reduce the liklihood that someone's testing on (say) beta 2 is not all completely invalidated on beta 4. A
This has been saved for the 8.4 release: http://momjian.postgresql.org/cgi-bin/pgpatches_hold --------------------------------------------------------------------------- Andrew Sullivan wrote: > On Sun, Dec 16, 2007 at 12:31:11PM -0500, Tom Lane wrote: > > > > Well, I wouldn't advocate making it in a minor release, but it's not > > clear how that translates into saying it can't go into 8.3. > > Just because we're well past feature freeze, in beta. I realise this seems > like a corner case, but the whole point of having betas where functionality > is more or less frozen is to reduce the liklihood that someone's testing on > (say) beta 2 is not all completely invalidated on beta 4. > > A > > ---------------------------(end of broadcast)--------------------------- > TIP 7: You can help support the PostgreSQL project by donating at > > http://www.postgresql.org/about/donate -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Simon Riggs <simon@2ndquadrant.com> writes:
> On Thu, 2007-12-13 at 22:23 -0800, Neil Conway wrote:
>> If "LIMIT n" means "emit at most n tuples", then a query that produces 0
>> rows with n < 0 is arguably violating its spec, since it has produced
>> more tuples than the LIMIT specified (0 > n). Interpreted this way, no
>> result set can be consistent with a negative limit, so I'd vote for
>> throwing an error.
> I even found an existing, unused error message called
> ERRCODE_INVALID_LIMIT_VALUE
> so here's a patch.
Applied, but using just ERRCODE_INVALID_PARAMETER_VALUE rather than
guessing what the SQL committee intended with that SQLSTATE.
regards, tom lane