SQL: Fix translation of math functions to painless (#35910)
`SIGN` and `RADIANS` where wrongly overriding `mathFunction()`. Converted `mathFunction()` to private in `MathFunction` since it shouldn't be overriden, as it uses the assigned `MathOperation` to get the funtion name for painless scripts. Fixes: #35654
This commit is contained in:
parent
8d9e21ffaa
commit
1da9c6faa0
|
@ -41,11 +41,8 @@ public abstract class MathFunction extends UnaryScalarFunction {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public String processScript(String template) {
|
public String processScript(String template) {
|
||||||
return super.processScript(format(Locale.ROOT, "{sql}.%s(%s)", mathFunction(), template));
|
return super.processScript(format(
|
||||||
}
|
Locale.ROOT, "{sql}.%s(%s)", getClass().getSimpleName().toLowerCase(Locale.ROOT), template));
|
||||||
|
|
||||||
protected String mathFunction() {
|
|
||||||
return getClass().getSimpleName().toLowerCase(Locale.ROOT);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|
|
@ -29,11 +29,6 @@ public class Radians extends MathFunction {
|
||||||
return new Radians(location(), newChild);
|
return new Radians(location(), newChild);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
|
||||||
protected String mathFunction() {
|
|
||||||
return "toRadians";
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected MathOperation operation() {
|
protected MathOperation operation() {
|
||||||
return MathOperation.RADIANS;
|
return MathOperation.RADIANS;
|
||||||
|
|
|
@ -34,11 +34,6 @@ public class Sign extends MathFunction {
|
||||||
return new Sign(location(), newChild);
|
return new Sign(location(), newChild);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
|
||||||
protected String mathFunction() {
|
|
||||||
return "signum";
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected MathOperation operation() {
|
protected MathOperation operation() {
|
||||||
return MathOperation.SIGN;
|
return MathOperation.SIGN;
|
||||||
|
|
|
@ -14,6 +14,7 @@ import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
|
||||||
import org.elasticsearch.xpack.sql.analysis.index.MappingException;
|
import org.elasticsearch.xpack.sql.analysis.index.MappingException;
|
||||||
import org.elasticsearch.xpack.sql.expression.Expression;
|
import org.elasticsearch.xpack.sql.expression.Expression;
|
||||||
import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
|
import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
|
||||||
|
import org.elasticsearch.xpack.sql.expression.function.scalar.math.MathProcessor.MathOperation;
|
||||||
import org.elasticsearch.xpack.sql.parser.SqlParser;
|
import org.elasticsearch.xpack.sql.parser.SqlParser;
|
||||||
import org.elasticsearch.xpack.sql.plan.logical.Filter;
|
import org.elasticsearch.xpack.sql.plan.logical.Filter;
|
||||||
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
|
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
|
||||||
|
@ -34,9 +35,13 @@ import org.elasticsearch.xpack.sql.util.DateUtils;
|
||||||
import org.junit.AfterClass;
|
import org.junit.AfterClass;
|
||||||
import org.junit.BeforeClass;
|
import org.junit.BeforeClass;
|
||||||
|
|
||||||
|
import java.util.Locale;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.TimeZone;
|
import java.util.TimeZone;
|
||||||
|
import java.util.stream.Stream;
|
||||||
|
|
||||||
|
import static org.elasticsearch.xpack.sql.expression.function.scalar.math.MathProcessor.MathOperation.E;
|
||||||
|
import static org.elasticsearch.xpack.sql.expression.function.scalar.math.MathProcessor.MathOperation.PI;
|
||||||
import static org.hamcrest.Matchers.endsWith;
|
import static org.hamcrest.Matchers.endsWith;
|
||||||
import static org.hamcrest.Matchers.startsWith;
|
import static org.hamcrest.Matchers.startsWith;
|
||||||
|
|
||||||
|
@ -367,4 +372,24 @@ public class QueryTranslatorTests extends ESTestCase {
|
||||||
assertThat(aggFilter.scriptTemplate().params().toString(), startsWith("[{a=MAX(int){a->"));
|
assertThat(aggFilter.scriptTemplate().params().toString(), startsWith("[{a=MAX(int){a->"));
|
||||||
assertThat(aggFilter.scriptTemplate().params().toString(), endsWith(", {v=[10, null, 20, 30]}]"));
|
assertThat(aggFilter.scriptTemplate().params().toString(), endsWith(", {v=[10, null, 20, 30]}]"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public void testTranslateMathFunction_HavingClause_Painless() {
|
||||||
|
MathOperation operation =
|
||||||
|
(MathOperation) randomFrom(Stream.of(MathOperation.values()).filter(o -> o != PI && o != E).toArray());
|
||||||
|
|
||||||
|
LogicalPlan p = plan("SELECT keyword, max(int) FROM test GROUP BY keyword HAVING " +
|
||||||
|
operation.name() + "(max(int)) > 10");
|
||||||
|
assertTrue(p instanceof Project);
|
||||||
|
assertTrue(p.children().get(0) instanceof Filter);
|
||||||
|
Expression condition = ((Filter) p.children().get(0)).condition();
|
||||||
|
assertFalse(condition.foldable());
|
||||||
|
QueryTranslation translation = QueryTranslator.toQuery(condition, true);
|
||||||
|
assertNull(translation.query);
|
||||||
|
AggFilter aggFilter = translation.aggFilter;
|
||||||
|
assertEquals("InternalSqlScriptUtils.nullSafeFilter(InternalSqlScriptUtils.gt(InternalSqlScriptUtils." +
|
||||||
|
operation.name().toLowerCase(Locale.ROOT) + "(params.a0),params.v0))",
|
||||||
|
aggFilter.scriptTemplate().toString());
|
||||||
|
assertThat(aggFilter.scriptTemplate().params().toString(), startsWith("[{a=MAX(int){a->"));
|
||||||
|
assertThat(aggFilter.scriptTemplate().params().toString(), endsWith(", {v=10}]"));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue