SQL: Correct error message (#30138)

* SQL: Correct error message

Error messages had placeholders that were not replaced; this PR fixes
that

Fix #30016

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java

@@ -10,6 +10,7 @@ import org.elasticsearch.xpack.sql.expression.Attribute;
 import org.elasticsearch.xpack.sql.expression.BinaryExpression;
 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.FieldAttribute;
 import org.elasticsearch.xpack.sql.expression.Literal;
 import org.elasticsearch.xpack.sql.expression.NamedExpression;
@@ -159,7 +160,7 @@ abstract class QueryTranslator {
             }
         }
 
-        throw new UnsupportedOperationException(format(Locale.ROOT, "Don't know how to translate %s %s", e.nodeName(), e));
+        throw new SqlIllegalArgumentException("Don't know how to translate {} {}", e.nodeName(), e);
     }
 
     static LeafAgg toAgg(String id, Function f) {
@@ -171,7 +172,7 @@ abstract class QueryTranslator {
             }
         }
 
-        throw new UnsupportedOperationException(format(Locale.ROOT, "Don't know how to translate %s %s", f.nodeName(), f));
+        throw new SqlIllegalArgumentException("Don't know how to translate {} {}", f.nodeName(), f);
     }
 
     static class GroupingContext {
@@ -395,8 +396,8 @@ abstract class QueryTranslator {
         if (arg instanceof Literal) {
             return String.valueOf(((Literal) arg).value());
         }
-        throw new SqlIllegalArgumentException("Does not know how to convert argument " + arg.nodeString()
-                + " for function " + af.nodeString());
+        throw new SqlIllegalArgumentException("Does not know how to convert argument {} for function {}", arg.nodeString(),
+                af.nodeString());
     }
 
     // TODO: need to optimize on ngram
@@ -505,9 +506,9 @@ abstract class QueryTranslator {
         @Override
         protected QueryTranslation asQuery(BinaryComparison bc, boolean onAggs) {
             Check.isTrue(bc.right().foldable(),
-                    "Line %d:%d - Comparisons against variables are not (currently) supported; offender %s in %s",
+                    "Line {}:{}: Comparisons against variables are not (currently) supported; offender [{}] in [{}]",
                     bc.right().location().getLineNumber(), bc.right().location().getColumnNumber(),
-                    bc.right().nodeName(), bc.nodeName());
+                    Expressions.name(bc.right()), bc.symbol());
 
             if (bc.left() instanceof NamedExpression) {
                 NamedExpression ne = (NamedExpression) bc.left();
@@ -605,8 +606,8 @@ abstract class QueryTranslator {
                 return new TermQuery(loc, name, value);
             }
 
-            Check.isTrue(false, "don't know how to translate binary comparison [{}] in [{}]", bc.right().nodeString(), bc);
-            return null;
+            throw new SqlIllegalArgumentException("Don't know how to translate binary comparison [{}] in [{}]", bc.right().nodeString(),
+                    bc);
         }
     }
 
@@ -700,9 +701,8 @@ abstract class QueryTranslator {
                 return new QueryTranslation(query, aggFilter);
             }
             else {
-                throw new UnsupportedOperationException("No idea how to translate " + e);
+                throw new SqlIllegalArgumentException("No idea how to translate " + e);
             }
-
         }
     }
 

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java

