Skip to content

Commit 1421da9

Browse files
committed
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
1 parent dc30a25 commit 1421da9

File tree

2 files changed

+25
-7
lines changed

2 files changed

+25
-7
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@
3636
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.StatementContext;
3737
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.StatementDefaultContext;
3838
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.UnquoteIdentifierContext;
39-
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.ValueExpressionContext;
40-
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.ValueExpressionDefaultContext;
4139
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
4240
import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue;
4341

@@ -241,7 +239,6 @@ static class CircuitBreakerListener extends SqlBaseBaseListener {
241239
ENTER_EXIT_RULE_MAPPING.put(StatementDefaultContext.class.getSimpleName(), StatementContext.class.getSimpleName());
242240
ENTER_EXIT_RULE_MAPPING.put(QueryPrimaryDefaultContext.class.getSimpleName(), QueryTermContext.class.getSimpleName());
243241
ENTER_EXIT_RULE_MAPPING.put(BooleanDefaultContext.class.getSimpleName(), BooleanExpressionContext.class.getSimpleName());
244-
ENTER_EXIT_RULE_MAPPING.put(ValueExpressionDefaultContext.class.getSimpleName(), ValueExpressionContext.class.getSimpleName());
245242
}
246243

247244
private boolean insideIn = false;
@@ -264,6 +261,9 @@ public void enterEveryRule(ParserRuleContext ctx) {
264261
if (ctx.getClass() != UnquoteIdentifierContext.class &&
265262
ctx.getClass() != QuoteIdentifierContext.class &&
266263
ctx.getClass() != BackQuotedIdentifierContext.class &&
264+
ctx.getClass() != SqlBaseParser.ConstantContext.class &&
265+
ctx.getClass() != SqlBaseParser.NumberContext.class &&
266+
ctx.getClass() != SqlBaseParser.ValueExpressionContext.class &&
267267
(insideIn == false || ctx.getClass() != PrimaryExpressionContext.class)) {
268268

269269
int currentDepth = depthCounts.putOrAdd(ctx.getClass().getSimpleName(), (short) 1, (short) 1);

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/SqlParserTests.java

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,9 +287,18 @@ public void testLimitToPreventStackOverflowFromLargeComplexSubselectTree() {
287287
}
288288

289289
public void testLimitStackOverflowForInAndLiteralsIsNotApplied() {
290-
int noChildren = 100_000;
290+
int noChildren = 10_000;
291291
LogicalPlan plan = parseStatement("SELECT * FROM t WHERE a IN(" +
292-
Joiner.on(",").join(nCopies(noChildren, "a + b")) + ")");
292+
Joiner.on(",").join(nCopies(noChildren, "a + 10")) + "," +
293+
Joiner.on(",").join(nCopies(noChildren, "-(-a - 10)")) + "," +
294+
Joiner.on(",").join(nCopies(noChildren, "20")) + "," +
295+
Joiner.on(",").join(nCopies(noChildren, "-20")) + "," +
296+
Joiner.on(",").join(nCopies(noChildren, "20.1234")) + "," +
297+
Joiner.on(",").join(nCopies(noChildren, "-20.4321")) + "," +
298+
Joiner.on(",").join(nCopies(noChildren, "1.1234E56")) + "," +
299+
Joiner.on(",").join(nCopies(noChildren, "-1.4321E-65")) + "," +
300+
Joiner.on(",").join(nCopies(noChildren, "'foo'")) + "," +
301+
Joiner.on(",").join(nCopies(noChildren, "'bar'")) + ")");
293302

294303
assertEquals(With.class, plan.getClass());
295304
assertEquals(Project.class, ((With) plan).child().getClass());
@@ -298,8 +307,17 @@ public void testLimitStackOverflowForInAndLiteralsIsNotApplied() {
298307
assertEquals(In.class, filter.condition().getClass());
299308
In in = (In) filter.condition();
300309
assertEquals("?a", in.value().toString());
301-
assertEquals(noChildren, in.list().size());
302-
assertThat(in.list().get(0).toString(), startsWith("(a) + (b)#"));
310+
assertEquals(noChildren * 2 + 8, in.list().size());
311+
assertThat(in.list().get(0).toString(), startsWith("(a) + 10#"));
312+
assertThat(in.list().get(noChildren).toString(), startsWith("-(-?a) - 10#"));
313+
assertEquals("20", in.list().get(noChildren * 2).toString());
314+
assertEquals("-20", in.list().get(noChildren * 2 + 1).toString());
315+
assertEquals("20.1234", in.list().get(noChildren * 2 + 2).toString());
316+
assertEquals("-20.4321", in.list().get(noChildren * 2 + 3).toString());
317+
assertEquals("1.1234E56", in.list().get(noChildren * 2 + 4).toString());
318+
assertEquals("-1.4321E-65", in.list().get(noChildren * 2 + 5).toString());
319+
assertEquals("foo", in.list().get(noChildren * 2 + 6).toString());
320+
assertEquals("bar", in.list().get(noChildren * 2 + 7).toString());
303321
}
304322

305323
public void testDecrementOfDepthCounter() {

0 commit comments

Comments
 (0)