Support UDT for BINARY (#3549)

---------

Signed-off-by: Heng Qian <[email protected]>

Author: qianheng-aws

core/src/main/java/org/opensearch/sql/calcite/type/ExprBinaryType.java

@@ -0,0 +1,16 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.sql.calcite.type;
+
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory;
+import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.ExprUDT;
+
+public class ExprBinaryType extends ExprSqlType {
+  public ExprBinaryType(OpenSearchTypeFactory typeFactory) {
+    super(typeFactory, ExprUDT.EXPR_BINARY, SqlTypeName.VARCHAR);
+  }
+}

core/src/main/java/org/opensearch/sql/calcite/utils/OpenSearchTypeFactory.java

@@ -6,6 +6,7 @@
 package org.opensearch.sql.calcite.utils;
 
 import static org.opensearch.sql.data.type.ExprCoreType.ARRAY;
+import static org.opensearch.sql.data.type.ExprCoreType.BINARY;
 import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN;
 import static org.opensearch.sql.data.type.ExprCoreType.BYTE;
 import static org.opensearch.sql.data.type.ExprCoreType.DATE;
@@ -41,6 +42,7 @@
 import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.sql.type.SqlTypeUtil;
 import org.opensearch.sql.calcite.type.AbstractExprRelDataType;
+import org.opensearch.sql.calcite.type.ExprBinaryType;
 import org.opensearch.sql.calcite.type.ExprDateType;
 import org.opensearch.sql.calcite.type.ExprIPType;
 import org.opensearch.sql.calcite.type.ExprTimeStampType;
@@ -67,6 +69,7 @@ public enum ExprUDT {
     EXPR_DATE(DATE),
     EXPR_TIME(TIME),
     EXPR_TIMESTAMP(TIMESTAMP),
+    EXPR_BINARY(BINARY),
     EXPR_IP(IP);
 
     // Associated `ExprCoreType`
@@ -120,6 +123,8 @@ public RelDataType createUDT(ExprUDT typeName) {
             yield new ExprTimeType(this);
           case EXPR_TIMESTAMP:
             yield new ExprTimeStampType(this);
+          case EXPR_BINARY:
+            yield new ExprBinaryType(this);
           case EXPR_IP:
             yield new ExprIPType(this);
         };
@@ -180,7 +185,7 @@ public static RelDataType convertExprTypeToRelDataType(ExprType fieldType, boole
       }
     } else {
       if (fieldType.legacyTypeName().equalsIgnoreCase("binary")) {
-        return TYPE_FACTORY.createSqlType(SqlTypeName.BINARY, nullable);
+        return TYPE_FACTORY.createUDT(ExprUDT.EXPR_BINARY, nullable);
       } else if (fieldType.legacyTypeName().equalsIgnoreCase("timestamp")) {
         return TYPE_FACTORY.createUDT(ExprUDT.EXPR_TIMESTAMP, nullable);
       } else if (fieldType.legacyTypeName().equalsIgnoreCase("date")) {
@@ -243,20 +248,9 @@ public static ExprType convertSqlTypeNameToExprType(SqlTypeName sqlTypeName) {
 
   /** Get legacy name for a RelDataType. */
   public static String getLegacyTypeName(RelDataType relDataType, QueryType queryType) {
-    if (relDataType instanceof AbstractExprRelDataType<?> udt) {
-      return udt.getExprType().legacyTypeName();
-    }
-    switch (relDataType.getSqlTypeName()) {
-      case BINARY:
-      case VARBINARY:
-        return "BINARY";
-      case GEOMETRY:
-        return "GEO_POINT";
-      default:
-        ExprType type = convertSqlTypeNameToExprType(relDataType.getSqlTypeName());
-        return (queryType == PPL ? PPL_SPEC.typeName(type) : type.legacyTypeName())
-            .toUpperCase(Locale.ROOT);
-    }
+    ExprType type = convertRelDataTypeToExprType(relDataType);
+    return (queryType == PPL ? PPL_SPEC.typeName(type) : type.legacyTypeName())
+        .toUpperCase(Locale.ROOT);
   }
 
   /** Converts a Calcite data type to OpenSearch ExprCoreType. */

core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java

@@ -51,6 +51,8 @@ public enum ExprCoreType implements ExprType {
   /** Geometry. Only support point now. */
   GEO_POINT(UNDEFINED),
 
+  BINARY(UNDEFINED),
+
   /** Struct. */
   STRUCT(UNDEFINED),
 

core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java

@@ -161,12 +161,7 @@ public void filter_relation_with_invalid_qualifiedName_ExpressionEvaluationExcep
 
     ExpressionEvaluationException exception =
         assertThrows(ExpressionEvaluationException.class, () -> analyze(typeMismatchPlan));
-    assertEquals(
-        "= function expected {[BYTE,BYTE],[SHORT,SHORT],[INTEGER,INTEGER],[LONG,LONG],"
-            + "[FLOAT,FLOAT],[DOUBLE,DOUBLE],[STRING,STRING],[BOOLEAN,BOOLEAN],[DATE,DATE],"
-            + "[TIME,TIME],[TIMESTAMP,TIMESTAMP],[INTERVAL,INTERVAL],[IP,IP],[GEO_POINT,GEO_POINT],"
-            + "[STRUCT,STRUCT],[ARRAY,ARRAY]}, but got [STRING,INTEGER]",
-        exception.getMessage());
+    assertEquals(getIncompatibleTypeErrMsg(STRING, INTEGER), exception.getMessage());
   }
 
   @Test

core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java

@@ -26,6 +26,7 @@
 import org.opensearch.sql.analysis.symbol.SymbolTable;
 import org.opensearch.sql.ast.tree.UnresolvedPlan;
 import org.opensearch.sql.config.TestConfig;
+import org.opensearch.sql.data.type.ExprCoreType;
 import org.opensearch.sql.data.type.ExprType;
 import org.opensearch.sql.datasource.DataSourceService;
 import org.opensearch.sql.datasource.RequestContext;
@@ -274,4 +275,14 @@ public Table applyArguments() {
       return table;
     }
   }
+
+  public static String getIncompatibleTypeErrMsg(ExprType lType, ExprType rType) {
+    return String.format(
+        "= function expected %s, but got [%s,%s]",
+        ExprCoreType.coreTypes().stream()
+            .map(type -> String.format("[%s,%s]", type.typeName(), type.typeName()))
+            .collect(Collectors.joining(",", "{", "}")),
+        lType,
+        rType);
+  }
 }

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteDataTypeIT.java

@@ -5,7 +5,6 @@
 
 package org.opensearch.sql.calcite.remote;
 
-import java.io.IOException;
 import org.opensearch.sql.ppl.DataTypeIT;
 
 public class CalciteDataTypeIT extends DataTypeIT {
@@ -15,17 +14,4 @@ public void init() throws Exception {
     enableCalcite();
     disallowCalciteFallback();
   }
-
-  @Override
-  public void test_nonnumeric_data_types() throws IOException {
-    withFallbackEnabled(
-        () -> {
-          try {
-            super.test_nonnumeric_data_types();
-          } catch (IOException e) {
-            throw new RuntimeException(e);
-          }
-        },
-        "ignore this class since IP type is unsupported in calcite engine");
-  }
 }

integ-test/src/test/java/org/opensearch/sql/ppl/DataTypeIT.java

@@ -14,9 +14,12 @@
 import static org.opensearch.sql.util.MatcherUtils.rows;
 import static org.opensearch.sql.util.MatcherUtils.schema;
 import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
+import static org.opensearch.sql.util.MatcherUtils.verifyDataRowsInOrder;
 import static org.opensearch.sql.util.MatcherUtils.verifySchema;
+import static org.opensearch.sql.util.MatcherUtils.verifySchemaInOrder;
 
 import java.io.IOException;
+import org.json.JSONArray;
 import org.json.JSONObject;
 import org.junit.Test;
 
@@ -48,18 +51,33 @@ public void test_numeric_data_types() throws IOException {
   @Test
   public void test_nonnumeric_data_types() throws IOException {
     JSONObject result = executeQuery(String.format("source=%s", TEST_INDEX_DATATYPE_NONNUMERIC));
-    verifySchema(
+    verifySchemaInOrder(
         result,
-        schema("boolean_value", "boolean"),
-        schema("keyword_value", "string"),
         schema("text_value", "string"),
-        schema("binary_value", "binary"),
-        schema("date_value", "timestamp"),
         schema("date_nanos_value", "timestamp"),
+        schema("date_value", "timestamp"),
+        schema("boolean_value", "boolean"),
         schema("ip_value", "ip"),
-        schema("object_value", "struct"),
         schema("nested_value", "array"),
-        schema("geo_point_value", "geo_point"));
+        schema("object_value", "struct"),
+        schema("keyword_value", "string"),
+        schema("geo_point_value", "geo_point"),
+        schema("binary_value", "binary"));
+    verifyDataRowsInOrder(
+        result,
+        rows(
+            "text",
+            "2019-03-24 01:34:46.123456789",
+            "2020-10-13 13:00:00",
+            true,
+            "127.0.0.1",
+            new JSONArray(
+                "[{\"last\": \"Smith\", \"first\": \"John\"}, {\"last\": \"White\", \"first\":"
+                    + " \"Alice\"}]"),
+            new JSONObject("{\"last\": \"Dale\", \"first\": \"Dale\"}"),
+            "keyword",
+            new JSONObject("{\"lon\": 74, \"lat\": 40.71}"),
+            "U29tZSBiaW5hcnkgYmxvYg=="));
   }
 
   @Test

integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java

@@ -12,9 +12,11 @@
 import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
 
 import java.io.IOException;
+import java.util.stream.Collectors;
 import org.hamcrest.MatcherAssert;
 import org.json.JSONObject;
 import org.junit.jupiter.api.Test;
+import org.opensearch.sql.data.type.ExprCoreType;
 
 public class WhereCommandIT extends PPLIntegTestCase {
 
@@ -175,8 +177,11 @@ public void testInWithIncompatibleType() {
   }
 
   protected String getIncompatibleTypeErrMsg() {
-    return "function expected"
-               + " {[BYTE,BYTE],[SHORT,SHORT],[INTEGER,INTEGER],[LONG,LONG],[FLOAT,FLOAT],[DOUBLE,DOUBLE],[STRING,STRING],[BOOLEAN,BOOLEAN],[DATE,DATE],[TIME,TIME],[TIMESTAMP,TIMESTAMP],[INTERVAL,INTERVAL],[IP,IP],[GEO_POINT,GEO_POINT],[STRUCT,STRUCT],[ARRAY,ARRAY]},"
-               + " but got [LONG,STRING]";
+    return String.format(
+        "function expected %s, but got %s",
+        ExprCoreType.coreTypes().stream()
+            .map(type -> String.format("[%s,%s]", type.typeName(), type.typeName()))
+            .collect(Collectors.joining(",", "{", "}")),
+        "[LONG,STRING]");
   }
 }