Skip to content

Commit ca61f77

Browse files
nik9000nik9000
authored
ESQL: Limit the toString of plan nodes (#145140) (#145274)
Truncates the `toString` for ESQL's plan nodes when used outside the golden tests to 10 lines of 110 characters each. Why 110? Because that's where we truncated before I got to this code. Why 10 lines? Because we weren't truncating at all before. Just splitting at 110 characters one time. 10 lines of body for each plan node is quite large, really. Imagine a pathologically large plan, like 1000 nodes. That means at worst it's `toString` would be a little more than a megabyte. That's terrible! But I'm still ok logging it.
1 parent 851c4e1 commit ca61f77

26 files changed

Lines changed: 484 additions & 177 deletions

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public String toString() {
142142
}
143143

144144
@Override
145-
public String nodeString() {
145+
public String nodeString(NodeStringFormat format) {
146146
return child.nodeString() + " AS " + name() + "#" + id();
147147
}
148148

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ public String toString() {
220220
}
221221

222222
@Override
223-
public String nodeString() {
223+
public String nodeString(NodeStringFormat format) {
224224
return toString();
225225
}
226226

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,12 @@ public String toString() {
224224
}
225225

226226
@Override
227-
public String propertiesToString(boolean skipIfChild) {
228-
return super.propertiesToString(false);
227+
public String toString(NodeStringFormat format) {
228+
return toString();
229+
}
230+
231+
@Override
232+
public String propertiesToString(boolean skipIfChild, NodeStringFormat format) {
233+
return super.propertiesToString(false, format);
229234
}
230235
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ public String toString() {
171171
}
172172

173173
@Override
174-
public String nodeString() {
174+
public String nodeString(NodeStringFormat format) {
175175
return toString() + "[" + dataType + "]";
176176
}
177177

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public String toString() {
117117
}
118118

119119
@Override
120-
public String nodeString() {
120+
public String nodeString(NodeStringFormat format) {
121121
return name();
122122
}
123123
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public boolean isMetric() {
144144
}
145145

146146
@Override
147-
public String nodeString() {
147+
public String nodeString(NodeStringFormat format) {
148148
return toString();
149149
}
150150

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public String unresolvedMessage() {
7777
}
7878

7979
@Override
80-
public String nodeString() {
80+
public String nodeString(NodeStringFormat format) {
8181
return toString();
8282
}
8383

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/core/expression/function/Function.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,10 @@ public boolean equals(Object obj) {
5959
}
6060

6161
@Override
62-
public String nodeString() {
62+
public String nodeString(NodeStringFormat format) {
6363
StringJoiner sj = new StringJoiner(",", functionName() + "(", ")");
6464
for (Expression ex : arguments()) {
65-
sj.add(ex.nodeString());
65+
sj.add(ex.nodeString(format));
6666
}
6767
return sj.toString();
6868
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/core/expression/predicate/BinaryPredicate.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public F function() {
6565
}
6666

6767
@Override
68-
public String nodeString() {
68+
public String nodeString(NodeStringFormat format) {
6969
return left().nodeString() + " " + symbol() + " " + right().nodeString();
7070
}
7171
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/core/tree/Node.java‎

Lines changed: 39 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111
import org.elasticsearch.xpack.esql.core.util.Holder;
1212

1313
import java.util.ArrayList;
14-
import java.util.BitSet;
15-
import java.util.Iterator;
1614
import java.util.List;
1715
import java.util.Objects;
1816
import java.util.function.BiConsumer;
@@ -37,8 +35,18 @@
3735
* @param <T> node type
3836
*/
3937
public abstract class Node<T extends Node<T>> implements NamedWriteable {
38+
/**
39+
* Maximum number of properties rendered by {@link #toString}.
40+
*/
4041
private static final int TO_STRING_MAX_PROP = 10;
42+
/**
43+
* Maximum number of characters per line rendered by {@link #toString}.
44+
*/
4145
public static final int TO_STRING_MAX_WIDTH = 110;
46+
/**
47+
* Maximum number of lines rendered by {@link #toString}.
48+
*/
49+
public static final int TO_STRING_MAX_LINES = 25;
4250

4351
private final Source source;
4452
private final List<T> children;
@@ -63,7 +71,7 @@ public String sourceText() {
6371
return source.text();
6472
}
6573

66-
public List<T> children() {
74+
public final List<T> children() {
6775
return children;
6876
}
6977

@@ -384,139 +392,47 @@ public List<Object> nodeProperties() {
384392
return info().properties();
385393
}
386394

387-
public String nodeString() {
395+
public enum NodeStringFormat {
396+
/** No list truncation, no line breaks due to string width. */
397+
FULL(Integer.MAX_VALUE, Integer.MAX_VALUE, Integer.MAX_VALUE),
398+
/** List truncation and line breaks due to string width applied. */
399+
LIMITED(TO_STRING_MAX_PROP, TO_STRING_MAX_WIDTH, TO_STRING_MAX_LINES);
400+
401+
final int maxProperties;
402+
final int maxWidth;
403+
final int maxLines;
404+
405+
NodeStringFormat(int maxProperties, int maxWidth, int maxLines) {
406+
this.maxProperties = maxProperties;
407+
this.maxWidth = maxWidth;
408+
this.maxLines = maxLines;
409+
}
410+
}
411+
412+
public final String nodeString() {
413+
return nodeString(NodeStringFormat.LIMITED);
414+
}
415+
416+
public String nodeString(NodeStringFormat format) {
388417
StringBuilder sb = new StringBuilder();
389418
sb.append(nodeName());
390419
sb.append("[");
391-
sb.append(propertiesToString(true));
420+
sb.append(propertiesToString(true, format));
392421
sb.append("]");
393422
return sb.toString();
394423
}
395424

396425
@Override
397426
public String toString() {
398-
return treeString(new StringBuilder(), 0, new BitSet()).toString();
427+
return toString(NodeStringFormat.LIMITED);
399428
}
400429

401-
/**
402-
* Render this {@link Node} as a tree like
403-
* <pre>
404-
* {@code
405-
* Project[[i{f}#0]]
406-
* \_Filter[i{f}#1]
407-
* \_SubQueryAlias[test]
408-
* \_EsRelation[test][i{f}#2]
409-
* }
410-
* </pre>
411-
*/
412-
final StringBuilder treeString(StringBuilder sb, int depth, BitSet hasParentPerDepth) {
413-
if (depth > 0) {
414-
// draw children
415-
for (int column = 0; column < depth; column++) {
416-
if (hasParentPerDepth.get(column)) {
417-
sb.append("|");
418-
// if not the last elder, adding padding (since each column has two chars ("|_" or "\_")
419-
if (column < depth - 1) {
420-
sb.append(" ");
421-
}
422-
} else {
423-
// if the child has no parent (elder on the previous level), it means its the last sibling
424-
sb.append((column == depth - 1) ? "\\" : " ");
425-
}
426-
}
427-
428-
sb.append("_");
429-
}
430-
431-
sb.append(nodeString());
432-
433-
@SuppressWarnings("HiddenField")
434-
List<T> children = children();
435-
if (children.isEmpty() == false) {
436-
sb.append("\n");
437-
}
438-
for (int i = 0; i < children.size(); i++) {
439-
T t = children.get(i);
440-
hasParentPerDepth.set(depth, i < children.size() - 1);
441-
t.treeString(sb, depth + 1, hasParentPerDepth);
442-
if (i < children.size() - 1) {
443-
sb.append("\n");
444-
}
445-
}
446-
return sb;
430+
public String toString(NodeStringFormat format) {
431+
return new NodeToString(format).treeString(this, 0).toString();
447432
}
448433

449-
/**
450-
* Render the properties of this {@link Node} one by
451-
* one like {@code foo bar baz}. These go inside the
452-
* {@code [} and {@code ]} of the output of {@link #treeString}.
453-
*/
454-
public String propertiesToString(boolean skipIfChild) {
455-
StringBuilder sb = new StringBuilder();
456-
457-
@SuppressWarnings("HiddenField")
458-
List<?> children = children();
459-
// eliminate children (they are rendered as part of the tree)
460-
int remainingProperties = TO_STRING_MAX_PROP;
461-
int maxWidth = 0;
462-
boolean needsComma = false;
463-
464-
List<Object> props = nodeProperties();
465-
for (Object prop : props) {
466-
// consider a property if it is not ignored AND
467-
// it's not a child (optional)
468-
if ((skipIfChild && (children.contains(prop) || children.equals(prop))) == false) {
469-
if (remainingProperties-- < 0) {
470-
sb.append("...").append(props.size() - TO_STRING_MAX_PROP).append("fields not shown");
471-
break;
472-
}
473-
474-
if (needsComma) {
475-
sb.append(",");
476-
}
477-
478-
String stringValue = toString(prop);
479-
480-
// : Objects.toString(prop);
481-
if (maxWidth + stringValue.length() > TO_STRING_MAX_WIDTH) {
482-
int cutoff = Math.max(0, TO_STRING_MAX_WIDTH - maxWidth);
483-
sb.append(stringValue.substring(0, cutoff));
484-
sb.append("\n");
485-
stringValue = stringValue.substring(cutoff);
486-
maxWidth = 0;
487-
}
488-
maxWidth += stringValue.length();
489-
sb.append(stringValue);
490-
491-
needsComma = true;
492-
}
493-
}
494-
495-
return sb.toString();
496-
}
497-
498-
private static String toString(Object obj) {
499-
StringBuilder sb = new StringBuilder();
500-
toString(sb, obj);
501-
return sb.toString();
502-
}
503-
504-
private static void toString(StringBuilder sb, Object obj) {
505-
if (obj instanceof Iterable) {
506-
sb.append("[");
507-
for (Iterator<?> it = ((Iterable<?>) obj).iterator(); it.hasNext();) {
508-
Object o = it.next();
509-
toString(sb, o);
510-
if (it.hasNext()) {
511-
sb.append(", ");
512-
}
513-
}
514-
sb.append("]");
515-
} else if (obj instanceof Node<?>) {
516-
sb.append(((Node<?>) obj).nodeString());
517-
} else {
518-
sb.append(Objects.toString(obj));
519-
}
434+
protected String propertiesToString(boolean skipIfChild, NodeStringFormat format) {
435+
return new NodePropertiesToString(format, this, skipIfChild).propertiesToString();
520436
}
521437

522438
private <U> boolean containsNull(List<U> us) {

0 commit comments

Comments
 (0)