Skip to content

Fix the order of LIMIT and OFFSET clauses when using the Presto flavor (issue #195) #196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 20, 2025

Conversation

barweiss
Copy link
Contributor

@barweiss barweiss commented Mar 20, 2025

Fix for #195:

Queries that have both a limit and an offset build a query with a limit clause before the offset clause.

fmt.Println(Select("*").From("table").Offset(5).Limit(10))
// SELECT * FROM table LIMIT 10 OFFSET 5

This seems to generate syntax errors in Trino. Presto documentation specifies OFFSET before LIMIT in the SELECT statement documentation too, but is not very explicit about the reverse not being supported.

If the OFFSET and LIMIT clauses are swapped, the query succeeds.

Expected:

fmt.Println(Select("*").From("table").Offset(5).Limit(10))
// SELECT * FROM table OFFSET 5 LIMIT 10

…r (issue #195)

Queries that have both a limit and an offset build a query with a limit clause *before* the offset clause.

```go
fmt.Println(Select("*").From("table").Offset(5).Limit(10))
// SELECT * FROM table LIMIT 10 OFFSET 5
```

This seems to generate syntax errors in Trino. [Presto documentation specifies OFFSET before LIMIT](https://prestodb.io/docs/current/sql/select.html#limit-clause) in the SELECT statement documentation too, but is not very explicit about the reverse not being supported.

If the OFFSET and LIMIT clauses are swapped, the query succeeds.

Expected:

```go
fmt.Println(Select("*").From("table").Offset(5).Limit(10))
// SELECT * FROM table OFFSET 5 LIMIT 10
```
@huandu
Copy link
Owner

huandu commented Mar 20, 2025

LGTM. Thanks for your contribution!

@huandu huandu added the bug label Mar 20, 2025
@huandu huandu merged commit 3e72c6a into huandu:master Mar 20, 2025
1 check passed
@coveralls
Copy link

Coverage Status

coverage: 96.452% (+0.01%) from 96.439%
when pulling baa3920 on barweiss:fix-presto-limit-and-offset-order
into 87b9c12 on huandu:master.

@barweiss barweiss deleted the fix-presto-limit-and-offset-order branch March 20, 2025 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants