Code clean up and comment update.

Signed-off-by: Yury-Fridlyand <[email protected]>

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

@@ -50,7 +50,7 @@ default boolean shouldCast(ExprType other) {
    * Get the parent type.
    */
   default List<ExprType> getParent() {
-    return Arrays.asList(UNKNOWN);
+    return List.of(UNKNOWN);
   }
 
   /**
@@ -65,12 +65,16 @@ default String legacyTypeName() {
     return typeName();
   }
 
-  // TODO doc
+  /**
+   * Perform field name conversion if needed before inserting it into a search query.
+   */
   default String convertFieldForSearchQuery(String fieldName) {
     return fieldName;
   }
 
-  // TODO doc
+  /**
+   * Perform value conversion if needed before inserting it into a search query.
+   */
   default Object convertValueForSearchQuery(ExprValue value) {
     return value.value();
   }

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

@@ -13,15 +13,8 @@
 
 /**
  * The definition of widening type rule for expression value.
- * ExprType     Widens to data types
- * INTEGER      LONG, FLOAT, DOUBLE
- * LONG         FLOAT, DOUBLE
- * FLOAT        DOUBLE
- * DOUBLE       DOUBLE
- * STRING       STRING
- * BOOLEAN      BOOLEAN
- * ARRAY        ARRAY
- * STRUCT       STRUCT
+ * See type widening definitions in {@link ExprCoreType}.
+ * For example, SHORT widens BYTE and so on.
  */
 @UtilityClass
 public class WideningTypeRule {
@@ -53,9 +46,9 @@ private static int distance(ExprType type1, ExprType type2, int distance) {
   }
 
   /**
-   * The max type among two types. The max is defined as follow
+   * The max type among two types. The max is defined as follows:
    * if type1 could widen to type2, then max is type2, vice versa
-   * if type1 could't widen to type2 and type2 could't widen to type1,
+   * if type1 couldn't widen to type2 and type2 couldn't widen to type1,
    * then throw {@link ExpressionEvaluationException}.
    *
    * @param type1 type1

docs/dev/text-type.md

@@ -36,3 +36,10 @@ The solution is to provide to `:core` non simplified types, but full types. Thos
 2. Update `OpenSearchDataType` (and inheritors if needed) to be comparable with `ExprCoreType`.
 3. Update `:core` to do proper comparison (use `.equals` instead of `==`).
 5. Update `:opensearch` to use the mapping information received from `:core` and properly build the search query.
+
+## Type Schema
+
+| JDBC type | `ExprCoreType` | `OpenSearchDataType` | OpenSearch type |
+| --- | --- | --- | --- |
+| `VARCHAR`/`CHAR` | `STRING` | -- | `keyword` |
+| `LONGVARCHAR`/`TEXT` | `STRING` | `OpenSearchTextType` | `text` |

opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java

@@ -17,12 +17,17 @@
 import org.apache.commons.lang3.EnumUtils;
 import org.opensearch.sql.data.type.ExprCoreType;
 import org.opensearch.sql.data.type.ExprType;
+import org.opensearch.sql.data.type.WideningTypeRule;
 
 /**
  * The extension of ExprType in OpenSearch.
  */
 public class OpenSearchDataType implements ExprType, Serializable {
 
+  /**
+   * Redefine comparison operation: class (or derived) could be compared with ExprCoreType too.
+   * Used in {@link WideningTypeRule#distance(ExprType, ExprType)}.
+   */
   public boolean equals(final Object o) {
     if (o == this) {
       return true;
@@ -46,17 +51,17 @@ public int hashCode() {
 
   @Override
   public List<ExprType> getParent() {
-    return exprCoreType == ExprCoreType.UNKNOWN
-        ? List.of(ExprCoreType.UNKNOWN)
-        : exprCoreType.getParent();
+    return exprCoreType.getParent();
   }
 
   @Override
   public boolean shouldCast(ExprType other) {
-    ExprCoreType otherCoreType = other instanceof ExprCoreType ? (ExprCoreType) other
-        : (other instanceof OpenSearchDataType
-            ? ((OpenSearchDataType) other).exprCoreType : ExprCoreType.UNKNOWN);
-    // TODO Copied from BuiltinFunctionRepository.isCastRequired
+    ExprCoreType otherCoreType = ExprCoreType.UNKNOWN;
+    if (other instanceof ExprCoreType) {
+      otherCoreType = (ExprCoreType) other;
+    } else if (other instanceof OpenSearchDataType) {
+      otherCoreType = ((OpenSearchDataType) other).exprCoreType;
+    }
     if (ExprCoreType.numberTypes().contains(exprCoreType)
         && ExprCoreType.numberTypes().contains(otherCoreType)) {
       return false;
@@ -163,9 +168,6 @@ public static Map<String, OpenSearchDataType> parseMapping(Map<String, Object> i
         return;
       }
       // create OpenSearchDataType
-
-      // TODO parse `fielddata`
-
       result.put(k, OpenSearchDataType.of(
           EnumUtils.getEnumIgnoreCase(OpenSearchDataType.MappingType.class, type),
           innerMap)
@@ -196,7 +198,7 @@ public static OpenSearchDataType of(MappingType mappingType, Map<String, Object>
         objectDataType.properties = properties;
         return objectDataType;
       case Text:
-        // TODO update these 2 below #1038 https://github.com/opensearch-project/sql/issues/1038
+        // don't parse `fielddata`, because it does not contain any info which could be used by SQL
         Map<String, OpenSearchDataType> fields =
             parseMapping((Map<String, Object>) innerMap.getOrDefault("fields", Map.of()));
         return (!fields.isEmpty()) ? OpenSearchTextType.of(fields) : OpenSearchTextType.of();

opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDateType.java

@@ -416,7 +416,7 @@ public String legacyTypeName() {
 
   @Override
   public Object convertValueForSearchQuery(ExprValue value) {
-    // TODO fix for https://github.com/opensearch-project/sql/issues/1847
+    // TODO add here fix for https://github.com/opensearch-project/sql/issues/1847
     return value.timestampValue().toEpochMilli();
   }
 }

opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchTextType.java

@@ -63,15 +63,11 @@ protected OpenSearchDataType cloneEmpty() {
 
   @Override
   public String convertFieldForSearchQuery(String fieldName) {
-    if (fields.size() > 1) {
-      // TODO or pick first?
-      throw new RuntimeException("too many text fields");
-    }
     if (fields.size() == 0) {
       return fieldName;
     }
-    // TODO what if field is not a keyword
-    // https://github.com/opensearch-project/sql/issues/1112
+    // Pick first field. What to do if there are multiple fields?
+    // Multi-field text support requested in https://github.com/opensearch-project/sql/issues/1887
     return String.format("%s.%s", fieldName, fields.keySet().toArray()[0]);
   }
 }

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java

@@ -127,7 +127,7 @@ public Map<String, ExprType> getFieldTypes() {
       cachedFieldTypes = OpenSearchDataType.traverseAndFlatten(cachedFieldOpenSearchTypes)
           .entrySet().stream().collect(
               LinkedHashMap::new,
-              (map, item) -> map.put(item.getKey(), item.getValue()/*.getExprType()*/),
+              (map, item) -> map.put(item.getKey(), item.getValue()),
               Map::putAll);
     }
     return cachedFieldTypes;

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/AggregationBuilderHelper.java

@@ -17,7 +17,6 @@
 import org.opensearch.sql.expression.FunctionExpression;
 import org.opensearch.sql.expression.LiteralExpression;
 import org.opensearch.sql.expression.ReferenceExpression;
-import org.opensearch.sql.opensearch.data.type.OpenSearchTextType;
 import org.opensearch.sql.opensearch.storage.serialization.ExpressionSerializer;
 
 /**

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/BucketAggregationBuilder.java

@@ -13,7 +13,6 @@
 import com.google.common.collect.ImmutableList;
 import java.util.List;
 import java.util.stream.Stream;
-
 import org.apache.commons.lang3.tuple.Triple;
 import org.opensearch.search.aggregations.bucket.composite.CompositeValuesSourceBuilder;
 import org.opensearch.search.aggregations.bucket.composite.DateHistogramValuesSourceBuilder;

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/core/ExpressionScript.java

@@ -29,7 +29,6 @@
 import org.opensearch.sql.expression.env.Environment;
 import org.opensearch.sql.expression.parse.ParseExpression;
 import org.opensearch.sql.opensearch.data.type.OpenSearchDataType;
-import org.opensearch.sql.opensearch.data.type.OpenSearchTextType;
 import org.opensearch.sql.opensearch.data.value.OpenSearchExprValueFactory;
 
 /**

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/LikeQuery.java

@@ -10,7 +10,6 @@
 import org.opensearch.index.query.WildcardQueryBuilder;
 import org.opensearch.sql.data.model.ExprValue;
 import org.opensearch.sql.data.type.ExprType;
-import org.opensearch.sql.opensearch.data.type.OpenSearchTextType;
 import org.opensearch.sql.opensearch.storage.script.StringUtils;
 
 public class LikeQuery extends LuceneQuery {

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/TermQuery.java

@@ -9,9 +9,7 @@
 import org.opensearch.index.query.QueryBuilder;
 import org.opensearch.index.query.QueryBuilders;
 import org.opensearch.sql.data.model.ExprValue;
-import org.opensearch.sql.data.type.ExprCoreType;
 import org.opensearch.sql.data.type.ExprType;
-import org.opensearch.sql.opensearch.data.type.OpenSearchTextType;
 
 /**
  * Lucene query that build term query for equality comparison.

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/sort/SortQueryBuilder.java

@@ -20,8 +20,6 @@
 import org.opensearch.sql.expression.Expression;
 import org.opensearch.sql.expression.FunctionExpression;
 import org.opensearch.sql.expression.ReferenceExpression;
-import org.opensearch.sql.expression.function.BuiltinFunctionName;
-import org.opensearch.sql.opensearch.data.type.OpenSearchTextType;
 
 /**
  * Builder of {@link SortBuilder}.