@@ -153,7 +153,7 @@ public class FieldAttributeTests extends ESTestCase {
     public void testStarExpansionExcludesObjectAndUnsupportedTypes() {
         LogicalPlan plan = plan("SELECT * FROM test");
         List<? extends NamedExpression> list = ((Project) plan).projections();
-        assertThat(list, hasSize(7));
+        assertThat(list, hasSize(8));
         List<String> names = Expressions.names(list);
         assertThat(names, not(hasItem("some")));
         assertThat(names, not(hasItem("some.dotted")));

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java

@@ -17,7 +17,7 @@ public class SysColumnsTests extends ESTestCase {
     public void testSysColumns() {
         List<List<?>> rows = new ArrayList<>();
         SysColumns.fillInRows("test", "index", TypesTests.loadMapping("mapping-multi-field-variation.json", true), null, rows, null);
-        assertEquals(15, rows.size());
+        assertEquals(16, rows.size());
         assertEquals(24, rows.get(0).size());
 
         List<?> row = rows.get(0);
@@ -38,13 +38,13 @@ public class SysColumnsTests extends ESTestCase {
         assertEquals(null, radix(row));
         assertEquals(Integer.MAX_VALUE, bufferLength(row));
 
-        row = rows.get(6);
+        row = rows.get(7);
         assertEquals("some.dotted", name(row));
         assertEquals(Types.STRUCT, sqlType(row));
         assertEquals(null, radix(row));
         assertEquals(-1, bufferLength(row));
 
-        row = rows.get(14);
+        row = rows.get(15);
         assertEquals("some.ambiguous.normalized", name(row));
         assertEquals(Types.VARCHAR, sqlType(row));
         assertEquals(null, radix(row));

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

@@ -6,6 +6,7 @@
 package org.elasticsearch.xpack.sql.planner;
 
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
 import org.elasticsearch.xpack.sql.analysis.analyzer.Analyzer;
 import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
 import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
@@ -18,9 +19,11 @@ 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.query.Query;
+import org.elasticsearch.xpack.sql.querydsl.query.RangeQuery;
 import org.elasticsearch.xpack.sql.querydsl.query.TermQuery;
 import org.elasticsearch.xpack.sql.type.EsField;
 import org.elasticsearch.xpack.sql.type.TypesTests;
+import org.joda.time.DateTime;
 
 import java.util.Map;
 import java.util.TimeZone;
@@ -84,4 +87,56 @@ public class QueryTranslatorTests extends ESTestCase {
         assertEquals("int", tq.term());
         assertEquals(5, tq.value());
     }
+
+    public void testComparisonAgainstColumns() {
+        LogicalPlan p = plan("SELECT some.string FROM test WHERE date > int");
+        assertTrue(p instanceof Project);
+        p = ((Project) p).child();
+        assertTrue(p instanceof Filter);
+        Expression condition = ((Filter) p).condition();
+        SqlIllegalArgumentException ex = expectThrows(SqlIllegalArgumentException.class, () -> QueryTranslator.toQuery(condition, false));
+        assertEquals("Line 1:43: Comparisons against variables are not (currently) supported; offender [int] in [>]", ex.getMessage());
+    }
+
+    public void testDateRange() {
+        LogicalPlan p = plan("SELECT some.string FROM test WHERE date > 1969-05-13");
+        assertTrue(p instanceof Project);
+        p = ((Project) p).child();
+        assertTrue(p instanceof Filter);
+        Expression condition = ((Filter) p).condition();
+        QueryTranslation translation = QueryTranslator.toQuery(condition, false);
+        Query query = translation.query;
+        assertTrue(query instanceof RangeQuery);
+        RangeQuery rq = (RangeQuery) query;
+        assertEquals("date", rq.field());
+        assertEquals(1951, rq.lower());
+    }
+
+    public void testDateRangeLiteral() {
+        LogicalPlan p = plan("SELECT some.string FROM test WHERE date > '1969-05-13'");
+        assertTrue(p instanceof Project);
+        p = ((Project) p).child();
+        assertTrue(p instanceof Filter);
+        Expression condition = ((Filter) p).condition();
+        QueryTranslation translation = QueryTranslator.toQuery(condition, false);
+        Query query = translation.query;
+        assertTrue(query instanceof RangeQuery);
+        RangeQuery rq = (RangeQuery) query;
+        assertEquals("date", rq.field());
+        assertEquals("1969-05-13", rq.lower());
+    }
+
+    public void testDateRangeCast() {
+        LogicalPlan p = plan("SELECT some.string FROM test WHERE date > CAST('1969-05-13T12:34:56Z' AS DATE)");
+        assertTrue(p instanceof Project);
+        p = ((Project) p).child();
+        assertTrue(p instanceof Filter);
+        Expression condition = ((Filter) p).condition();
+        QueryTranslation translation = QueryTranslator.toQuery(condition, false);
+        Query query = translation.query;
+        assertTrue(query instanceof RangeQuery);
+        RangeQuery rq = (RangeQuery) query;
+        assertEquals("date", rq.field());
+        assertEquals(DateTime.parse("1969-05-13T12:34:56Z"), rq.lower());
+    }
 }

x-pack/plugin/sql/src/test/resources/mapping-multi-field-variation.json

@@ -4,6 +4,7 @@
         "int" : { "type" : "integer" },
         "text" : { "type" : "text" },
         "keyword" : { "type" : "keyword" },
+        "date" :  { "type" : "date" },
         "unsupported" : { "type" : "ip_range" },
         "some" : {
             "properties" : {