honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SQL: Fix issue with negative literels and parentheses (#48113) 

Previously when a numeric literal was enclosed in parentheses and then
negated, the negation was lost and the number was considered positive, e.g.:
`-(5)` was considered as `5` instead of `-5`
`- ( (1.28) )` was considered as `1.28` instead of `-1.28`

Fixes: #48009

(cherry picked from commit 4dee4bf3b34081062ba2e28ab8524a066812a180)
This commit is contained in:
Marios Trivyzas 2019-10-16 12:45:57 +02:00
parent 8f815240b3
commit 3233bce8cb
No known key found for this signature in database
GPG Key ID: 8817B46B0CF36A3F
4 changed files with 67 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java
View File

@ -869,11 +869,28 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
if (parentCtx != null) {
if (parentCtx instanceof SqlBaseParser.NumericLiteralContext) {
parentCtx = parentCtx.getParent();
if (parentCtx != null && parentCtx instanceof SqlBaseParser.ConstantDefaultContext) {
if (parentCtx instanceof ConstantDefaultContext) {
parentCtx = parentCtx.getParent();
if (parentCtx != null && parentCtx instanceof SqlBaseParser.ValueExpressionDefaultContext) {
if (parentCtx instanceof ValueExpressionDefaultContext) {
parentCtx = parentCtx.getParent();
if (parentCtx != null && parentCtx instanceof SqlBaseParser.ArithmeticUnaryContext) {
// Skip parentheses, e.g.: - (( (2.15) ) )
while (parentCtx instanceof PredicatedContext) {
parentCtx = parentCtx.getParent();
if (parentCtx instanceof SqlBaseParser.BooleanDefaultContext) {
parentCtx = parentCtx.getParent();
}
if (parentCtx instanceof SqlBaseParser.ExpressionContext) {
parentCtx = parentCtx.getParent();
}
if (parentCtx instanceof SqlBaseParser.ParenthesizedExpressionContext) {
parentCtx = parentCtx.getParent();
}
if (parentCtx instanceof ValueExpressionDefaultContext) {
parentCtx = parentCtx.getParent();
}
}
if (parentCtx instanceof ArithmeticUnaryContext) {
if (((ArithmeticUnaryContext) parentCtx).MINUS() != null) {
return source(parentCtx);
}

13
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/TestUtils.java
View File

@ -16,16 +16,18 @@ import java.time.Clock;
import java.time.Duration;
import java.time.ZonedDateTime;
import java.time.ZoneId;
import java.util.StringJoiner;
import static org.elasticsearch.test.ESTestCase.randomAlphaOfLength;
import static org.elasticsearch.test.ESTestCase.randomBoolean;
import static org.elasticsearch.test.ESTestCase.randomFrom;
import static org.elasticsearch.test.ESTestCase.randomInt;
import static org.elasticsearch.test.ESTestCase.randomIntBetween;
import static org.elasticsearch.test.ESTestCase.randomNonNegativeLong;
import static org.elasticsearch.test.ESTestCase.randomZone;
public class TestUtils {
public final class TestUtils {
private TestUtils() {}
@ -42,7 +44,7 @@ public class TestUtils {
*
* @return {@link ZonedDateTime} instance for the current date-time with milliseconds precision in UTC
*/
public static final ZonedDateTime now() {
public static ZonedDateTime now() {
return ZonedDateTime.now(Clock.tick(Clock.system(DateUtils.UTC), Duration.ofMillis(1)));
}
@ -74,5 +76,12 @@ public class TestUtils {
randomBoolean());
}
public static String randomWhitespaces() {
StringJoiner sj = new StringJoiner("");
for (int i = 0; i < randomInt(10); i++) {
sj.add(randomFrom(" ", "\t", "\r", "\n"));
}
return sj.toString();
}
}

10
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java
View File

@ -22,10 +22,10 @@ import org.junit.Assert;
import java.util.List;
import java.util.Locale;
import java.util.StringJoiner;
import static java.lang.String.format;
import static java.util.Arrays.asList;
import static org.elasticsearch.xpack.sql.TestUtils.randomWhitespaces;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
@ -35,14 +35,6 @@ public class EscapedFunctionsTests extends ESTestCase {
private final SqlParser parser = new SqlParser();
private String randomWhitespaces() {
StringJoiner sj = new StringJoiner("");
for (int i = 0; i < randomInt(10); i++) {
sj.add(randomFrom(" ", "\t", "\r", "\n"));
}
return sj.toString();
}
private String buildExpression(String escape, String pattern, Object value) {
return format(Locale.ROOT, "{" + randomWhitespaces() + escape + " " + randomWhitespaces() +
pattern + randomWhitespaces() + "}", value);

38
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java
View File

@ -6,6 +6,7 @@
package org.elasticsearch.xpack.sql.parser;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.TestUtils;
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.Literal;
import org.elasticsearch.xpack.sql.expression.function.UnresolvedFunction;
@ -202,10 +203,39 @@ public class ExpressionTests extends ESTestCase {
Expression expr = parser.createExpression("- 6");
assertEquals(Literal.class, expr.getClass());
assertEquals("- 6", expr.sourceText());
assertEquals(-6, expr.fold());
expr = parser.createExpression("- ( 6.134 )");
assertEquals(Literal.class, expr.getClass());
assertEquals("- ( 6.134 )", expr.sourceText());
assertEquals(-6.134, expr.fold());
expr = parser.createExpression("- ( ( (1.25) ) )");
assertEquals(Literal.class, expr.getClass());
assertEquals("- ( ( (1.25) ) )", expr.sourceText());
assertEquals(-1.25, expr.fold());
int numberOfParentheses = randomIntBetween(3, 10);
double value = randomDouble();
StringBuilder sb = new StringBuilder("-");
for (int i = 0; i < numberOfParentheses; i++) {
sb.append("(").append(TestUtils.randomWhitespaces());
}
sb.append(value);
for (int i = 0; i < numberOfParentheses; i++) {
sb.append(")");
if (i < numberOfParentheses - 1) {
sb.append(TestUtils.randomWhitespaces());
}
}
expr = parser.createExpression(sb.toString());
assertEquals(Literal.class, expr.getClass());
assertEquals(sb.toString(), expr.sourceText());
assertEquals(- value, expr.fold());
}
public void testComplexArithmetic() {
String sql = "-(((a-2)-(-3))+b)";
String sql = "-(((a-2)- (( - ( ( 3) )) ) )+ b)";
Expression expr = parser.createExpression(sql);
assertEquals(Neg.class, expr.getClass());
Neg neg = (Neg) expr;
@ -213,15 +243,15 @@ public class ExpressionTests extends ESTestCase {
assertEquals(1, neg.children().size());
assertEquals(Add.class, neg.children().get(0).getClass());
Add add = (Add) neg.children().get(0);
assertEquals("((a-2)-(-3))+b", add.sourceText());
assertEquals("((a-2)- (( - ( ( 3) )) ) )+ b", add.sourceText());
assertEquals(2, add.children().size());
assertEquals("?b", add.children().get(1).toString());
assertEquals(Sub.class, add.children().get(0).getClass());
Sub sub1 = (Sub) add.children().get(0);
assertEquals("(a-2)-(-3)", sub1.sourceText());
assertEquals("(a-2)- (( - ( ( 3) )) )", sub1.sourceText());
assertEquals(2, sub1.children().size());
assertEquals(Literal.class, sub1.children().get(1).getClass());
assertEquals("-3", sub1.children().get(1).sourceText());
assertEquals("- ( ( 3) )", sub1.children().get(1).sourceText());
assertEquals(Sub.class, sub1.children().get(0).getClass());
Sub sub2 = (Sub) sub1.children().get(0);
assertEquals(2, sub2.children().size());