Skip to content
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

Add LIMIT, OFFSET and TIES post #70

Merged
merged 1 commit into from
Feb 3, 2020
Merged

Conversation

findepi
Copy link
Member

@findepi findepi commented Feb 2, 2020

No description provided.

@cla-bot cla-bot bot added the cla-signed label Feb 2, 2020
_posts/2020-02-02-fetch-offset-ties.md Outdated Show resolved Hide resolved
_posts/2020-02-02-fetch-offset-ties.md Outdated Show resolved Hide resolved
@findepi
Copy link
Member Author

findepi commented Feb 3, 2020

AC


`LIMIT` / `FETCH FIRST ... ROWS ONLY`, `FETCH FIRST ... WITH TIES` and `OFFSET` are powerful and very useful clauses
that come especially handy when writing ad-hoc queries over big data sets. Since semantics of these clauses depend on query
results being well ordered, they are best used with `ORDER BY` that defined proper ordering. Without proper ordering
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defined -> defines

If we look again at the database systems mentioned above, it turns out many of them support the standard
syntax too: Oracle, DB2, SQL Server and PostgreSQL (although that's not documented currently).

And Presto? Presto has `LIMIT n` support since 2012. In [Presto 310](https://prestosql.io/docs/current/release/release-310.html),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

310 instead of Presto 310?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO "310" is not sufficient for describing link target.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This renders as:
And Presto? Presto has LIMIT n support since 2012. In Presto 310, Presto gained also the FETCH FIRST n ROWS ONLY support.
Too much "Presto".
Maybe "Since 310, Presto gained..."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. "In Presto 310, we added ..."

```

The `FETCH FIRST n ROWS WITH TIES` clause retains all rows with equal values of the ordering keys (the `ORDER BY` clause) as
the last row that would be returned by the `FETCH FIRST n ROWS ONLY` clause (or its equivalent, `LIMIT n`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skip "or its equivalent..."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed


Per the SQL Standard, the `FETCH FIRST n ROWS ONLY` clause can be prepended with `OFFSET m`, to skip `m` initial rows.
In such a case, it makes sense to use `FETCH NEXT ...` variant of the clause -- it's allowed with and without `OFFSET`,
but definitely looks better with that clause.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skip " -- it's allowed...". It makes the piece harder to read.
There are more variants of the clause not mentioned in this post.
Optionally, you could describe all of them in one place in grammar-like style:
FETCH (FIRST | NEXT) n? (ROW | ROWS) (ONLY | WITH TIES)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will keep this, but will also add ref to docs which describe full grammar

@findepi findepi merged commit b8be315 into trinodb:master Feb 3, 2020
@findepi findepi deleted the fetch-offset branch February 3, 2020 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants