Enhance doc and error message handling for bins on time-related fields (#4713)

Author: ahkcs

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

@@ -360,6 +360,17 @@ public RelNode visit(TableScan scan) {
         final RelRunner runner = connection.unwrap(RelRunner.class);
         return runner.prepareStatement(rel);
       } catch (SQLException e) {
+        // Detect if error is due to window functions in unsupported context (bins on time fields)
+        String errorMsg = e.getMessage();
+        if (errorMsg != null
+            && errorMsg.contains("Error while preparing plan")
+            && errorMsg.contains("WIDTH_BUCKET")) {
+          throw new UnsupportedOperationException(
+              "The 'bins' parameter on timestamp fields requires: (1) pushdown to be enabled"
+                  + " (controlled by plugins.calcite.pushdown.enabled, enabled by default), and"
+                  + " (2) the timestamp field to be used as an aggregation bucket (e.g., 'stats"
+                  + " count() by @timestamp').");
+        }
         throw Util.throwAsRuntime(e);
       }
     }

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

@@ -28,7 +28,11 @@ public RexNode createExpression(
     CountBin countBin = (CountBin) node;
 
     // Create validated binnable field (validates that field is numeric or time-based)
-    // Note: bins parameter works with both numeric and time-based fields
+    // Note: bins parameter on time-based fields requires:
+    //       1. Pushdown to be enabled (controlled by plugins.calcite.pushdown.enabled, enabled by
+    //          default)
+    //       2. The time-based field to be used as an aggregation bucket
+    //          (e.g., stats count() by @timestamp)
     String fieldName = BinFieldValidator.extractFieldName(node);
     BinnableField field = new BinnableField(fieldExpr, fieldExpr.getType(), fieldName);
 

docs/user/ppl/cmd/bin.md

@@ -20,7 +20,12 @@ bin \<field\> [span=\<interval\>] [minspan=\<interval\>] [bins=\<count\>] [align
     * day (d, day, days)  
     * month (M, mon, month, months)  
 * 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. The bins parameter must be between 2 and 50000 (inclusive).  
+* bins: optional. The maximum number of equal-width bins to create. Cannot be used with span or minspan parameters. The bins parameter must be between 2 and 50000 (inclusive).
+
+  **Limitation**: The bins parameter on timestamp fields has the following requirements:
+
+  1. **Pushdown must be enabled**: Controlled by ``plugins.calcite.pushdown.enabled`` (enabled by default). When pushdown is disabled, use the ``span`` parameter instead (e.g., ``bin @timestamp span=5m``).
+  2. **Timestamp field must be used as an aggregation bucket**: The binned timestamp field must be used in a ``stats`` aggregation (e.g., ``source=events | bin @timestamp bins=3 | stats count() by @timestamp``). Using bins on timestamp fields outside of aggregation buckets is not supported.  
 * aligntime: optional. Align the bin times for time-based fields. Valid only for time-based discretization. Options:  
   * earliest: Align bins to the earliest timestamp in the data  
   * latest: Align bins to the latest timestamp in the data  

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

@@ -5,6 +5,7 @@
 
 package org.opensearch.sql.calcite.remote;
 
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.opensearch.sql.legacy.TestsConstants.*;
@@ -988,6 +989,33 @@ public void testStatsWithBinsOnTimeAndTermField_Avg() throws IOException {
         rows(40.25, "us-west", "2024-07-01 00:01:00"));
   }
 
+  @Test
+  public void testBinsOnTimeFieldWithPushdownDisabled_ShouldFail() throws IOException {
+    // Verify that bins parameter on timestamp fields fails with clear error when pushdown disabled
+    enabledOnlyWhenPushdownIsDisabled();
+
+    ResponseException exception =
+        assertThrows(
+            ResponseException.class,
+            () ->
+                executeQuery(
+                    "source=events_null | bin @timestamp bins=3 | stats count() by @timestamp"));
+
+    // Verify the error message clearly explains the limitation and suggests solutions
+    // Note: bins parameter on timestamp fields requires BOTH:
+    //   1. Pushdown to be enabled (plugins.calcite.pushdown.enabled=true, enabled by default)
+    //   2. The timestamp field to be used as an aggregation bucket (e.g., stats count() by
+    // @timestamp)
+    String errorMessage = exception.getMessage();
+    assertTrue(
+        "Expected clear error message about bins parameter requirements on timestamp fields, but"
+            + " got: "
+            + errorMessage,
+        errorMessage.contains("bins' parameter on timestamp fields requires")
+            && errorMessage.contains("pushdown to be enabled")
+            && errorMessage.contains("aggregation bucket"));
+  }
+
   @Test
   public void testBinWithNestedFieldWithoutExplicitProjection() throws IOException {
     // Test bin command on nested field without explicit fields projection

integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java

@@ -323,6 +323,10 @@ protected void enabledOnlyWhenPushdownIsEnabled() throws IOException {
     Assume.assumeTrue("This test is only for when push down is enabled", !isPushdownDisabled());
   }
 
+  protected void enabledOnlyWhenPushdownIsDisabled() throws IOException {
+    Assume.assumeTrue("This test is only for when push down is disabled", isPushdownDisabled());
+  }
+
   public void updatePushdownSettings() throws IOException {
     String pushdownEnabled = String.valueOf(GlobalPushdownConfig.enabled);
     assert !pushdownEnabled.isBlank() : "Pushdown enabled setting cannot be empty";