Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -51,7 +53,7 @@ public boolean nullable() {

@Override
public String toString() {
return name() + "#" + id();
return functionName + functionArgsForToString() + "#" + id();
}

public String functionName() {
Expand All @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good case of overloading/method dispatch.
Literal already returns its value or, in case it is named, name=value.
Function does something similar I believe (especially for operators).

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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down