Add support for OFFSET clause

[fgwang7w]

This PR adds support for the result offset clause in SQL query expressions.

Syntax

<query expression> ::=
  [ <with clause> ] <query expression body>
      [ <order by clause> ] [ <result offset clause> ]

<result offset clause> ::=
  OFFSET <offset row count> { ROW | ROWS }

<offset row count> ::=
  <simple value specification>

The OFFSET node is implemented by transforming it to RowNumber and Filter or FilterProject nodes. An example query plan is as follows:

Resolves #14837

Cherry-pick of trinodb/trino#723, trinodb/trino#732, trinodb/trino#746, trinodb/trino#820

Co-authored-by: kasiafi 30203062+kasiafi@users.noreply.github.com

Test plan - Add UT coverage, and test the feature locally

== RELEASE NOTES ==

General Changes
* Add support for the result offset clause in SQL query expressions. This feature can be enabled by setting the session property ``offset_clause_enabled`` or configuration property ``offset-clause-enabled`` to true.

[rongrong]

Can we make sure we have a "kill switch" for this feature? A configuration to disable it if things go wrong in production.

[fgwang7w]

Can we make sure we have a "kill switch" for this feature? A configuration to disable it if things go wrong in production.

A session-level control switch enable-offset is added

[fgwang7w]

@rongrong @yingsu00 @tdcmeehan please help review the PR, thanks

[yingsu00]

This PR needs a release note. Please refer to https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines to add one in the PR description section. The release bot will automatically gather it from all merged PRs.

[yingsu00]

Let's use the same commit names as the Trino ones so that readers will be easy to reason about the history

[yingsu00]

@fgwang7w Hi George, thanks for back porting the PR. I am running out of time and only finished the first part. I'll flush the comments I have so far and finish the rest tomorrow.

[yingsu00]

isEnableOffset -> isOffsetClauseEnabled

[yingsu00]

@fgwang7w I don't see this function name changed yet.

[yingsu00]

Where is the unit test TestImplementOffset.java? It should be added in package com.facebook.presto.sql.planner.iterative.rule

[yingsu00]

@fgwang7w Would it be possible to add a few plan examples for OFFSET clause in the PR message?

[fgwang7w]

@fgwang7w Would it be possible to add a few plan examples for OFFSET clause in the PR message?

Thank you for reviewing the PR. Please help review the revision. TestImplementOffset is added back, query plan is updated in the PR message session.

[yingsu00]

Commit message 1

I'd prefer changing the message Add support for OFFSET syntax to its original message Add support for OFFSET syntax in grammar and AST

Cherry-pick of trino/#723 This doesn't resolve to the correct http link but to a different commit. Let's use full URL:
Cherry-pick of https://github.com/trinodb/trino/pull/723

[yingsu00]

I strongly believe trinodb/trino#746 should belong to the first commit(parser changes) not the second(planner changes). Would you please also update the commit messages so that the first commit contains 723 and 746 and the second commit contains the rest? Remember to use the full URL so that they resolve to the correct commits. Then add another commit for the documentation. Thanks!

[fgwang7w]

@yingsu00 Thanks for suggestions and review. Please help review the revisions again.

[yingsu00]

@fgwang7w Hi George, it looks in good shape. Just two small catches:

1. The function name isEnableOffset in SystemSessionProperties was still not updated
2. The commit message for the second commit still has 746 included. Please remove 746 from the commit message.

[yingsu00]

@fgwang7w I don't see this function name changed yet.

[yingsu00]

@rongrong I think this PR is almost good to go (waiting for two small issues being fixed). Do you want to take a second pass? Thanks!

[yingsu00]

@fgwang7w Just discussed with Rongrong. Will you be able to set the session and config properties default to false, and move the 3rd commit to the first?

[fgwang7w]

@yingsu00 @rongrong thanks for the feedback. I have changed the default to disable. So the commits are in following order:

1. enable parser yet fails at analyzer
2. implement offset
3. disable offset
4. update doc.

Let me know if you have any more feedback. Thanks

[NikhilCollooru]

@rongrong looks like all the comments are addressed. Can you take a look and merge it .

[rongrong]

Thanks for your contribution!