update according to review comments

fang-xing-esql · Oct 31, 2024 · ac30df1 · ac30df1
1 parent 0517663
commit ac30df1
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 37 deletions.
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java
@@ -1227,9 +1227,15 @@ private Expression resolveConvertFunction(AbstractConvertFunction convert, List<
             if (convert.field() instanceof FieldAttribute fa && fa.field() instanceof InvalidMappedField imf) {
                 HashMap<TypeResolutionKey, Expression> typeResolutions = new HashMap<>();
                 Set<DataType> supportedTypes = convert.supportedTypes();
-                if (convert instanceof FoldablesConvertFunction fcf && supportedTypes.isEmpty()) {
+                if (convert instanceof FoldablesConvertFunction fcf) {
                     // FoldablesConvertFunction does not accept fields as inputs, they only accept constants
-                    return resolveFoldablesConvertFunction(fcf);
+                    String unresolvedMessage = "argument of ["
+                        + fcf.sourceText()
+                        + "] must be a constant, received ["
+                        + Expressions.name(fa)
+                        + "]";
+                    Expression ua = new UnresolvedAttribute(fa.source(), fa.name(), unresolvedMessage);
+                    return fcf.replaceChildren(Collections.singletonList(ua));
                 }
                 imf.types().forEach(type -> {
                     if (supportedTypes.contains(type.widenSmallNumeric())) {
@@ -1249,25 +1255,6 @@ private Expression resolveConvertFunction(AbstractConvertFunction convert, List<
             return convert;
         }
 
-        private static Expression resolveFoldablesConvertFunction(FoldablesConvertFunction fcf) {
-            if (fcf.field() instanceof FieldAttribute fa && fa.field() instanceof InvalidMappedField imf) {
-                String unresolvedMessage = "argument of ["
-                    + fcf.sourceText()
-                    + "] must be a constant, received ["
-                    + Expressions.name(fa)
-                    + "]";
-                String types = imf.getTypesToIndices().keySet().stream().collect(Collectors.joining(","));
-                Expression ua = new UnsupportedAttribute(
-                    fa.source(),
-                    fa.name(),
-                    new UnsupportedEsField(imf.getName(), types),
-                    unresolvedMessage
-                );
-                return fcf.replaceChildren(Collections.singletonList(ua));
-            }
-            return fcf;
-        }
-
         private Expression createIfDoesNotAlreadyExist(
             FieldAttribute fa,
             MultiTypeEsField resolvedField,

diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java
@@ -245,22 +245,33 @@ public void testUnsupportedAndMultiTypedFields() {
             error("from test* | where multi_typed is not null", analyzer)
         );
 
-        assertEquals(
-            "1:47: Cannot use field [unsupported] with unsupported type [flattened]",
-            error("from test* | eval x = now() + to_timeduration(unsupported)", analyzer)
-        );
-        assertEquals(
-            "1:45: argument of [to_dateperiod(multi_typed)] must be a constant, received [multi_typed]",
-            error("from test* | eval x = now() + to_dateperiod(multi_typed)", analyzer)
-        );
-        assertThat(
-            error("from test* | eval x = unsupported, y = now() + to_timeduration(x)", analyzer),
-            containsString("1:23: Cannot use field [unsupported] with unsupported type [flattened]")
-        );
-        assertThat(
-            error("from test* | eval x = multi_typed, y = now() + to_dateperiod(x)", analyzer),
-            containsString("1:48: argument of [to_dateperiod(x)] must be [date_period or string], found value [x] type [unsupported]")
-        );
+        for (String functionName : List.of("to_timeduration", "to_dateperiod")) {
+            String lineNumber = functionName.equalsIgnoreCase("to_timeduration") ? "47" : "45";
+            String errorType = functionName.equalsIgnoreCase("to_timeduration") ? "time_duration" : "date_period";
+            assertEquals(
+                "1:" + lineNumber + ": Cannot use field [unsupported] with unsupported type [flattened]",
+                error("from test* | eval x = now() + " + functionName + "(unsupported)", analyzer)
+            );
+            assertEquals(
+                "1:" + lineNumber + ": argument of [" + functionName + "(multi_typed)] must be a constant, received [multi_typed]",
+                error("from test* | eval x = now() + " + functionName + "(multi_typed)", analyzer)
+            );
+            assertThat(
+                error("from test* | eval x = unsupported, y = now() + " + functionName + "(x)", analyzer),
+                containsString("1:23: Cannot use field [unsupported] with unsupported type [flattened]")
+            );
+            assertThat(
+                error("from test* | eval x = multi_typed, y = now() + " + functionName + "(x)", analyzer),
+                containsString(
+                    "1:48: argument of ["
+                        + functionName
+                        + "(x)] must be ["
+                        + errorType
+                        + " or string], "
+                        + "found value [x] type [unsupported]"
+                )
+            );
+        }
     }
 
     public void testRoundFunctionInvalidInputs() {