Additional review comments, including move constants to `ExprValueUti…

…ls`.

Signed-off-by: currantw <[email protected]>

core/src/main/java/org/opensearch/sql/analysis/Analyzer.java

@@ -21,7 +21,6 @@
 import static org.opensearch.sql.utils.MLCommonsConstants.RCF_ANOMALY_GRADE;
 import static org.opensearch.sql.utils.MLCommonsConstants.RCF_SCORE;
 import static org.opensearch.sql.utils.MLCommonsConstants.TIME_FIELD;
-import static org.opensearch.sql.utils.PathUtils.SEPARATOR;
 import static org.opensearch.sql.utils.SystemIndexUtils.DATASOURCES_TABLE_NAME;
 
 import com.google.common.collect.ImmutableList;
@@ -77,6 +76,7 @@
 import org.opensearch.sql.common.antlr.SyntaxCheckException;
 import org.opensearch.sql.common.utils.StringUtils;
 import org.opensearch.sql.data.model.ExprMissingValue;
+import org.opensearch.sql.data.model.ExprValueUtils;
 import org.opensearch.sql.data.type.ExprCoreType;
 import org.opensearch.sql.data.type.ExprType;
 import org.opensearch.sql.datasource.DataSourceService;
@@ -511,12 +511,13 @@ public LogicalPlan visitFlatten(Flatten node, AnalysisContext context) {
     Map<String, ExprType> fieldsMap = env.lookupAllTupleFields(FIELD_NAME);
 
     final String fieldParentPathPrefix =
-        fieldName.contains(SEPARATOR)
-            ? fieldName.substring(0, fieldName.lastIndexOf(SEPARATOR)) + SEPARATOR
+        fieldName.contains(ExprValueUtils.QUALIFIED_NAME_SEPARATOR)
+            ? fieldName.substring(0, fieldName.lastIndexOf(ExprValueUtils.QUALIFIED_NAME_SEPARATOR))
+                + ExprValueUtils.QUALIFIED_NAME_SEPARATOR
             : "";
 
     // Get entries for paths that are descended from the flattened field.
-    final String fieldDescendantPathPrefix = fieldName + SEPARATOR;
+    final String fieldDescendantPathPrefix = fieldName + ExprValueUtils.QUALIFIED_NAME_SEPARATOR;
     List<Map.Entry<String, ExprType>> fieldDescendantEntries =
         fieldsMap.entrySet().stream()
             .filter(e -> e.getKey().startsWith(fieldDescendantPathPrefix))
@@ -525,14 +526,8 @@ public LogicalPlan visitFlatten(Flatten node, AnalysisContext context) {
     // Get fields to add from descendant entries.
     Map<String, ExprType> addFieldsMap = new HashMap<>();
     for (Map.Entry<String, ExprType> entry : fieldDescendantEntries) {
-      String path = entry.getKey();
-
-      // Build the new path.
-      String newPath = path.substring(fieldDescendantPathPrefix.length());
-      if (!fieldParentPathPrefix.isEmpty()) {
-        newPath = fieldParentPathPrefix + newPath;
-      }
-
+      String newPath =
+          fieldParentPathPrefix + entry.getKey().substring(fieldDescendantPathPrefix.length());
       addFieldsMap.put(newPath, entry.getValue());
     }
 

core/src/main/java/org/opensearch/sql/analysis/DataSourceSchemaIdentifierNameResolver.java

@@ -7,9 +7,8 @@
 
 package org.opensearch.sql.analysis;
 
-import static org.opensearch.sql.utils.PathUtils.SEPARATOR;
-
 import java.util.List;
+import org.opensearch.sql.data.model.ExprValueUtils;
 import org.opensearch.sql.datasource.DataSourceService;
 
 public class DataSourceSchemaIdentifierNameResolver {
@@ -35,7 +34,7 @@ public DataSourceSchemaIdentifierNameResolver(
       DataSourceService dataSourceService, List<String> parts) {
     this.dataSourceService = dataSourceService;
     List<String> remainingParts = captureSchemaName(captureDataSourceName(parts));
-    identifierName = String.join(SEPARATOR, remainingParts);
+    identifierName = String.join(ExprValueUtils.QUALIFIED_NAME_SEPARATOR, remainingParts);
   }
 
   public String getIdentifierName() {

core/src/main/java/org/opensearch/sql/data/model/ExprValueUtils.java

@@ -16,18 +16,28 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Pattern;
 import lombok.experimental.UtilityClass;
 import org.opensearch.sql.data.type.ExprCoreType;
 import org.opensearch.sql.exception.ExpressionEvaluationException;
 
 /** The definition of {@link ExprValue} factory. */
 @UtilityClass
 public class ExprValueUtils {
+
+  // Literal constants
   public static final ExprValue LITERAL_TRUE = ExprBooleanValue.of(true);
   public static final ExprValue LITERAL_FALSE = ExprBooleanValue.of(false);
   public static final ExprValue LITERAL_NULL = ExprNullValue.of();
   public static final ExprValue LITERAL_MISSING = ExprMissingValue.of();
 
+  /** Qualified name separator string */
+  public final String QUALIFIED_NAME_SEPARATOR = ".";
+
+  /** Pattern that matches the qualified name separator string */
+  public final Pattern QUALIFIED_NAME_SEPARATOR_PATTERN =
+      Pattern.compile(QUALIFIED_NAME_SEPARATOR, Pattern.LITERAL);
+
   public static ExprValue booleanValue(Boolean value) {
     return value ? LITERAL_TRUE : LITERAL_FALSE;
   }

core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java

@@ -5,15 +5,14 @@
 
 package org.opensearch.sql.expression;
 
-import static org.opensearch.sql.utils.PathUtils.SEPARATOR;
-
 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.model.ExprValueUtils;
 import org.opensearch.sql.data.type.ExprCoreType;
 import org.opensearch.sql.data.type.ExprType;
 import org.opensearch.sql.expression.env.Environment;
@@ -106,7 +105,8 @@ public ExprValue resolve(ExprTupleValue value) {
   }
 
   private ExprValue resolve(ExprValue value, List<String> paths) {
-    ExprValue wholePathValue = value.keyValue(String.join(SEPARATOR, paths));
+    ExprValue wholePathValue =
+        value.keyValue(String.join(ExprValueUtils.QUALIFIED_NAME_SEPARATOR, paths));
     // For array types only first index currently supported.
     if (value.type().equals(ExprCoreType.ARRAY)) {
       wholePathValue = value.collectionValue().get(0).keyValue(paths.get(0));

core/src/main/java/org/opensearch/sql/planner/physical/FlattenOperator.java

@@ -5,8 +5,6 @@
 
 package org.opensearch.sql.planner.physical;
 
-import static org.opensearch.sql.utils.PathUtils.SEPARATOR_PATTERN;
-
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -53,11 +51,11 @@ public ExprValue next() {
   /**
    * Flattens the {@link ExprTupleValue} at the specified path within the given root value and
    * returns the result. Returns the unmodified root value if it does not contain a value at the
-   * specified path.
+   * specified path. rootExprValue is expected to be an {@link ExprTupleValue}.
    */
   private static ExprValue flattenExprValueAtPath(ExprValue rootExprValue, String path) {
 
-    Matcher matcher = SEPARATOR_PATTERN.matcher(path);
+    Matcher matcher = ExprValueUtils.QUALIFIED_NAME_SEPARATOR_PATTERN.matcher(path);
     Map<String, ExprValue> exprValueMap = ExprValueUtils.getTupleValue(rootExprValue);
 
     // [A] Flatten nested struct value