Skip to content

Commit

Permalink
Disable unsupported PPL function expressions (#478)
Browse files Browse the repository at this point in the history
Signed-off-by: Tomoyuki Morita <moritato@amazon.com>
  • Loading branch information
ykmr1224 authored Jul 26, 2024
1 parent 3c8a490 commit 98bd79a
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 32 deletions.
2 changes: 1 addition & 1 deletion ppl-spark-integration/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ The next samples of PPL queries are currently supported:
- `where` - [See details](https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/cmd/where.rst)
- `fields` - [See details](https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/cmd/fields.rst)
- `head` - [See details](https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/cmd/head.rst)
- `stats` - [See details](https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/cmd/stats.rst) (supports AVG, COUNT, MAX, MIN and SUM aggregation functions)
- `stats` - [See details](https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/cmd/stats.rst) (supports AVG, COUNT, DISTINCT_COUNT, MAX, MIN and SUM aggregation functions)
- `sort` - [See details](https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/cmd/sort.rst)
- `correlation` - [See details](../docs/PPL-Correlation-command.md)

Expand Down
9 changes: 1 addition & 8 deletions ppl-spark-integration/src/main/antlr4/OpenSearchPPLParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ commands
: whereCommand
| correlateCommand
| fieldsCommand
| renameCommand
| statsCommand
| sortCommand
| headCommand
Expand Down Expand Up @@ -224,8 +223,6 @@ statsFunction
: statsFunctionName LT_PRTHS valueExpression RT_PRTHS # statsFunctionCall
| COUNT LT_PRTHS RT_PRTHS # countAllFunctionCall
| (DISTINCT_COUNT | DC) LT_PRTHS valueExpression RT_PRTHS # distinctCountFunctionCall
| percentileAggFunction # percentileAggFunctionCall
| takeAggFunction # takeAggFunctionCall
;

statsFunctionName
Expand Down Expand Up @@ -257,18 +254,14 @@ logicalExpression
| left = logicalExpression OR right = logicalExpression # logicalOr
| left = logicalExpression (AND)? right = logicalExpression # logicalAnd
| left = logicalExpression XOR right = logicalExpression # logicalXor
| booleanExpression # booleanExpr
| relevanceExpression # relevanceExpr
;

comparisonExpression
: left = valueExpression comparisonOperator right = valueExpression # compareExpr
;

valueExpression
: left = valueExpression binaryOperator = (STAR | DIVIDE | MODULE) right = valueExpression # binaryArithmetic
| left = valueExpression binaryOperator = (PLUS | MINUS) right = valueExpression # binaryArithmetic

This comment has been minimized.

Copy link
@LantaoJin

LantaoJin Jul 26, 2024

Member

why this binary operator be removed. the ppl source=index | where a=1+1 will fail. @ykmr1224

This comment has been minimized.

Copy link
@ykmr1224

ykmr1224 Jul 29, 2024

Author Collaborator

Those math expressions are converted to Function class, and Function raised "Not Supported operation" error. You can enable it again in your PR once that is supported.

| primaryExpression # valueExpressionDefault
: primaryExpression # valueExpressionDefault
| LT_PRTHS valueExpression RT_PRTHS # parentheticValueExpr
;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,6 @@ public UnresolvedExpression visitCompareExpr(OpenSearchPPLParser.CompareExprCont
return new Compare(ctx.comparisonOperator().getText(), visit(ctx.left), visit(ctx.right));
}

/**
* Value Expression.
*/
@Override
public UnresolvedExpression visitBinaryArithmetic(OpenSearchPPLParser.BinaryArithmeticContext ctx) {
return new Function(
ctx.binaryOperator.getText(), Arrays.asList(visit(ctx.left), visit(ctx.right)));
}

@Override
public UnresolvedExpression visitParentheticValueExpr(OpenSearchPPLParser.ParentheticValueExprContext ctx) {
return visit(ctx.valueExpression()); // Discard parenthesis around
Expand Down Expand Up @@ -172,20 +163,6 @@ public UnresolvedExpression visitPercentileAggFunction(OpenSearchPPLParser.Perce
Collections.singletonList(new Argument("rank", (Literal) visit(ctx.value))));
}

@Override
public UnresolvedExpression visitTakeAggFunctionCall(
OpenSearchPPLParser.TakeAggFunctionCallContext ctx) {
ImmutableList.Builder<UnresolvedExpression> builder = ImmutableList.builder();
builder.add(
new UnresolvedArgument(
"size",
ctx.takeAggFunction().size != null
? visit(ctx.takeAggFunction().size)
: new Literal(DEFAULT_TAKE_FUNCTION_SIZE_VALUE, DataType.INTEGER)));
return new AggregateFunction(
"take", visit(ctx.takeAggFunction().fieldExpression()), builder.build());
}

/**
* Eval function.
*/
Expand Down

0 comments on commit 98bd79a

Please sign in to comment.