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

(cherry picked from commit ef4c51e)

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

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);
 

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

@@ -333,6 +333,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";