SQL: Implement TOP as an alternative to LIMIT #57428
Conversation
Add basic support for `TOP X` as a synonym to LIMIT X which is used by [MS-SQL server](https://docs.microsoft.com/en-us/sql/t-sql/queries/top-transact-sql?view=sql-server-ver15), e.g.: ``` SELECT TOP 5 a, b, c FROM test ``` TOP in SQL server also supports the `PERCENTAGE` amd `WITH TIES` keywords which this implementation doesn't. Don't allow usage of both TOP and LIMIT in the same query. Refers to elastic#41195
|
Pinging @elastic/es-ql (:Query Languages/SQL) |
| @@ -243,4 +258,8 @@ public LogicalPlan visitTableName(TableNameContext ctx) { | |||
| TableIdentifier tableIdentifier = visitTableIdentifier(ctx.tableIdentifier()); | |||
| return new UnresolvedRelation(source(ctx), tableIdentifier, alias, ctx.FROZEN() != null); | |||
| } | |||
|
|
|||
| private Limit wrapWithLimit(LogicalPlan plan, Source source, Token limit) { | |||
There was a problem hiding this comment.
I'd prefer calling the method simply limit - it's not really wrapping but instead adding a new node to the tree.
|
|
||
| where | ||
|
|
||
| count:: is a positive integer or zero indicating the maximum *possible* number of results being returned (as there might be less matches |
There was a problem hiding this comment.
I think fewer goes better with matches.
There was a problem hiding this comment.
Was a copy from LIMIT, so fixing it there as well.
| ---- | ||
|
|
||
| [NOTE] | ||
| <<sql-syntax-top, `TOP`>> and <<sql-syntax-limit, `LIMIT`>> cannot be used in combination in the same query and error is returned. |
There was a problem hiding this comment.
...cannot be used together in the same query and an error is returned otherwise sounds better? If so, same sentence was used for LIMIT and the change is needed there as well.
|
|
||
| // TOP | ||
| SqlBaseParser.TopClauseContext topClauseContext = ctx.topClause(); | ||
| if (topClauseContext != null) { |
There was a problem hiding this comment.
Why not combining all three conditions in the same if? More readable this way?
| plan = new Limit(source(limitClause), new Literal(source(limitClause), | ||
| Integer.parseInt(limit.getText()), DataTypes.INTEGER), plan); | ||
| if (plan instanceof Limit) { | ||
| throw new ParsingException(source(limitClause), "Cannot use both TOP and LIMIT in the same query"); |
There was a problem hiding this comment.
I would rephrase the error a bit - TOP and LIMIT are not allowed in the same query - use one or the other.
Add basic support for `TOP X` as a synonym to LIMIT X which is used by [MS-SQL server](https://docs.microsoft.com/en-us/sql/t-sql/queries/top-transact-sql?view=sql-server-ver15), e.g.: ``` SELECT TOP 5 a, b, c FROM test ``` TOP in SQL server also supports the `PERCENTAGE` and `WITH TIES` keywords which this implementation doesn't. Don't allow usage of both TOP and LIMIT in the same query. Refers to #41195 (cherry picked from commit 2f5ab81)
Add basic support for
TOP Xas a synonym to LIMIT X which is usedby MS-SQL server,
e.g.:
TOP in SQL server also supports the
PERCENTAGEamdWITH TIESkeywords which this implementation doesn't.
Don't allow usage of both TOP and LIMIT in the same query.
Refers to #41195