Skip to content

Commit 0a14f27

Browse files
authored
ESQL: Limit size of Literal#toString (#117842) (#117849)
This `toString` is rendered in task output and progress. Let's make sure it's not massive.
1 parent a6950cb commit 0a14f27

File tree

4 files changed

+103
-13
lines changed
  • docs/changelog
  • test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack
  • x-pack/plugin/esql-core/src

4 files changed

+103
-13
lines changed

docs/changelog/117842.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 117842
2+
summary: Limit size of `Literal#toString`
3+
area: ES|QL
4+
type: bug
5+
issues: []

test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack/HeapAttackIT.java

Lines changed: 73 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -295,31 +295,35 @@ private Response concat(int evals) throws IOException {
295295
* Returns many moderately long strings.
296296
*/
297297
public void testManyConcat() throws IOException {
298+
int strings = 300;
298299
initManyLongs();
299-
Response resp = manyConcat(300);
300-
Map<?, ?> map = responseAsMap(resp);
301-
ListMatcher columns = matchesList();
302-
for (int s = 0; s < 300; s++) {
303-
columns = columns.item(matchesMap().entry("name", "str" + s).entry("type", "keyword"));
304-
}
305-
MapMatcher mapMatcher = matchesMap();
306-
assertMap(map, mapMatcher.entry("columns", columns).entry("values", any(List.class)).entry("took", greaterThanOrEqualTo(0)));
300+
Response resp = manyConcat("FROM manylongs", strings);
301+
assertManyStrings(resp, strings);
307302
}
308303

309304
/**
310305
* Hits a circuit breaker by building many moderately long strings.
311306
*/
312307
public void testHugeManyConcat() throws IOException {
313308
initManyLongs();
314-
assertCircuitBreaks(() -> manyConcat(2000));
309+
assertCircuitBreaks(() -> manyConcat("FROM manylongs", 2000));
310+
}
311+
312+
/**
313+
* Returns many moderately long strings.
314+
*/
315+
public void testManyConcatFromRow() throws IOException {
316+
int strings = 2000;
317+
Response resp = manyConcat("ROW a=9999, b=9999, c=9999, d=9999, e=9999", strings);
318+
assertManyStrings(resp, strings);
315319
}
316320

317321
/**
318322
* Tests that generate many moderately long strings.
319323
*/
320-
private Response manyConcat(int strings) throws IOException {
324+
private Response manyConcat(String init, int strings) throws IOException {
321325
StringBuilder query = startQuery();
322-
query.append("FROM manylongs | EVAL str = CONCAT(");
326+
query.append(init).append(" | EVAL str = CONCAT(");
323327
query.append(
324328
Arrays.stream(new String[] { "a", "b", "c", "d", "e" })
325329
.map(f -> "TO_STRING(" + f + ")")
@@ -344,7 +348,64 @@ private Response manyConcat(int strings) throws IOException {
344348
query.append("str").append(s);
345349
}
346350
query.append("\"}");
347-
return query(query.toString(), null);
351+
return query(query.toString(), "columns");
352+
}
353+
354+
/**
355+
* Returns many moderately long strings.
356+
*/
357+
public void testManyRepeat() throws IOException {
358+
int strings = 30;
359+
initManyLongs();
360+
Response resp = manyRepeat("FROM manylongs", strings);
361+
assertManyStrings(resp, 30);
362+
}
363+
364+
/**
365+
* Hits a circuit breaker by building many moderately long strings.
366+
*/
367+
public void testHugeManyRepeat() throws IOException {
368+
initManyLongs();
369+
assertCircuitBreaks(() -> manyRepeat("FROM manylongs", 75));
370+
}
371+
372+
/**
373+
* Returns many moderately long strings.
374+
*/
375+
public void testManyRepeatFromRow() throws IOException {
376+
int strings = 10000;
377+
Response resp = manyRepeat("ROW a = 99", strings);
378+
assertManyStrings(resp, strings);
379+
}
380+
381+
/**
382+
* Tests that generate many moderately long strings.
383+
*/
384+
private Response manyRepeat(String init, int strings) throws IOException {
385+
StringBuilder query = startQuery();
386+
query.append(init).append(" | EVAL str = TO_STRING(a)");
387+
for (int s = 0; s < strings; s++) {
388+
query.append(",\nstr").append(s).append("=REPEAT(str, 10000)");
389+
}
390+
query.append("\n|KEEP ");
391+
for (int s = 0; s < strings; s++) {
392+
if (s != 0) {
393+
query.append(", ");
394+
}
395+
query.append("str").append(s);
396+
}
397+
query.append("\"}");
398+
return query(query.toString(), "columns");
399+
}
400+
401+
private void assertManyStrings(Response resp, int strings) throws IOException {
402+
Map<?, ?> map = responseAsMap(resp);
403+
ListMatcher columns = matchesList();
404+
for (int s = 0; s < strings; s++) {
405+
columns = columns.item(matchesMap().entry("name", "str" + s).entry("type", "keyword"));
406+
}
407+
MapMatcher mapMatcher = matchesMap();
408+
assertMap(map, mapMatcher.entry("columns", columns));
348409
}
349410

350411
public void testManyEval() throws IOException {

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Literal.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,11 @@ public boolean equals(Object obj) {
122122

123123
@Override
124124
public String toString() {
125-
return String.valueOf(value);
125+
String str = String.valueOf(value);
126+
if (str.length() > 500) {
127+
return str.substring(0, 500) + "...";
128+
}
129+
return str;
126130
}
127131

128132
@Override

x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/LiteralTests.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@
66
*/
77
package org.elasticsearch.xpack.esql.core.expression;
88

9+
import joptsimple.internal.Strings;
10+
911
import org.elasticsearch.test.ESTestCase;
1012
import org.elasticsearch.xpack.esql.core.InvalidArgumentException;
1113
import org.elasticsearch.xpack.esql.core.tree.AbstractNodeTestCase;
14+
import org.elasticsearch.xpack.esql.core.tree.Source;
1215
import org.elasticsearch.xpack.esql.core.tree.SourceTests;
1316
import org.elasticsearch.xpack.esql.core.type.Converter;
1417
import org.elasticsearch.xpack.esql.core.type.DataType;
@@ -17,6 +20,7 @@
1720
import java.util.ArrayList;
1821
import java.util.Arrays;
1922
import java.util.List;
23+
import java.util.Objects;
2024
import java.util.function.Function;
2125
import java.util.function.Supplier;
2226

@@ -29,9 +33,12 @@
2933
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
3034
import static org.elasticsearch.xpack.esql.core.type.DataType.LONG;
3135
import static org.elasticsearch.xpack.esql.core.type.DataType.SHORT;
36+
import static org.hamcrest.Matchers.equalTo;
3237

3338
public class LiteralTests extends AbstractNodeTestCase<Literal, Expression> {
39+
3440
static class ValueAndCompatibleTypes {
41+
3542
final Supplier<Object> valueSupplier;
3643
final List<DataType> validDataTypes;
3744

@@ -120,6 +127,19 @@ public void testReplaceChildren() {
120127
assertEquals("this type of node doesn't have any children to replace", e.getMessage());
121128
}
122129

130+
public void testToString() {
131+
assertThat(new Literal(Source.EMPTY, 1, LONG).toString(), equalTo("1"));
132+
assertThat(new Literal(Source.EMPTY, "short", KEYWORD).toString(), equalTo("short"));
133+
// toString should limit it's length
134+
String tooLong = Strings.repeat('a', 510);
135+
assertThat(new Literal(Source.EMPTY, tooLong, KEYWORD).toString(), equalTo(Strings.repeat('a', 500) + "..."));
136+
137+
for (ValueAndCompatibleTypes g : GENERATORS) {
138+
Literal lit = new Literal(Source.EMPTY, g.valueSupplier.get(), randomFrom(g.validDataTypes));
139+
assertThat(lit.toString(), equalTo(Objects.toString(lit.value())));
140+
}
141+
}
142+
123143
private static Object randomValueOfTypeOtherThan(Object original, DataType type) {
124144
for (ValueAndCompatibleTypes gen : GENERATORS) {
125145
if (gen.validDataTypes.get(0) == type) {

0 commit comments

Comments
 (0)