enhance error handling and doc

Signed-off-by: Kai Huang <ahkcs@amazon.com>

Author: ahkcs

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

@@ -338,12 +338,13 @@ public static boolean isUserDefinedType(RelDataType type) {
   }
 
   /**
-   * Checks if the RelDataType represents a numeric field. Supports both standard SQL numeric types
-   * (INTEGER, BIGINT, SMALLINT, TINYINT, FLOAT, DOUBLE, DECIMAL, REAL) and OpenSearch UDT numeric
-   * types.
+   * Checks if the RelDataType represents a numeric field or a string field that can be coerced to
+   * numeric. Supports standard SQL numeric types (INTEGER, BIGINT, SMALLINT, TINYINT, FLOAT,
+   * DOUBLE, DECIMAL, REAL), OpenSearch UDT numeric types, and string types (VARCHAR, CHAR) which
+   * can contain numeric values and will be automatically coerced.
    *
    * @param fieldType the RelDataType to check
-   * @return true if the type is numeric, false otherwise
+   * @return true if the type is numeric or a coercible string type, false otherwise
    */
   public static boolean isNumericType(RelDataType fieldType) {
     // Check standard SQL numeric types
@@ -359,6 +360,11 @@ public static boolean isNumericType(RelDataType fieldType) {
       return true;
     }
 
+    // Check string types (VARCHAR, CHAR) which can be coerced to numeric for binning
+    if (sqlType == SqlTypeName.VARCHAR || sqlType == SqlTypeName.CHAR) {
+      return true;
+    }
+
     // Check for OpenSearch UDT numeric types
     if (isUserDefinedType(fieldType)) {
       AbstractExprRelDataType<?> exprType = (AbstractExprRelDataType<?>) fieldType;

core/src/main/java/org/opensearch/sql/calcite/utils/binning/BinnableField.java

@@ -9,11 +9,9 @@
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rex.RexNode;
 import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory;
+import org.opensearch.sql.exception.SemanticCheckException;
 
-/**
- * Represents a field that supports binning operations. Supports numeric, time-based, and string
- * fields. Type coercion for string fields is handled automatically by Calcite's type system.
- */
+/** Represents a field that supports binning operations. */
 @Getter
 public class BinnableField {
   private final RexNode fieldExpr;
@@ -23,12 +21,12 @@ public class BinnableField {
   private final boolean isNumeric;
 
   /**
-   * Creates a BinnableField for binning operations. Supports numeric, time-based, and string
-   * fields. Type coercion for string fields is handled by Calcite's type system.
+   * Creates a BinnableField. Validates that the field type is compatible with binning operations.
    *
    * @param fieldExpr The Rex expression for the field
    * @param fieldType The relational data type of the field
    * @param fieldName The name of the field (for error messages)
+   * @throws SemanticCheckException if the field type is not supported for binning
    */
   public BinnableField(RexNode fieldExpr, RelDataType fieldType, String fieldName) {
     this.fieldExpr = fieldExpr;
@@ -37,6 +35,12 @@ public BinnableField(RexNode fieldExpr, RelDataType fieldType, String fieldName)
 
     this.isTimeBased = OpenSearchTypeFactory.isTimeBasedType(fieldType);
     this.isNumeric = OpenSearchTypeFactory.isNumericType(fieldType);
+
+    // Reject truly unsupported types (e.g., BOOLEAN, ARRAY, MAP)
+    if (!isNumeric && !isTimeBased) {
+      throw new SemanticCheckException(
+          String.format("Cannot apply binning to field '%s': unsupported type", fieldName));
+    }
   }
 
   /**

core/src/main/java/org/opensearch/sql/expression/function/udf/binning/MinspanBucketFunction.java

@@ -73,7 +73,14 @@ public Expression implement(
     /** Minspan bucket calculation. */
     public static String calculateMinspanBucket(
         Number fieldValue, Number minSpanParam, Number dataRange, Number maxValue) {
-      if (fieldValue == null || minSpanParam == null || dataRange == null || maxValue == null) {
+      // Detect NULL from failed type coercion (e.g., CAST("abc" AS DOUBLE) returns NULL)
+      if (dataRange == null || maxValue == null) {
+        throw new IllegalArgumentException(
+            "Cannot apply binning: field contains non-numeric string values. "
+                + "Only numeric types or string types with valid numeric values are supported.");
+      }
+
+      if (fieldValue == null || minSpanParam == null) {
         return null;
       }
 

core/src/main/java/org/opensearch/sql/expression/function/udf/binning/RangeBucketFunction.java

@@ -79,7 +79,14 @@ public Expression implement(
     /** Range bucket calculation with expansion algorithm and magnitude-based width. */
     public static String calculateRangeBucket(
         Number fieldValue, Number dataMin, Number dataMax, Number startParam, Number endParam) {
-      if (fieldValue == null || dataMin == null || dataMax == null) {
+      // Detect NULL from failed type coercion (e.g., CAST("abc" AS DOUBLE) returns NULL)
+      if (dataMin == null || dataMax == null) {
+        throw new IllegalArgumentException(
+            "Cannot apply binning: field contains non-numeric string values. "
+                + "Only numeric types or string types with valid numeric values are supported.");
+      }
+
+      if (fieldValue == null) {
         return null;
       }
 

core/src/main/java/org/opensearch/sql/expression/function/udf/binning/SpanBucketFunction.java

@@ -66,7 +66,14 @@ public Expression implement(
 
     /** Span bucket calculation. */
     public static String calculateSpanBucket(Number fieldValue, Number spanParam) {
-      if (fieldValue == null || spanParam == null) {
+      // Detect NULL from failed type coercion (e.g., CAST("abc" AS DOUBLE) returns NULL)
+      if (fieldValue == null) {
+        throw new IllegalArgumentException(
+            "Cannot apply binning: field contains non-numeric string values. "
+                + "Only numeric types or string types with valid numeric values are supported.");
+      }
+
+      if (spanParam == null) {
         return null;
       }
 

core/src/main/java/org/opensearch/sql/expression/function/udf/binning/WidthBucketFunction.java

@@ -91,7 +91,14 @@ public Expression implement(
     /** Width bucket calculation using nice number algorithm. */
     public static String calculateWidthBucket(
         Number fieldValue, Number numBinsParam, Number dataRange, Number maxValue) {
-      if (fieldValue == null || numBinsParam == null || dataRange == null || maxValue == null) {
+      // Detect NULL from failed type coercion (e.g., CAST("abc" AS DOUBLE) returns NULL)
+      if (dataRange == null || maxValue == null) {
+        throw new IllegalArgumentException(
+            "Cannot apply binning: field contains non-numeric string values. "
+                + "Only numeric types or string types with valid numeric values are supported.");
+      }
+
+      if (fieldValue == null || numBinsParam == null) {
         return null;
       }
 

docs/user/ppl/cmd/bin.rst

@@ -16,13 +16,13 @@ bin
 
 Description
 ============
-| The ``bin`` command groups numeric values into buckets of equal intervals, making it useful for creating histograms and analyzing data distribution. It takes a numeric or time-based field and generates a new field with values that represent the lower bound of each bucket. String fields containing numeric values are automatically converted to numeric types through type coercion.
+| The ``bin`` command groups numeric values into buckets of equal intervals, making it useful for creating histograms and analyzing data distribution. It takes a numeric or time-based field and generates a new field with values that represent the lower bound of each bucket. String fields containing valid numeric values (e.g., "25", "30.5") are automatically converted to numeric types through type coercion.
 
 Syntax
 ============
 bin <field> [span=<interval>] [minspan=<interval>] [bins=<count>] [aligntime=(earliest | latest | <time-specifier>)] [start=<value>] [end=<value>]
 
-* field: mandatory. The field to bin. Accepts numeric fields, time-based fields, or string fields containing numeric values (automatically coerced to numeric type).
+* field: mandatory. The field to bin. Accepts numeric fields, time-based fields, or string fields containing valid numeric values (e.g., "25", "30.5"). String fields with numeric values are automatically coerced to numeric type. Non-numeric strings will result in an error.
 * span: optional. The interval size for each bin. Cannot be used with bins or minspan parameters.
 * minspan: optional. The minimum interval size for automatic span calculation. Cannot be used with span or bins parameters.
 * bins: optional. The maximum number of equal-width bins to create. Cannot be used with span or minspan parameters.
@@ -534,7 +534,7 @@ PPL query::
 Example 20: Type coercion with string fields
 ==============================================
 
-The ``bin`` command automatically converts string fields containing numeric values to numeric types, enabling binning operations on keyword/text fields.
+The ``bin`` command automatically converts string fields containing valid numeric values to numeric types, enabling binning operations on keyword/text fields. Only strings that represent valid numbers (e.g., "25", "30.5", "100") are supported. Non-numeric strings will cause an error.
 
 PPL query::
 
@@ -547,5 +547,5 @@ PPL query::
     | 3       | 30-40   |
     +---------+---------+
 
-In this example, even though ``age_str`` is a string field (keyword type), the bin command automatically coerces it to numeric values for binning.
+In this example, even though ``age_str`` is a string field (keyword type) containing numeric values like "28", "32", etc., the bin command automatically coerces these numeric strings to numeric values for binning. If ``age_str`` contained non-numeric strings like "young" or "old", the query would fail.
 

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

@@ -6,6 +6,7 @@
 package org.opensearch.sql.calcite.remote;
 
 import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.opensearch.sql.legacy.TestsConstants.*;
 import static org.opensearch.sql.util.MatcherUtils.rows;
 import static org.opensearch.sql.util.MatcherUtils.schema;
@@ -1067,4 +1068,21 @@ public void testBinWithEvalCreatedDottedFieldName() throws IOException {
         rows(false, "go", "opentelemetry", 16, 1, "12-14"),
         rows(true, "rust", "opentelemetry", 12, 1, "14-16"));
   }
+
+  @Test
+  public void testBinOnNonNumericStringField() {
+    // Test that binning on a string field with non-numeric values fails with clear error
+    ResponseException exception =
+        assertThrows(
+            ResponseException.class,
+            () -> {
+              executeQuery(
+                  String.format("source=%s | bin firstname bins=3 | head 1", TEST_INDEX_ACCOUNT));
+            });
+
+    String errorMessage = exception.getMessage();
+    assertTrue(
+        "Error should indicate non-numeric string values",
+        errorMessage.contains("non-numeric string values"));
+  }
 }

ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLBinTest.java

@@ -5,9 +5,13 @@
 
 package org.opensearch.sql.ppl.calcite;
 
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.test.CalciteAssert;
 import org.junit.Test;
+import org.opensearch.sql.exception.SemanticCheckException;
 
 public class CalcitePPLBinTest extends CalcitePPLAbstractTest {
 
@@ -166,4 +170,15 @@ public void testBinWithAligntime() {
             + " 3600 / 1) * 3600) `SYS_START`\n"
             + "FROM `scott`.`products_temporal`");
   }
+
+  @Test
+  public void testBinOnNonBinnableType() {
+    // Test that binning on truly unsupported types (not numeric, time, or string) fails
+    String ppl = "source=products_temporal | eval bool_field = true | bin bool_field bins=3";
+
+    SemanticCheckException exception =
+        assertThrows(SemanticCheckException.class, () -> getRelNode(ppl));
+    assertTrue(exception.getMessage().contains("Cannot apply binning"));
+    assertTrue(exception.getMessage().contains("unsupported type"));
+  }
 }