Throw exception for zero span

Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>

Author: yuancu

core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java

@@ -492,15 +492,19 @@ public static Span spanFromSpanLengthLiteral(
       UnresolvedExpression field, Literal spanLengthLiteral) {
     if (spanLengthLiteral.getType() == DataType.STRING) {
       String spanText = spanLengthLiteral.getValue().toString();
-      String valueStr = spanText.replaceAll("[^0-9]", "");
-      String unitStr = spanText.replaceAll("[0-9]", "");
+      String valueStr = spanText.replaceAll("[^0-9-]", "");
+      String unitStr = spanText.replaceAll("[0-9-]", "");
 
       if (valueStr.isEmpty()) {
         // No numeric value found, use the literal as-is
         return new Span(field, spanLengthLiteral, SpanUnit.NONE);
       } else {
         // Parse numeric value and unit
         Integer value = Integer.parseInt(valueStr);
+        if (value <= 0) {
+          throw new IllegalArgumentException(
+              String.format("Zero or negative time interval not supported: %s", spanText));
+        }
         SpanUnit unit = unitStr.isEmpty() ? SpanUnit.NONE : SpanUnit.of(unitStr);
         return span(field, intLiteral(value), unit);
       }

integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/4527.yml

@@ -0,0 +1,50 @@
+setup:
+  - do:
+      query.settings:
+        body:
+          transient:
+            plugins.calcite.enabled : true
+  - do:
+      indices.create:
+        index: test_timechart_span_validation
+        body:
+          mappings:
+            properties:
+              "@timestamp":
+                type: date_nanos
+              packets:
+                type: long
+  - do:
+      bulk:
+        index: test_timechart_span_validation
+        refresh: true
+        body:
+          - '{"index": {}}'
+          - '{"@timestamp": "2024-01-15T10:30:04.567890123Z", "packets": 100}'
+          - '{"index": {}}'
+          - '{"@timestamp": "2024-01-15T10:31:04.567890123Z", "packets": 150}'
+          - '{"index": {}}'
+          - '{"@timestamp": "2024-01-15T10:32:04.567890123Z", "packets": 120}'
+
+---
+teardown:
+  - do:
+      query.settings:
+        body:
+          transient:
+            plugins.calcite.enabled : false
+
+---
+"timechart with zero span should return validation error":
+  - skip:
+      features:
+        - headers
+        - allowed_warnings
+  - do:
+      catch: bad_request
+      headers:
+        Content-Type: 'application/json'
+      ppl:
+        body:
+          query: source=test_timechart_span_validation | timechart span=0m per_second(packets)
+  - match: {"$body": "/Zero\\s+or\\s+negative\\s+time\\s+interval\\s+not\\s+supported/"}

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

@@ -6,6 +6,7 @@
 package org.opensearch.sql.ppl.calcite;
 
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import java.util.List;
@@ -342,6 +343,13 @@ public void testTimechartWithUseOtherBeforeLimit() {
     assertNotNull(plan);
   }
 
+  @Test
+  public void testTimechartUsingZeroSpanShouldThrow() {
+    String ppl = "source=events | timechart span=0h limit=5 count() by host";
+    Throwable t = assertThrows(IllegalArgumentException.class, () -> parsePPL(ppl));
+    verifyErrorMessageContains(t, "Zero or negative time interval not supported: 0h");
+  }
+
   private UnresolvedPlan parsePPL(String query) {
     PPLSyntaxParser parser = new PPLSyntaxParser();
     AstBuilder astBuilder = new AstBuilder(query);