SQL: Relax StackOverflow circuit breaker for constants (#38572)
Constant numbers (of any form: integers, decimals, negatives, scientific) and strings shouldn't increase the depth counters as they don't contribute to the increment of the stack depth. Fixes: #38571
This commit is contained in:
parent
6abe99808a
commit
871036bd21
|
@ -6,7 +6,6 @@
|
||||||
package org.elasticsearch.xpack.sql.parser;
|
package org.elasticsearch.xpack.sql.parser;
|
||||||
|
|
||||||
import com.carrotsearch.hppc.ObjectShortHashMap;
|
import com.carrotsearch.hppc.ObjectShortHashMap;
|
||||||
|
|
||||||
import org.antlr.v4.runtime.BaseErrorListener;
|
import org.antlr.v4.runtime.BaseErrorListener;
|
||||||
import org.antlr.v4.runtime.CharStream;
|
import org.antlr.v4.runtime.CharStream;
|
||||||
import org.antlr.v4.runtime.CommonToken;
|
import org.antlr.v4.runtime.CommonToken;
|
||||||
|
@ -37,8 +36,6 @@ import org.elasticsearch.xpack.sql.parser.SqlBaseParser.QuoteIdentifierContext;
|
||||||
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.StatementContext;
|
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.StatementContext;
|
||||||
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.StatementDefaultContext;
|
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.StatementDefaultContext;
|
||||||
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.UnquoteIdentifierContext;
|
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.UnquoteIdentifierContext;
|
||||||
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.ValueExpressionContext;
|
|
||||||
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.ValueExpressionDefaultContext;
|
|
||||||
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
|
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
|
||||||
import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue;
|
import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue;
|
||||||
|
|
||||||
|
@ -242,7 +239,6 @@ public class SqlParser {
|
||||||
ENTER_EXIT_RULE_MAPPING.put(StatementDefaultContext.class.getSimpleName(), StatementContext.class.getSimpleName());
|
ENTER_EXIT_RULE_MAPPING.put(StatementDefaultContext.class.getSimpleName(), StatementContext.class.getSimpleName());
|
||||||
ENTER_EXIT_RULE_MAPPING.put(QueryPrimaryDefaultContext.class.getSimpleName(), QueryTermContext.class.getSimpleName());
|
ENTER_EXIT_RULE_MAPPING.put(QueryPrimaryDefaultContext.class.getSimpleName(), QueryTermContext.class.getSimpleName());
|
||||||
ENTER_EXIT_RULE_MAPPING.put(BooleanDefaultContext.class.getSimpleName(), BooleanExpressionContext.class.getSimpleName());
|
ENTER_EXIT_RULE_MAPPING.put(BooleanDefaultContext.class.getSimpleName(), BooleanExpressionContext.class.getSimpleName());
|
||||||
ENTER_EXIT_RULE_MAPPING.put(ValueExpressionDefaultContext.class.getSimpleName(), ValueExpressionContext.class.getSimpleName());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private boolean insideIn = false;
|
private boolean insideIn = false;
|
||||||
|
@ -265,6 +261,9 @@ public class SqlParser {
|
||||||
if (ctx.getClass() != UnquoteIdentifierContext.class &&
|
if (ctx.getClass() != UnquoteIdentifierContext.class &&
|
||||||
ctx.getClass() != QuoteIdentifierContext.class &&
|
ctx.getClass() != QuoteIdentifierContext.class &&
|
||||||
ctx.getClass() != BackQuotedIdentifierContext.class &&
|
ctx.getClass() != BackQuotedIdentifierContext.class &&
|
||||||
|
ctx.getClass() != SqlBaseParser.ConstantContext.class &&
|
||||||
|
ctx.getClass() != SqlBaseParser.NumberContext.class &&
|
||||||
|
ctx.getClass() != SqlBaseParser.ValueExpressionContext.class &&
|
||||||
(insideIn == false || ctx.getClass() != PrimaryExpressionContext.class)) {
|
(insideIn == false || ctx.getClass() != PrimaryExpressionContext.class)) {
|
||||||
|
|
||||||
int currentDepth = depthCounts.putOrAdd(ctx.getClass().getSimpleName(), (short) 1, (short) 1);
|
int currentDepth = depthCounts.putOrAdd(ctx.getClass().getSimpleName(), (short) 1, (short) 1);
|
||||||
|
|
|
@ -294,9 +294,18 @@ public class SqlParserTests extends ESTestCase {
|
||||||
}
|
}
|
||||||
|
|
||||||
public void testLimitStackOverflowForInAndLiteralsIsNotApplied() {
|
public void testLimitStackOverflowForInAndLiteralsIsNotApplied() {
|
||||||
int noChildren = 100_000;
|
int noChildren = 10_000;
|
||||||
LogicalPlan plan = parseStatement("SELECT * FROM t WHERE a IN(" +
|
LogicalPlan plan = parseStatement("SELECT * FROM t WHERE a IN(" +
|
||||||
Joiner.on(",").join(nCopies(noChildren, "a + b")) + ")");
|
Joiner.on(",").join(nCopies(noChildren, "a + 10")) + "," +
|
||||||
|
Joiner.on(",").join(nCopies(noChildren, "-(-a - 10)")) + "," +
|
||||||
|
Joiner.on(",").join(nCopies(noChildren, "20")) + "," +
|
||||||
|
Joiner.on(",").join(nCopies(noChildren, "-20")) + "," +
|
||||||
|
Joiner.on(",").join(nCopies(noChildren, "20.1234")) + "," +
|
||||||
|
Joiner.on(",").join(nCopies(noChildren, "-20.4321")) + "," +
|
||||||
|
Joiner.on(",").join(nCopies(noChildren, "1.1234E56")) + "," +
|
||||||
|
Joiner.on(",").join(nCopies(noChildren, "-1.4321E-65")) + "," +
|
||||||
|
Joiner.on(",").join(nCopies(noChildren, "'foo'")) + "," +
|
||||||
|
Joiner.on(",").join(nCopies(noChildren, "'bar'")) + ")");
|
||||||
|
|
||||||
assertEquals(With.class, plan.getClass());
|
assertEquals(With.class, plan.getClass());
|
||||||
assertEquals(Project.class, ((With) plan).child().getClass());
|
assertEquals(Project.class, ((With) plan).child().getClass());
|
||||||
|
@ -305,8 +314,17 @@ public class SqlParserTests extends ESTestCase {
|
||||||
assertEquals(In.class, filter.condition().getClass());
|
assertEquals(In.class, filter.condition().getClass());
|
||||||
In in = (In) filter.condition();
|
In in = (In) filter.condition();
|
||||||
assertEquals("?a", in.value().toString());
|
assertEquals("?a", in.value().toString());
|
||||||
assertEquals(noChildren, in.list().size());
|
assertEquals(noChildren * 2 + 8, in.list().size());
|
||||||
assertThat(in.list().get(0).toString(), startsWith("Add[?a,?b]"));
|
assertThat(in.list().get(0).toString(), startsWith("Add[?a,10]#"));
|
||||||
|
assertThat(in.list().get(noChildren).toString(), startsWith("Neg[Sub[Neg[?a]#"));
|
||||||
|
assertEquals("20", in.list().get(noChildren * 2).toString());
|
||||||
|
assertEquals("-20", in.list().get(noChildren * 2 + 1).toString());
|
||||||
|
assertEquals("20.1234", in.list().get(noChildren * 2 + 2).toString());
|
||||||
|
assertEquals("-20.4321", in.list().get(noChildren * 2 + 3).toString());
|
||||||
|
assertEquals("1.1234E56", in.list().get(noChildren * 2 + 4).toString());
|
||||||
|
assertEquals("-1.4321E-65", in.list().get(noChildren * 2 + 5).toString());
|
||||||
|
assertEquals("'foo'=foo", in.list().get(noChildren * 2 + 6).toString());
|
||||||
|
assertEquals("'bar'=bar", in.list().get(noChildren * 2 + 7).toString());
|
||||||
}
|
}
|
||||||
|
|
||||||
public void testDecrementOfDepthCounter() {
|
public void testDecrementOfDepthCounter() {
|
||||||
|
|
Loading…
Reference in New Issue