Address comments

Signed-off-by: Tomoyuki Morita <moritato@amazon.com>

Author: ykmr1224

core/src/main/java/org/opensearch/sql/ast/expression/QualifiedName.java

@@ -22,6 +22,7 @@
 @Getter
 @EqualsAndHashCode(callSuper = false)
 public class QualifiedName extends UnresolvedExpression {
+  public static final String DELIMITER = ".";
   private final List<String> parts;
 
   public QualifiedName(String name) {
@@ -94,7 +95,7 @@ public QualifiedName rest() {
   }
 
   public String toString() {
-    return String.join(".", this.parts);
+    return String.join(DELIMITER, this.parts);
   }
 
   @Override
@@ -106,4 +107,20 @@ public List<UnresolvedExpression> getChild() {
   public <R, C> R accept(AbstractNodeVisitor<R, C> nodeVisitor, C context) {
     return nodeVisitor.visitQualifiedName(this, context);
   }
+
+  public String sub(int fromIndex, int toIndex) {
+    return String.join(DELIMITER, parts.subList(fromIndex, toIndex));
+  }
+
+  public String sub(int fromIndex) {
+    return String.join(DELIMITER, parts.subList(fromIndex, parts.size()));
+  }
+
+  public String getPart(int at) {
+    return parts.get(at);
+  }
+
+  public int getPartsCount() {
+    return parts.size();
+  }
 }

core/src/main/java/org/opensearch/sql/calcite/ExtendedRexBuilder.java

@@ -197,7 +197,10 @@ public RexNode castToTargetType(RexNode node, RexNode target) {
     return makeCast(target.getType(), node, true, true);
   }
 
-  /** Utility to cast ANY to specific types to avoid compare issue */
+  /**
+   * Utility to cast ANY to specific types to avoid compare issue in Calcite:
+   * https://issues.apache.org/jira/browse/CALCITE-7206
+   */
   RexNode castAnyToAlignTypes(RexNode rexNode, CalcitePlanContext context) {
     return rexNode.accept(
         new RexConverter() {

core/src/main/java/org/opensearch/sql/calcite/rel/QualifiedNameResolver.java

@@ -109,43 +109,24 @@ private static RexNode resolveInNonJoinCondition(
         .orElseThrow(() -> getNotFoundException(nameNode));
   }
 
-  private static String joinParts(List<String> parts, int start, int length) {
-    StringBuilder sb = new StringBuilder();
-    for (int i = 0; i < length; i++) {
-      if (start < i) {
-        sb.append(".");
-      }
-      sb.append(parts.get(start + i));
-    }
-    return sb.toString();
-  }
-
-  private static String joinParts(List<String> parts, int start) {
-    return joinParts(parts, start, parts.size() - start);
-  }
-
   private static Optional<RexNode> resolveFieldWithAlias(
       QualifiedName nameNode, CalcitePlanContext context, int inputCount) {
-    List<String> parts = nameNode.getParts();
     log.debug(
-        "resolveFieldWithAlias() called with nameNode={}, parts={}, inputCount={}",
-        nameNode,
-        parts,
-        inputCount);
+        "resolveFieldWithAlias() called with nameNode={}, inputCount={}", nameNode, inputCount);
 
-    if (parts.size() >= 2) {
+    if (nameNode.getPartsCount() >= 2) {
       // Consider first part as table alias
-      String alias = parts.get(0);
+      String alias = nameNode.getPart(0);
       log.debug("resolveFieldWithAlias() trying alias={}", alias);
 
       // Try to resolve the longest match first
-      for (int length = parts.size() - 1; 1 <= length; length--) {
-        String field = joinParts(parts, 1, length);
+      for (int length = nameNode.getPartsCount() - 1; 1 <= length; length--) {
+        String field = nameNode.sub(1, length);
         log.debug("resolveFieldWithAlias() trying field={} with length={}", field, length);
 
         Optional<RexNode> fieldNode = tryToResolveField(alias, field, context, inputCount);
         if (fieldNode.isPresent()) {
-          return Optional.of(resolveFieldAccess(context, parts, 1, length, fieldNode.get()));
+          return Optional.of(resolveFieldAccess(context, nameNode, 1, length, fieldNode.get()));
         }
       }
     }
@@ -154,18 +135,16 @@ private static Optional<RexNode> resolveFieldWithAlias(
 
   private static Optional<RexNode> resolveDynamicFieldsWithAlias(
       QualifiedName nameNode, CalcitePlanContext context, int inputCount) {
-    List<String> parts = nameNode.getParts();
     log.debug(
-        "resolveDynamicFieldsWithAlias() called with nameNode={}, parts={}, inputCount={}",
+        "resolveDynamicFieldsWithAlias() called with nameNode={}, inputCount={}",
         nameNode,
-        parts,
         inputCount);
 
-    if (parts.size() >= 2) {
+    if (nameNode.getPartsCount() >= 2) {
       // Consider first part as table alias
-      String alias = parts.get(0);
+      String alias = nameNode.getPart(0);
 
-      String fieldName = String.join(".", parts.subList(1, parts.size()));
+      String fieldName = nameNode.sub(1);
       Optional<RexNode> dynamicField =
           tryToResolveField(alias, DYNAMIC_FIELDS_MAP, context, inputCount);
       return dynamicField.map(field -> createItemAccess(field, fieldName, context));
@@ -176,18 +155,14 @@ private static Optional<RexNode> resolveDynamicFieldsWithAlias(
 
   private static Optional<RexNode> resolveDynamicFields(
       QualifiedName nameNode, CalcitePlanContext context, int inputCount) {
-    List<String> parts = nameNode.getParts();
     log.debug(
-        "resolveDynamicFields() called with nameNode={}, parts={}, inputCount={}",
-        nameNode,
-        parts,
-        inputCount);
+        "resolveDynamicFields() called with nameNode={}, inputCount={}", nameNode, inputCount);
 
     List<Set<String>> inputFieldNames = collectInputFieldNames(context, inputCount);
 
     for (int i = 0; i < inputCount; i++) {
       if (inputFieldNames.get(i).contains(DYNAMIC_FIELDS_MAP)) {
-        String fieldName = String.join(".", parts);
+        String fieldName = nameNode.toString();
         RexNode dynamicField = context.relBuilder.field_(inputCount, i, DYNAMIC_FIELDS_MAP);
         RexNode itemAccess = createItemAccess(dynamicField, fieldName, context);
         return Optional.of(itemAccess);
@@ -218,16 +193,15 @@ private static Optional<RexNode> resolveFieldWithoutAlias(
 
     List<Set<String>> inputFieldNames = collectInputFieldNames(context, inputCount);
 
-    List<String> parts = nameNode.getParts();
-    for (int length = parts.size(); 1 <= length; length--) {
-      String fieldName = joinParts(parts, 0, length);
+    for (int length = nameNode.getPartsCount(); 1 <= length; length--) {
+      String fieldName = nameNode.sub(0, length);
       log.debug("resolveFieldWithoutAlias() trying fieldName={} with length={}", fieldName, length);
 
       int foundInput = findInputContainingFieldName(inputCount, inputFieldNames, fieldName);
       log.debug("resolveFieldWithoutAlias() foundInput={}", foundInput);
       if (foundInput != -1) {
         RexNode fieldNode = context.relBuilder.field_(inputCount, foundInput, fieldName);
-        return Optional.of(resolveFieldAccess(context, parts, 0, length, fieldNode));
+        return Optional.of(resolveFieldAccess(context, nameNode, 0, length, fieldNode));
       }
     }
     return Optional.empty();
@@ -268,10 +242,9 @@ private static Optional<RexNode> resolveRenamedField(
       QualifiedName nameNode, CalcitePlanContext context) {
     log.debug("resolveRenamedField() called with nameNode={}", nameNode);
 
-    List<String> parts = nameNode.getParts();
-    if (parts.size() >= 2) {
+    if (nameNode.getPartsCount() >= 2) {
       List<String> candidates = findCandidatesByRenamedFieldName(nameNode, context);
-      String alias = parts.get(0);
+      String alias = nameNode.getPart(0);
       for (String candidate : candidates) {
         try {
           return Optional.of(context.relBuilder.field_(alias, candidate));
@@ -289,20 +262,19 @@ private static Optional<RexNode> resolveRenamedField(
    */
   private static List<String> findCandidatesByRenamedFieldName(
       QualifiedName renamedFieldName, CalcitePlanContext context) {
-    String originalFieldName = joinParts(renamedFieldName.getParts(), 1);
+    String originalFieldName = renamedFieldName.sub(1);
     return context.relBuilder.peek().getRowType().getFieldNames().stream()
         .filter(col -> getNameBeforeRename(col).equals(originalFieldName))
         .toList();
   }
 
   private static String getNameBeforeRename(String fieldName) {
-    return fieldName.substring(fieldName.indexOf(".") + 1);
+    return fieldName.substring(fieldName.indexOf(QualifiedName.DELIMITER) + 1);
   }
 
   private static Optional<RexNode> resolveCorrelationField(
       QualifiedName nameNode, CalcitePlanContext context) {
     log.debug("resolveCorrelationField() called with nameNode={}", nameNode);
-    List<String> parts = nameNode.getParts();
     return context
         .peekCorrelVar()
         .map(
@@ -311,12 +283,12 @@ private static Optional<RexNode> resolveCorrelationField(
               // Try full match, then consider first part as table alias
               for (int start = 0; start <= 1; start++) {
                 // Try to resolve the longest match first
-                for (int length = parts.size() - start; 1 <= length; length--) {
-                  String fieldName = joinParts(parts, start, length);
+                for (int length = nameNode.getPartsCount() - start; 1 <= length; length--) {
+                  String fieldName = nameNode.sub(start, length);
                   log.debug("resolveCorrelationField() trying fieldName={}", fieldName);
                   if (fieldNameList.contains(fieldName)) {
                     RexNode field = context.relBuilder.field_(correlation, fieldName);
-                    return resolveFieldAccess(context, parts, start, length, field);
+                    return resolveFieldAccess(context, nameNode, start, length, field);
                   }
                 }
               }
@@ -325,11 +297,11 @@ private static Optional<RexNode> resolveCorrelationField(
   }
 
   private static RexNode resolveFieldAccess(
-      CalcitePlanContext context, List<String> parts, int start, int length, RexNode field) {
-    if (length == parts.size() - start) {
+      CalcitePlanContext context, QualifiedName name, int start, int length, RexNode field) {
+    if (length == name.getPartsCount() - start) {
       return field;
     } else {
-      String itemName = joinParts(parts, length + start, parts.size() - 1 - length);
+      String itemName = name.sub(length + start, name.getPartsCount() - 1 - length);
       return createItemAccess(field, itemName, context);
     }
   }