Skip to content

Commit 7f21b17

Browse files
Marios Trivyzaskcm
Marios Trivyzas
authored andcommitted
SQL: Remove more ANTLR4 grammar ambiguities (#34074)
The `-` and `+` as a number literal prefix are already parsed by the rule in `valueExpression`. To accommodate this, there are some code changes that enables the `ExpressionBuilder` to parse Literal integers and decimals together with the `-/+` prefix sign (if exists) and validate them (wrong format, large numbers, etc.). Follows: #33854
1 parent e96446f commit 7f21b17

File tree

5 files changed

+355
-293
lines changed

5 files changed

+355
-293
lines changed

x-pack/plugin/sql/src/main/antlr/SqlBase.g4

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,8 @@ unquoteIdentifier
307307
;
308308

309309
number
310-
: (PLUS | MINUS)? DECIMAL_VALUE #decimalLiteral
311-
| (PLUS | MINUS)? INTEGER_VALUE #integerLiteral
310+
: DECIMAL_VALUE #decimalLiteral
311+
| INTEGER_VALUE #integerLiteral
312312
;
313313

314314
string

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

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,9 @@ public Object visitArithmeticUnary(ArithmeticUnaryContext ctx) {
286286
case SqlBaseParser.PLUS:
287287
return value;
288288
case SqlBaseParser.MINUS:
289+
if (value instanceof Literal) { // Minus already processed together with literal number
290+
return value;
291+
}
289292
return new Neg(source(ctx.operator), value);
290293
default:
291294
throw new ParsingException(loc, "Unknown arithemtic {}", ctx.operator.getText());
@@ -483,38 +486,40 @@ public Expression visitStringLiteral(StringLiteralContext ctx) {
483486

484487
@Override
485488
public Literal visitDecimalLiteral(DecimalLiteralContext ctx) {
489+
String ctxText = (hasMinusFromParent(ctx) ? "-" : "") + ctx.getText();
486490
double value;
487491
try {
488-
value = Double.parseDouble(ctx.getText());
492+
value = Double.parseDouble(ctxText);
489493
} catch (NumberFormatException nfe) {
490-
throw new ParsingException(source(ctx), "Cannot parse number [{}]", ctx.getText());
494+
throw new ParsingException(source(ctx), "Cannot parse number [{}]", ctxText);
491495
}
492496
if (Double.isInfinite(value)) {
493-
throw new ParsingException(source(ctx), "Number [{}] is too large", ctx.getText());
497+
throw new ParsingException(source(ctx), "Number [{}] is too large", ctxText);
494498
}
495499
if (Double.isNaN(value)) {
496-
throw new ParsingException(source(ctx), "[{}] cannot be parsed as a number (NaN)", ctx.getText());
500+
throw new ParsingException(source(ctx), "[{}] cannot be parsed as a number (NaN)", ctxText);
497501
}
498502
return new Literal(source(ctx), Double.valueOf(value), DataType.DOUBLE);
499503
}
500504

501505
@Override
502506
public Literal visitIntegerLiteral(IntegerLiteralContext ctx) {
507+
String ctxText = (hasMinusFromParent(ctx) ? "-" : "") + ctx.getText();
503508
long value;
504509
try {
505-
value = Long.parseLong(ctx.getText());
510+
value = Long.parseLong(ctxText);
506511
} catch (NumberFormatException nfe) {
507512
try {
508-
BigInteger bi = new BigInteger(ctx.getText());
513+
BigInteger bi = new BigInteger(ctxText);
509514
try {
510515
bi.longValueExact();
511516
} catch (ArithmeticException ae) {
512-
throw new ParsingException(source(ctx), "Number [{}] is too large", ctx.getText());
517+
throw new ParsingException(source(ctx), "Number [{}] is too large", ctxText);
513518
}
514519
} catch (NumberFormatException ex) {
515520
// parsing fails, go through
516521
}
517-
throw new ParsingException(source(ctx), "Cannot parse number [{}]", ctx.getText());
522+
throw new ParsingException(source(ctx), "Cannot parse number [{}]", ctxText);
518523
}
519524

520525
DataType type = DataType.LONG;
@@ -681,4 +686,21 @@ public Literal visitGuidEscapedLiteral(GuidEscapedLiteralContext ctx) {
681686

682687
return new Literal(source(ctx), string, DataType.KEYWORD);
683688
}
689+
690+
private boolean hasMinusFromParent(SqlBaseParser.NumberContext ctx) {
691+
ParserRuleContext parentCtx = ctx.getParent();
692+
if (parentCtx != null && parentCtx instanceof SqlBaseParser.NumericLiteralContext) {
693+
parentCtx = parentCtx.getParent();
694+
if (parentCtx != null && parentCtx instanceof SqlBaseParser.ConstantDefaultContext) {
695+
parentCtx = parentCtx.getParent();
696+
if (parentCtx != null && parentCtx instanceof SqlBaseParser.ValueExpressionDefaultContext) {
697+
parentCtx = parentCtx.getParent();
698+
if (parentCtx != null && parentCtx instanceof SqlBaseParser.ArithmeticUnaryContext) {
699+
return ((ArithmeticUnaryContext) parentCtx).MINUS() != null;
700+
}
701+
}
702+
}
703+
}
704+
return false;
705+
}
684706
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/SqlBaseLexer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -409,8 +409,8 @@ public SqlBaseLexer(CharStream input) {
409409
"\u0316\u0319\5\u00cfh\2\u0317\u0319\t\4\2\2\u0318\u0315\3\2\2\2\u0318"+
410410
"\u0316\3\2\2\2\u0318\u0317\3\2\2\2\u0319\u031a\3\2\2\2\u031a\u0318\3\2"+
411411
"\2\2\u031a\u031b\3\2\2\2\u031b\u00c6\3\2\2\2\u031c\u0320\5\u00d1i\2\u031d"+
412-
"\u0320\5\u00cfh\2\u031e\u0320\t\3\2\2\u031f\u031c\3\2\2\2\u031f\u031d"+
413-
"\3\2\2\2\u031f\u031e\3\2\2\2\u0320\u0321\3\2\2\2\u0321\u031f\3\2\2\2\u0321"+
412+
"\u0320\5\u00cfh\2\u031e\u0320\7a\2\2\u031f\u031c\3\2\2\2\u031f\u031d\3"+
413+
"\2\2\2\u031f\u031e\3\2\2\2\u0320\u0321\3\2\2\2\u0321\u031f\3\2\2\2\u0321"+
414414
"\u0322\3\2\2\2\u0322\u00c8\3\2\2\2\u0323\u0329\7$\2\2\u0324\u0328\n\5"+
415415
"\2\2\u0325\u0326\7$\2\2\u0326\u0328\7$\2\2\u0327\u0324\3\2\2\2\u0327\u0325"+
416416
"\3\2\2\2\u0328\u032b\3\2\2\2\u0329\u0327\3\2\2\2\u0329\u032a\3\2\2\2\u032a"+

0 commit comments

Comments
 (0)