Browse Source

SQL: Fix translation to painless for conditionals (#36636)

Add missing `formatTemplate()` for conditional functions which
resulted in incomplete painless script. Moreover the specific
return type of Object in the painless signatures resulted in
casting exceptions when conditional functions are used in the
ORDER BY.

Fixes: #36631
Marios Trivyzas 6 years ago
parent
commit
db8f07c665

+ 12 - 0
x-pack/plugin/sql/qa/src/main/resources/null.sql-spec

@@ -11,6 +11,9 @@ SELECT COALESCE(null, ABS(MAX(emp_no)) + 1, 123) AS c FROM test_emp GROUP BY lan
 coalesceWhere
 SELECT COALESCE(null, ABS(emp_no) + 1, 123) AS c FROM test_emp WHERE COALESCE(null, ABS(emp_no) + 1, 123, 321) > 100 ORDER BY emp_no NULLS FIRST LIMIT 5;
 
+coalesceOrderBy
+SELECT COALESCE(null, ABS(emp_no) + 1, 123) AS c FROM test_emp ORDER BY c NULLS FIRST LIMIT 5;
+
 ifNullField
 SELECT IFNULL(null, ABS(emp_no) + 1) AS "ifnull" FROM test_emp ORDER BY emp_no LIMIT 5;
 
@@ -23,6 +26,9 @@ SELECT NULLIF(10002, ABS(emp_no) + 1) AS c, emp_no FROM test_emp WHERE NULLIF(10
 nullIfHaving
 SELECT NULLIF(10030, ABS(MAX(emp_no)) + 1) AS nif FROM test_emp GROUP BY languages HAVING nif IS NOT NULL ORDER BY languages;
 
+nullIfOrderBy
+SELECT NULLIF(10030, ABS(emp_no + 1)) AS nif FROM test_emp ORDER BY nif NULLS FIRST LIMIT 5;
+
 greatestField
 SELECT GREATEST(emp_no - 1 + 3, ABS(emp_no) + 1) AS "greatest" FROM test_emp ORDER BY emp_no LIMIT 5;
 
@@ -32,6 +38,9 @@ SELECT emp_no FROM test_emp WHERE GREATEST(10005, ABS(emp_no) + 1, null, emp_no
 greatestHaving
 SELECT GREATEST(10096, ABS(MAX(emp_no)) + 1) AS gt FROM test_emp GROUP BY languages HAVING gt >= 10098 ORDER BY languages;
 
+greatestOrderBy
+SELECT GREATEST(10096, ABS(emp_no + 1)) AS gt FROM test_emp ORDER BY gt LIMIT 10;
+
 leastField
 SELECT LEAST(emp_no - 1 + 3, ABS(emp_no) + 1) AS "least" FROM test_emp ORDER BY emp_no LIMIT 5;
 
@@ -40,3 +49,6 @@ SELECT emp_no FROM test_emp WHERE LEAST(10005, ABS(emp_no) + 1, null, emp_no - 1
 
 leastHaving
 SELECT LEAST(10098, ABS(MAX(emp_no)) + 1) AS lt FROM test_emp GROUP BY languages HAVING lt >= 10095 ORDER BY languages;
+
+leastOrderBy
+SELECT LEAST(10096, ABS(emp_no + 1)) AS lt FROM test_emp ORDER BY lt LIMIT 10;

+ 1 - 1
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/ArbitraryConditionalFunction.java

@@ -61,6 +61,6 @@ public abstract class ArbitraryConditionalFunction extends ConditionalFunction {
             params.script(scriptTemplate.params());
         }
 
-        return new ScriptTemplate(template.toString(), params.build(), dataType());
+        return new ScriptTemplate(formatTemplate(template.toString()), params.build(), dataType());
     }
 }

+ 1 - 1
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/NullIf.java

@@ -75,7 +75,7 @@ public class NullIf extends ConditionalFunction {
         params.script(left.params());
         params.script(right.params());
 
-        return new ScriptTemplate(template, params.build(), dataType);
+        return new ScriptTemplate(formatTemplate(template), params.build(), dataType);
     }
 
     @Override

+ 4 - 4
x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt

@@ -46,10 +46,10 @@ class org.elasticsearch.xpack.sql.expression.function.scalar.whitelist.InternalS
 #
 # Null
 #
-  Object coalesce(java.util.List)
-  Object greatest(java.util.List)
-  Object least(java.util.List)
-  Object nullif(Object, Object)
+  def coalesce(java.util.List)
+  def greatest(java.util.List)
+  def least(java.util.List)
+  def nullif(Object, Object)
 
 #
 # Regex

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

@@ -15,12 +15,15 @@ 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.expression.gen.script.ScriptTemplate;
 import org.elasticsearch.xpack.sql.parser.SqlParser;
+import org.elasticsearch.xpack.sql.plan.logical.Aggregate;
 import org.elasticsearch.xpack.sql.plan.logical.Filter;
 import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
 import org.elasticsearch.xpack.sql.plan.logical.Project;
 import org.elasticsearch.xpack.sql.planner.QueryTranslator.QueryTranslation;
 import org.elasticsearch.xpack.sql.querydsl.agg.AggFilter;
+import org.elasticsearch.xpack.sql.querydsl.agg.GroupByScriptKey;
 import org.elasticsearch.xpack.sql.querydsl.query.ExistsQuery;
 import org.elasticsearch.xpack.sql.querydsl.query.NotQuery;
 import org.elasticsearch.xpack.sql.querydsl.query.Query;
@@ -392,4 +395,30 @@ public class QueryTranslatorTests extends ESTestCase {
         assertThat(aggFilter.scriptTemplate().params().toString(), startsWith("[{a=MAX(int){a->"));
         assertThat(aggFilter.scriptTemplate().params().toString(), endsWith(", {v=10}]"));
     }
+
+    public void testTranslateCoalesce_GroupBy_Painless() {
+        LogicalPlan p = plan("SELECT COALESCE(int, 10) FROM test GROUP BY 1");
+        assertTrue(p instanceof Aggregate);
+        Expression condition = ((Aggregate) p).groupings().get(0);
+        assertFalse(condition.foldable());
+        QueryTranslator.GroupingContext groupingContext = QueryTranslator.groupBy(((Aggregate) p).groupings());
+        assertNotNull(groupingContext);
+        ScriptTemplate scriptTemplate = ((GroupByScriptKey) groupingContext.tail).script();
+        assertEquals("InternalSqlScriptUtils.coalesce([InternalSqlScriptUtils.docValue(doc,params.v0),params.v1])",
+            scriptTemplate.toString());
+        assertEquals("[{v=int}, {v=10}]", scriptTemplate.params().toString());
+    }
+
+    public void testTranslateNullIf_GroupBy_Painless() {
+        LogicalPlan p = plan("SELECT NULLIF(int, 10) FROM test GROUP BY 1");
+        assertTrue(p instanceof Aggregate);
+        Expression condition = ((Aggregate) p).groupings().get(0);
+        assertFalse(condition.foldable());
+        QueryTranslator.GroupingContext groupingContext = QueryTranslator.groupBy(((Aggregate) p).groupings());
+        assertNotNull(groupingContext);
+        ScriptTemplate scriptTemplate = ((GroupByScriptKey) groupingContext.tail).script();
+        assertEquals("InternalSqlScriptUtils.nullif(InternalSqlScriptUtils.docValue(doc,params.v0),params.v1)",
+            scriptTemplate.toString());
+        assertEquals("[{v=int}, {v=10}]", scriptTemplate.params().toString());
+    }
 }