OFFSET should be allowed to be 0 #1937
Conversation
|
Looks good, although it seems odd that |
|
Yeah, I wasn't brave enough to just let it... I could try and see what it breaks :-) |
|
@pauldix your hunch was correct... zero was totally ok! |
| @@ -1435,8 +1435,8 @@ func (p *Parser) parseOptionalTokenAndInt(t Token) (int, error) { | |||
| // Parse number. | |||
| n, _ := strconv.ParseInt(lit, 10, 64) | |||
|
|
|||
| if n < 1 { | |||
| msg := fmt.Sprintf("%s must be > 0", t.String()) | |||
| if n < 0 { | |||
There was a problem hiding this comment.
any reason to validate this as all? Wouldn't it be better to just look at the error from ParseInt? I'm guessing that's why it wasn't allowing zero before.
There was a problem hiding this comment.
So you get an error like this?
> show series limit -1
ERR: error parsing query: LIMIT must be > 0 at line 1, char 19
There was a problem hiding this comment.
That was a deliberate design decision we came to a while back (search Slack).
If my recollection is correct we decided that LIMIT 0 didn't make much sense, since it should be interpreted literally as "give me no records". SQLite, MySQL (and I believe Postgres) all return 0 records if you ask for LIMIT 0. Since there was still arguments about this we simply prohibited LIMIT 0 in code. Obviously OFFSET 0 is a different issue, but OFFSET did not exist at the time of the discussions.
There was a problem hiding this comment.
I was just thinking because it was parseOptionalTokenAndInt, not necessarily LIMIT. But sure, this makes sense. Merging it!
OFFSET should be allowed to be 0
chore(ci): Setup nightly build
Fixes #1924