Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Commit

Permalink
Syntax and semantic exceptions handling for unsupported features (#345)
Browse files Browse the repository at this point in the history
* Added a filter in semantic layer for unsupported features: nested functions, aggregations with function aggregators, functions with aggregation arguments

* Excluded the special esfunctions that support nested

* Added unit tests

* Added integ tests

* Added COUNT in function-nested aggregates; added error msg assertions to the tests; corrected explanation for tests to ignore
  • Loading branch information
chloe-zh authored Jan 16, 2020
1 parent 179429a commit 77efb0b
Show file tree
Hide file tree
Showing 7 changed files with 305 additions and 20 deletions.
68 changes: 63 additions & 5 deletions src/main/antlr/OpenDistroSqlParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,21 @@ expressions
: expression (',' expression)*
;

aggregateFunction
: functionAsAggregatorFunction #functionAsAggregatorFunctionCall
| aggregateWindowedFunction #aggregateWindowedFunctionCall
;

// Functions
scalarFunction
: scalarFunctionName '(' nestedFunctionArgs+ ')' #nestedFunctionCall
| scalarFunctionName '(' functionArgs? ')' #scalarFunctionCall
;

functionCall
: specificFunction #specificFunctionCall
| aggregateWindowedFunction #aggregateFunctionCall
| scalarFunctionName '(' functionArgs? ')' #scalarFunctionCall
: aggregateFunction #aggregateFunctionCall
| scalarFunctionName '(' aggregateWindowedFunction ')' #aggregationAsArgFunctionCall
| scalarFunction #scalarFunctionsCall
| specificFunction #specificFunctionCall
| fullId '(' functionArgs? ')' #udfFunctionCall
;

Expand Down Expand Up @@ -324,10 +332,30 @@ aggregateWindowedFunction
| COUNT '(' aggregator=DISTINCT functionArgs ')'
;

functionAsAggregatorFunction
: (AVG | MAX | MIN | SUM)
'(' aggregator=(ALL | DISTINCT)? functionCall ')'
| COUNT '(' aggregator=(ALL | DISTINCT)? functionCall ')'
;

scalarFunctionName
: functionNameBase
;

/*
Separated aggregate to function-aggregator and nonfunction-aggregator aggregations.
Current related rules: aggregateWindowedFunction, functionAsAggregatorFunction, aggregateFunction, functionCall
Original rules: functionCall (as is shown in below changes), no aggregateWindowFunction, no functionAsAggregatorFunction,
no aggregateFunction
====
Separated function argument rule to nonFunctionCall and functionCall
functions with functionCall arguments are taken as nested functions
Current related rules: functionArgs, functionArg, nestedFunctionArgs
Original rules:
functionArgs
: (constant | fullColumnName | functionCall | expression)
(
Expand All @@ -340,6 +368,36 @@ functionArg
: constant | fullColumnName | functionCall | expression
;
====
Accordingly functionCall rule is changed by separating scalar functions
to nested functions and non-nested functons.
Current related rules: functionCall, scalarFunction
Original rule:
functionCall
: specificFunction #specificFunctionCall
| aggregateWindowedFunction #aggregateFunctionCall
| scalarFunctionName '(' functionArgs? ')' #scalarFunctionCall
| fullId '(' functionArgs? ')' #udfFunctionCall
;
*/

functionArgs
: (constant | fullColumnName | expression)
(
','
(constant | fullColumnName | expression)
)*
;

functionArg
: constant | fullColumnName | expression
;

nestedFunctionArgs
: functionCall (',' functionArgs)?
;


// Expressions, predicates

Expand Down Expand Up @@ -414,7 +472,7 @@ functionNameBase
| LOG10 | LOG2 | LOWER | LTRIM | MAKETIME | MODULUS | MONTH | MONTHNAME | MULTIPLY
| NOW | PI | POW | POWER | RADIANS | RAND | REPLACE | RIGHT | RINT | ROUND | RTRIM
| SIGN | SIGNUM | SIN | SINH | SQRT | SUBSTRING | SUBTRACT | TAN | TIMESTAMP | TRIM
| UPPER | YEAR
| UPPER | YEAR | ADDDATE | ADDTIME | GREATEST | LEAST
;

esFunctionNameBase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.AggregationAsArgFunctionCallContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.AggregateWindowedFunctionContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.AtomTableItemContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.BinaryComparisonPredicateContext;
Expand All @@ -43,9 +43,11 @@
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.FunctionNameBaseContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.InPredicateContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.IsExpressionContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.MathOperatorContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.MinusSelectContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.OuterJoinContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.PredicateContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.RegexpPredicateContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.RootContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.ScalarFunctionCallContext;
import static com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.SelectElementsContext;
Expand Down Expand Up @@ -217,13 +219,25 @@ public T visitUdfFunctionCall(UdfFunctionCallContext ctx) {
return reduce(func, ctx.functionArgs());
}

// This check should be able to accomplish in grammar
@Override
public T visitScalarFunctionCall(ScalarFunctionCallContext ctx) {
UnsupportedSemanticVerifier.verify(ctx);
T func = visit(ctx.scalarFunctionName());
return reduce(func, ctx.functionArgs());
}

@Override
public T visitMathOperator(MathOperatorContext ctx) {
UnsupportedSemanticVerifier.verify(ctx);
return super.visitMathOperator(ctx);
}

@Override
public T visitRegexpPredicate(RegexpPredicateContext ctx) {
UnsupportedSemanticVerifier.verify(ctx);
return super.visitRegexpPredicate(ctx);
}

@Override
public T visitSelectElements(SelectElementsContext ctx) {
return visitor.visitSelect(ctx.selectElement().
Expand All @@ -247,6 +261,12 @@ public T visitSelectExpressionElement(SelectExpressionElementContext ctx) {
return visitSelectItem(ctx.expression(), ctx.uid());
}

@Override
public T visitAggregationAsArgFunctionCall(AggregationAsArgFunctionCallContext ctx) {
UnsupportedSemanticVerifier.verify(ctx);
return super.visitAggregationAsArgFunctionCall(ctx);
}

@Override
public T visitAggregateWindowedFunction(AggregateWindowedFunctionContext ctx) {
String funcName = ctx.getChild(0).getText();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package com.amazon.opendistroforelasticsearch.sql.antlr.visitor;

import com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser;
import com.amazon.opendistroforelasticsearch.sql.exception.SqlFeatureNotImplementedException;
import com.amazon.opendistroforelasticsearch.sql.utils.StringUtils;
import com.google.common.collect.Sets;

import java.util.Set;

import com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.AggregationAsArgFunctionCallContext;
import com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.MathOperatorContext;
import com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.RegexpPredicateContext;
import com.amazon.opendistroforelasticsearch.sql.antlr.parser.OpenDistroSqlParser.ScalarFunctionCallContext;

public class UnsupportedSemanticVerifier {

private static final Set<String> mathConstants = Sets.newHashSet(
"e", "pi"
);

private static final Set<String> supportedNestedFunctions = Sets.newHashSet(
"nested", "reverse_nested", "score", "match_query", "matchquery"
);

/**
* The following two sets include the functions and operators that have requested or issued by users
* but the plugin does not support yet.
*/
private static final Set<String> unsupportedFunctions = Sets.newHashSet(
"adddate", "addtime", "datetime", "greatest", "least"
);

private static final Set<String> unsupportedOperators = Sets.newHashSet(
"div"
);


/**
* The scalar function calls are separated into (a)typical function calls; (b)nested function calls with functions
* as arguments, like abs(log(...)); (c)aggregations with functions as aggregators, like max(abs(....)).
* Currently, we do not support nested functions or nested aggregations, aka (b) and (c).
* However, for the special EsFunctions included in the [supportedNestedFunctions] set, we have supported them in
* nested function calls and aggregations (b&c). Besides, the math constants included in the [mathConstants] set
* are regraded as scalar functions, but they are working well in the painless script.
*
* Thus, the types of functions to throw exceptions:
* (I)case (b) except that the arguments are from the [mathConstants] set;
* (II) case (b) except that the arguments are from the [supportedNestedFunctions] set;
* (III) case (c) except that the aggregators are from thet [supportedNestedFunctions] set.
*/
public static void verify(ScalarFunctionCallContext ctx) {
String funcName = StringUtils.toLower(ctx.scalarFunctionName().getText());

// type (III)
if (ctx.parent.parent instanceof OpenDistroSqlParser.FunctionAsAggregatorFunctionContext
&& !(supportedNestedFunctions.contains(StringUtils.toLower(funcName)))) {
throw new SqlFeatureNotImplementedException(StringUtils.format(
"Aggregation calls with function aggregator like [%s] are not supported yet",
ctx.parent.parent.getText()));

// type (I) and (II)
} else if (ctx.parent.parent instanceof OpenDistroSqlParser.NestedFunctionArgsContext
&& !(mathConstants.contains(funcName) || supportedNestedFunctions.contains(funcName))) {
throw new SqlFeatureNotImplementedException(StringUtils.format(
"Nested function calls like [%s] are not supported yet", ctx.parent.parent.parent.getText()));

// unsupported functions
} else if (unsupportedFunctions.contains(funcName)) {
throw new SqlFeatureNotImplementedException(StringUtils.format("Function [%s] is not supported yet",
funcName));
}
}

/**
* For functions with aggregation arguments, like abs(max(...));
* Temporarily added since this type of functions is under fixing.
*/
public static void verify(AggregationAsArgFunctionCallContext ctx) {
throw new SqlFeatureNotImplementedException(StringUtils.format(
"Nested function calls with aggregation argument like [%s] are not supported yet", ctx.getText()));
}

public static void verify(MathOperatorContext ctx) {
if (unsupportedOperators.contains(StringUtils.toLower(ctx.getText()))) {
throw new SqlFeatureNotImplementedException(StringUtils.format("Operator [%s] is not supported yet",
ctx.getText()));
}
}

public static void verify(RegexpPredicateContext ctx) {
throw new SqlFeatureNotImplementedException("Regexp predicate is not supported yet");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ public void compareSubstringFunctionCallEqualsToNumberShouldFail() {
}

@Test
public void compareLogAndAbsFunctionCallWithIntegerSmallerThanStringShouldFail() {
public void compareLogFunctionCallWithIntegerSmallerThanStringShouldFail() {
expectValidationFailWithErrorMessages(
"SELECT * FROM semantics WHERE LOG(ABS(age)) < 'test'",
"SELECT * FROM semantics WHERE LOG(age) < 'test'",
"Operator [<] cannot work with [DOUBLE, STRING]."
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

package com.amazon.opendistroforelasticsearch.sql.antlr.semantic;

import org.junit.Ignore;
import com.amazon.opendistroforelasticsearch.sql.antlr.semantic.types.base.ESDataType;
import org.junit.Test;

Expand Down Expand Up @@ -95,6 +96,7 @@ public void logFunctionCallWithUnknownFieldShouldPass() {
validate("SELECT LOG(new_field) FROM semantics");
}

@Ignore("Test set to ignore due to nested functions not supported and blocked by throwing SqlFeatureNotImplementedException")
@Test
public void substringWithLogFunctionCallWithUnknownFieldShouldPass() {
expectValidationFailWithErrorMessages(
Expand All @@ -104,21 +106,25 @@ public void substringWithLogFunctionCallWithUnknownFieldShouldPass() {
);
}

@Ignore("Test set to ignore due to nested functions not supported and blocked by throwing SqlFeatureNotImplementedException")
@Test
public void logFunctionCallWithResultOfAbsFunctionCallWithOneNumberShouldPass() {
validate("SELECT LOG(ABS(age)) FROM semantics");
}

@Ignore("Test set to ignore due to nested functions not supported and blocked by throwing SqlFeatureNotImplementedException")
@Test
public void logFunctionCallWithMoreNestedFunctionCallWithOneNumberShouldPass() {
validate("SELECT LOG(ABS(SQRT(balance))) FROM semantics");
}

@Ignore("Test set to ignore due to nested functions not supported and blocked by throwing SqlFeatureNotImplementedException")
@Test
public void substringFunctionCallWithResultOfAnotherSubstringAndAbsFunctionCallShouldPass() {
validate("SELECT SUBSTRING(SUBSTRING(city, ABS(age), 1), 2, ABS(1)) FROM semantics");
}

@Ignore("Test set to ignore due to nested functions not supported and blocked by throwing SqlFeatureNotImplementedException")
@Test
public void substringFunctionCallWithResultOfMathFunctionCallShouldFail() {
expectValidationFailWithErrorMessages(
Expand All @@ -128,6 +134,7 @@ public void substringFunctionCallWithResultOfMathFunctionCallShouldFail() {
);
}

@Ignore("Test set to ignore due to nested functions not supported and blocked by throwing SqlFeatureNotImplementedException")
@Test
public void logFunctionCallWithResultOfSubstringFunctionCallShouldFail() {
expectValidationFailWithErrorMessages(
Expand Down
Loading

0 comments on commit 77efb0b

Please sign in to comment.