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

Author: matriv

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/MathFunction.java

@@ -41,11 +41,8 @@ public Object fold() {
 
     @Override
     public String processScript(String template) {
-        return super.processScript(format(Locale.ROOT, "{sql}.%s(%s)", mathFunction(), template));
-    }
-
-    protected String mathFunction() {
-        return getClass().getSimpleName().toLowerCase(Locale.ROOT);
+        return super.processScript(format(
+            Locale.ROOT, "{sql}.%s(%s)", getClass().getSimpleName().toLowerCase(Locale.ROOT), template));
     }
 
     @Override

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Radians.java

@@ -29,11 +29,6 @@ protected Radians replaceChild(Expression newChild) {
         return new Radians(location(), newChild);
     }
 
-    @Override
-    protected String mathFunction() {
-        return "toRadians";
-    }
-
     @Override
     protected MathOperation operation() {
         return MathOperation.RADIANS;

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Sign.java

@@ -34,11 +34,6 @@ protected Sign replaceChild(Expression newChild) {
         return new Sign(location(), newChild);
     }
 
-    @Override
-    protected String mathFunction() {
-        return "signum";
-    }
-
     @Override
     protected MathOperation operation() {
         return MathOperation.SIGN;

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java

@@ -13,6 +13,7 @@
 import org.elasticsearch.xpack.sql.analysis.index.MappingException;
 import org.elasticsearch.xpack.sql.expression.Expression;
 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.plan.logical.Filter;
 import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
@@ -32,9 +33,13 @@
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 
+import java.util.Locale;
 import java.util.Map;
 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.startsWith;
 
@@ -365,4 +370,24 @@ public void testTranslateInExpression_HavingClause_PainlessAndNullHandling() {
         assertThat(aggFilter.scriptTemplate().params().toString(), startsWith("[{a=MAX(int){a->"));
         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}]"));
+    }
 }