Merge remote-tracking branch 'upstream/main' into issues/3418

LantaoJin · LantaoJin · commit caafbf2ba0a4 · 2025-03-27T10:44:54.000+08:00
diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java
@@ -15,10 +15,12 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 import org.apache.calcite.plan.RelOptTable;
 import org.apache.calcite.plan.ViewExpanders;
@@ -145,6 +147,7 @@ public RelNode visitProject(Project node, CalcitePlanContext context) {
     visitChildren(node, context);
     List<RexNode> projectList;
     if (node.getProjectList().stream().anyMatch(e -> e instanceof AllFields)) {
+      tryToRemoveNestedFields(context);
       return context.relBuilder.peek();
     } else {
       projectList =
@@ -160,6 +163,23 @@ public RelNode visitProject(Project node, CalcitePlanContext context) {
     return context.relBuilder.peek();
   }
 
+  /** See logic in {@link org.opensearch.sql.analysis.symbol.SymbolTable#lookupAllFields} */
+  private void tryToRemoveNestedFields(CalcitePlanContext context) {
+    Set<String> allFields = new HashSet<>(context.relBuilder.peek().getRowType().getFieldNames());
+    List<RexNode> duplicatedNestedFields =
+        allFields.stream()
+            .filter(
+                field -> {
+                  int lastDot = field.lastIndexOf(".");
+                  return -1 != lastDot && allFields.contains(field.substring(0, lastDot));
+                })
+            .map(field -> (RexNode) context.relBuilder.field(field))
+            .toList();
+    if (!duplicatedNestedFields.isEmpty()) {
+      context.relBuilder.projectExcept(duplicatedNestedFields);
+    }
+  }
+
   @Override
   public RelNode visitRename(Rename node, CalcitePlanContext context) {
     visitChildren(node, context);
diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java b/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java
@@ -236,6 +236,7 @@ public RexNode visitQualifiedName(QualifiedName node, CalcitePlanContext context
       }
     }
 
+    // TODO: Need to support nested fields https://github.com/opensearch-project/sql/issues/3459
     // 2. resolve QualifiedName in non-join condition
     String qualifiedName = node.toString();
     List<String> currentFields = context.relBuilder.peek().getRowType().getFieldNames();
diff --git a/core/src/main/java/org/opensearch/sql/calcite/udf/textUDF/LocateFunction.java b/core/src/main/java/org/opensearch/sql/calcite/udf/textUDF/LocateFunction.java
@@ -14,8 +14,8 @@ public Object eval(Object... args) {
       return new IllegalArgumentException(
           "Invalid number of arguments, locate function expects 2 or 3 arguments");
     }
-    String stringText = (String) args[0];
-    String targetText = (String) args[1];
+    String stringText = (String) args[1];
+    String targetText = (String) args[0];
     if (stringText == null || targetText == null) {
       return null;
     }
diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/BuiltinFunctionUtils.java b/core/src/main/java/org/opensearch/sql/calcite/utils/BuiltinFunctionUtils.java
@@ -249,6 +249,9 @@ static List<RexNode> translateArgument(
                     context.rexBuilder.makeLiteral(" ")));
         RTrimArgs.addAll(argList);
         return RTrimArgs;
+      case "STRCMP":
+        List<RexNode> StrcmpArgs = List.of(argList.get(1), argList.get(0));
+        return StrcmpArgs;
       case "ATAN":
         List<RexNode> AtanArgs = new ArrayList<>(argList);
         if (AtanArgs.size() == 1) {
diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/OpenSearchTypeFactory.java b/core/src/main/java/org/opensearch/sql/calcite/utils/OpenSearchTypeFactory.java
@@ -105,6 +105,8 @@ public static RelDataType convertExprTypeToRelDataType(ExprType fieldType, boole
           return TYPE_FACTORY.createArrayType(
               TYPE_FACTORY.createSqlType(SqlTypeName.ANY, nullable), -1);
         case STRUCT:
+          // TODO: should use RelRecordType instead of MapSqlType here
+          // https://github.com/opensearch-project/sql/issues/3459
           final RelDataType relKey = TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR);
           return TYPE_FACTORY.createMapType(
               relKey, TYPE_FACTORY.createSqlType(SqlTypeName.BINARY), nullable);
diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprValueUtils.java b/core/src/main/java/org/opensearch/sql/data/model/ExprValueUtils.java
@@ -5,6 +5,8 @@
 
 package org.opensearch.sql.data.model;
 
+import static org.opensearch.sql.utils.ExpressionUtils.PATH_SEP;
+
 import inet.ipaddr.IPAddress;
 import java.sql.Date;
 import java.sql.Time;
@@ -215,4 +217,18 @@ public static IPAddress getIpValue(ExprValue exprValue) {
   public static Boolean getBooleanValue(ExprValue exprValue) {
     return exprValue.booleanValue();
   }
+
+  public static ExprValue resolveRefPaths(ExprValue value, List<String> paths) {
+    ExprValue wholePathValue = value.keyValue(String.join(PATH_SEP, paths));
+    // For array types only first index currently supported.
+    if (value.type().equals(ExprCoreType.ARRAY)) {
+      wholePathValue = value.collectionValue().getFirst().keyValue(paths.getFirst());
+    }
+
+    if (!wholePathValue.isMissing() || paths.size() == 1) {
+      return wholePathValue;
+    } else {
+      return resolveRefPaths(value.keyValue(paths.getFirst()), paths.subList(1, paths.size()));
+    }
+  }
 }
diff --git a/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java b/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java
@@ -5,16 +5,14 @@
 
 package org.opensearch.sql.expression;
 
-import static org.opensearch.sql.utils.ExpressionUtils.PATH_SEP;
-
 import java.util.Arrays;
 import java.util.List;
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.RequiredArgsConstructor;
 import org.opensearch.sql.data.model.ExprTupleValue;
 import org.opensearch.sql.data.model.ExprValue;
-import org.opensearch.sql.data.type.ExprCoreType;
+import org.opensearch.sql.data.model.ExprValueUtils;
 import org.opensearch.sql.data.type.ExprType;
 import org.opensearch.sql.expression.env.Environment;
 
@@ -102,20 +100,6 @@ public String toString() {
    * </pre>
    */
   public ExprValue resolve(ExprTupleValue value) {
-    return resolve(value, paths);
-  }
-
-  private ExprValue resolve(ExprValue value, List<String> paths) {
-    ExprValue wholePathValue = value.keyValue(String.join(PATH_SEP, paths));
-    // For array types only first index currently supported.
-    if (value.type().equals(ExprCoreType.ARRAY)) {
-      wholePathValue = value.collectionValue().get(0).keyValue(paths.get(0));
-    }
-
-    if (!wholePathValue.isMissing() || paths.size() == 1) {
-      return wholePathValue;
-    } else {
-      return resolve(value.keyValue(paths.get(0)), paths.subList(1, paths.size()));
-    }
+    return ExprValueUtils.resolveRefPaths(value, paths);
   }
 }
diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/fallback/CalciteObjectFieldOperateIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/fallback/CalciteObjectFieldOperateIT.java
@@ -5,10 +5,8 @@
 
 package org.opensearch.sql.calcite.remote.fallback;
 
-import org.junit.Ignore;
 import org.opensearch.sql.ppl.ObjectFieldOperateIT;
 
-@Ignore("https://github.com/opensearch-project/sql/issues/3452")
 public class CalciteObjectFieldOperateIT extends ObjectFieldOperateIT {
   @Override
   public void init() throws Exception {
diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/nonfallback/NonFallbackCalciteObjectFieldOperateIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/nonfallback/NonFallbackCalciteObjectFieldOperateIT.java
@@ -5,10 +5,8 @@
 
 package org.opensearch.sql.calcite.remote.nonfallback;
 
-import org.junit.Ignore;
 import org.opensearch.sql.calcite.remote.fallback.CalciteObjectFieldOperateIT;
 
-@Ignore("https://github.com/opensearch-project/sql/issues/3452")
 public class NonFallbackCalciteObjectFieldOperateIT extends CalciteObjectFieldOperateIT {
   @Override
   public void init() throws Exception {
diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/nonfallback/NonFallbackCalciteTextFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/nonfallback/NonFallbackCalciteTextFunctionIT.java
@@ -5,7 +5,6 @@
 
 package org.opensearch.sql.calcite.remote.nonfallback;
 
-import org.junit.Ignore;
 import org.opensearch.sql.calcite.remote.fallback.CalciteTextFunctionIT;
 
 public class NonFallbackCalciteTextFunctionIT extends CalciteTextFunctionIT {
@@ -14,28 +13,4 @@ public void init() throws Exception {
     super.init();
     disallowCalciteFallback();
   }
-
-  @Ignore("https://github.com/opensearch-project/sql/issues/3467")
-  @Override
-  public void testAscii() {}
-
-  @Ignore("https://github.com/opensearch-project/sql/issues/3467")
-  @Override
-  public void testLeft() {}
-
-  @Ignore("https://github.com/opensearch-project/sql/issues/3467")
-  @Override
-  public void testLocate() {}
-
-  @Ignore("https://github.com/opensearch-project/sql/issues/3467")
-  @Override
-  public void testReplace() {}
-
-  @Ignore("https://github.com/opensearch-project/sql/issues/3467")
-  @Override
-  public void testStrcmp() {}
-
-  @Ignore("https://github.com/opensearch-project/sql/issues/3467")
-  @Override
-  public void testSubstr() {}
 }
diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/standalone/CalcitePPLStringBuiltinFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/standalone/CalcitePPLStringBuiltinFunctionIT.java
@@ -157,7 +157,7 @@ public void testLocate() {
     JSONObject actual =
         executeQuery(
             String.format(
-                "source=%s | where locate(name, 'Ja')=1 | fields name, age",
+                "source=%s | where locate('Ja', name)=1 | fields name, age",
                 TEST_INDEX_STATE_COUNTRY_WITH_NULL));
 
     verifySchema(actual, schema("name", "string"), schema("age", "integer"));
diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/ObjectFieldOperateIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/ObjectFieldOperateIT.java
@@ -52,7 +52,10 @@ public void group_object_field_in_stats() throws IOException {
     JSONObject result =
         executeQuery(
             String.format("source=%s | stats count() by city.name", TEST_INDEX_DEEP_NESTED));
-    verifySchema(result, schema("count()", "int"), schema("city.name", "string"));
+    verifySchema(
+        result,
+        schema("count()", isCalciteEnabled() ? "bigint" : "int"),
+        schema("city.name", "string"));
     verifyDataRows(result, rows(1, "Seattle"));
   }
 
@@ -66,4 +69,16 @@ public void sort_by_object_field() throws IOException {
     verifySchema(result, schema("city.name", "string"), schema("city.location.latitude", "double"));
     verifyDataRows(result, rows("Seattle", 10.5));
   }
+
+  @Test
+  public void verify_schema_without_fields() throws IOException {
+    JSONObject result =
+        executeQuery(String.format("source=%s | sort city.name ", TEST_INDEX_DEEP_NESTED));
+    verifySchema(
+        result,
+        schema("projects", "array"),
+        schema("accounts", "struct"),
+        schema("city", "struct"),
+        schema("account", "struct"));
+  }
 }
diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexEnumerator.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexEnumerator.java
@@ -11,8 +11,8 @@
 import lombok.EqualsAndHashCode;
 import lombok.ToString;
 import org.apache.calcite.linq4j.Enumerator;
-import org.opensearch.sql.data.model.ExprNullValue;
 import org.opensearch.sql.data.model.ExprValue;
+import org.opensearch.sql.data.model.ExprValueUtils;
 import org.opensearch.sql.opensearch.client.OpenSearchClient;
 import org.opensearch.sql.opensearch.request.OpenSearchRequest;
 import org.opensearch.sql.opensearch.response.OpenSearchResponse;
@@ -66,14 +66,13 @@ public Object current() {
      * See {@link PhysTypeImpl}
      */
     if (fields.size() == 1) {
-      return current
-          .tupleValue()
-          .getOrDefault(fields.getFirst(), ExprNullValue.of())
-          .valueForCalcite();
+      return resolveForCalcite(current, fields.getFirst());
     }
-    return fields.stream()
-        .map(k -> current.tupleValue().getOrDefault(k, ExprNullValue.of()).valueForCalcite())
-        .toArray();
+    return fields.stream().map(field -> resolveForCalcite(current, field)).toArray();
+  }
+
+  private Object resolveForCalcite(ExprValue value, String rawPath) {
+    return ExprValueUtils.resolveRefPaths(value, List.of(rawPath.split("\\."))).valueForCalcite();
   }
 
   @Override

Original file line number	Diff line number	Diff line change
`@@ -236,6 +236,7 @@ public RexNode visitQualifiedName(QualifiedName node, CalcitePlanContext context`
`236`	`236`	`}`
`237`	`237`	`}`
`238`	`238`
	`239`	`+ // TODO: Need to support nested fields https://github.com/opensearch-project/sql/issues/3459`
`239`	`240`	`// 2. resolve QualifiedName in non-join condition`
`240`	`241`	`String qualifiedName = node.toString();`
`241`	`242`	`List<String> currentFields = context.relBuilder.peek().getRowType().getFieldNames();`
Original file line number	Diff line number	Diff line change
`@@ -14,8 +14,8 @@ public Object eval(Object... args) {`
`14`	`14`	`return new IllegalArgumentException(`
`15`	`15`	`"Invalid number of arguments, locate function expects 2 or 3 arguments");`
`16`	`16`	`}`
`17`		`- String stringText = (String) args[0];`
`18`		`- String targetText = (String) args[1];`
	`17`	`+ String stringText = (String) args[1];`
	`18`	`+ String targetText = (String) args[0];`
`19`	`19`	`if (stringText == null \|\| targetText == null) {`
`20`	`20`	`return null;`
`21`	`21`	`}`