From 66fccef593086e95e60b9a38c6dbeb677609c5d4 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Fri, 14 Sep 2018 15:35:33 +0200 Subject: [PATCH] SQL: Enchance output of explain for optimized plan When a literal expression is replaced with its evaluated value, in the optimizer, then use this new evaluated value in the explain plan instead of the original expression. --- .../sql/expression/function/Function.java | 31 ++++++++++++++++++- .../xpack/sql/parser/ExpressionTests.java | 4 +-- .../xpack/qa/sql/nosecurity/CliExplainIT.java | 18 +++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/Function.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/Function.java index 000a9c097c98f..c605c5b6f7efd 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/Function.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/Function.java @@ -8,11 +8,13 @@ import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.ExpressionId; import org.elasticsearch.xpack.sql.expression.Expressions; +import org.elasticsearch.xpack.sql.expression.Literal; import org.elasticsearch.xpack.sql.expression.NamedExpression; import org.elasticsearch.xpack.sql.tree.Location; import org.elasticsearch.xpack.sql.util.StringUtils; import java.util.List; +import java.util.Objects; import java.util.StringJoiner; /** @@ -51,7 +53,7 @@ public boolean nullable() { @Override public String toString() { - return name() + "#" + id(); + return functionName + functionArgsForToString() + "#" + id(); } public String functionName() { @@ -75,4 +77,31 @@ protected String functionArgs() { public boolean functionEquals(Function f) { return f != null && getClass() == f.getClass() && arguments().equals(f.arguments()); } + + /** + * Build the string representation of the function arguments for the {@link #toString()}. + * - If an argument is a Function call {@code toString()} to work recursively + * - If an argument is a Literal then just use the evaluated value. + * - If an argument is a NamedExpression then use it's {@code name()} to avoid long confusing output + * - If none of those resolve to the {@code toString()} method of the argument + * + * @return String representation of the function args for the {@link #toString} representation + */ + private String functionArgsForToString() { + StringJoiner sj = new StringJoiner(",", "(", ")"); + for (Expression child : children()) { + String val; + if (child instanceof Function) { + val = child.toString(); + } else if (child instanceof Literal) { + val = Objects.toString(((Literal) child).value()); + } else if (child instanceof NamedExpression && child.resolved()) { + val = Expressions.name(child); + } else { + val = child.toString(); + } + sj.add(val); + } + return sj.toString(); + } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java index 0ee0c9bcca122..0871dfd9a1f45 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java @@ -16,8 +16,6 @@ import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.Sub; import org.elasticsearch.xpack.sql.type.DataType; -import static org.hamcrest.core.StringStartsWith.startsWith; - public class ExpressionTests extends ESTestCase { private final SqlParser parser = new SqlParser(); @@ -137,7 +135,7 @@ public void testComplexArithmetic() { Expression expr = parser.createExpression("-(((a-2)-(-3))+b)"); assertEquals(Neg.class, expr.getClass()); Neg neg = (Neg) expr; - assertThat(neg.name(), startsWith("-(((a) - 2) - -3) + (b)#")); + assertEquals("-ADD(SUB(SUB(?a,2),-3),?b)", neg.name().replaceAll("#\\d+", "")); assertEquals(1, neg.children().size()); assertEquals(Add.class, neg.children().get(0).getClass()); Add add = (Add) neg.children().get(0); diff --git a/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java b/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java index cbf6d0d476e57..022c199b9bd3b 100644 --- a/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java +++ b/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java @@ -160,4 +160,22 @@ public void testExplainWithCount() throws IOException { assertThat(readLine(), startsWith("}]")); assertEquals("", readLine()); } + + public void testOptimizedPlanWithFoldableLiteralExpressions() throws IOException { + index("test", body -> body.field("test_field", "test_value1").field("i", 1)); + index("test", body -> body.field("test_field", "test_value2").field("i", 2)); + + assertThat(command("EXPLAIN (PLAN OPTIMIZED) SELECT POWER(i, (1 + 2)) FROM test"), containsString("plan")); + assertThat(readLine(), startsWith("----------")); + assertEquals("Project[[POWER(i,3)]]", readLine().replaceAll("#\\d+", "")); + assertThat(readLine(), startsWith("\\_EsRelation[test][i{f}")); + assertEquals("", readLine()); + + assertThat(command("EXPLAIN (PLAN OPTIMIZED) SELECT POWER(i, (1 - 2)) * ABS(10 + 20) FROM test"), + containsString("plan")); + assertThat(readLine(), startsWith("----------")); + assertEquals("Project[[MUL(POWER(i,-1),30)]]", readLine().replaceAll("#\\d+", "")); + assertThat(readLine(), startsWith("\\_EsRelation[test][i{f}#")); + assertEquals("", readLine()); + } }