Make the error messages for decimal span length more meaningful

Signed-off-by: Yuanchun Shen <[email protected]>

Author: yuancu

ppl/src/main/antlr/OpenSearchPPLLexer.g4

@@ -509,40 +509,35 @@ ALIGNTIME:                          'ALIGNTIME';
 // Must precede ID to avoid conflicts with identifier matching
 PERCENTILE_SHORTCUT:                PERC(INTEGER_LITERAL | DECIMAL_LITERAL) | 'P'(INTEGER_LITERAL | DECIMAL_LITERAL);
 
-SPANLENGTH: [0-9]+ (
-    'US' |'CS'|'DS'
-    |'MS'|'MILLISECOND'|'MILLISECONDS'
-    |'S'|'SEC'|'SECS'|'SECOND'|'SECONDS'
-    |'MIN'|'MINS'|'MINUTE'|'MINUTES'
-    |'H'|'HR'|'HRS'|'HOUR'|'HOURS'
-    |'H'|'HR'|'HRS'|'HOUR'|'HOURS'
-    |'D'|'DAY'|'DAYS'
-    |'W'|'WEEK'|'WEEKS'
-    |'M'|'MON'|'MONTH'|'MONTHS'
-    |'Q'|'QTR'|'QTRS'|'QUARTER'|'QUARTERS'
-    |'Y'|'YR'|'YRS'|'YEAR'|'YEARS'
-);
+fragment DAY_OR_DOUBLE:             'D';
+fragment COMMON_TIME_UNIT:           'S'|'SEC'|'SECOND'
+                                    |'M'|'MIN'|'MINUTE'
+                                    |'H'|'HR'|'HOUR'
+                                    |'DAY'|'W'|'WEEK'
+                                    |'MON'|'MONTH'
+                                    |'Q'|'QTR'|'QUARTER'
+                                    |'Y'|'YR'|'YEAR';
+fragment PLURAL_UNIT:               'MILLISECONDS'|'SECS'|'SECONDS'|'MINS'|'MINUTES'|'HRS'|'HOURS'
+                                    |'DAYS'|'MONTHS'|'QTRS'|'QUARTERS'|'YRS'|'YEARS';
+fragment SPANUNIT:                  COMMON_TIME_UNIT | PLURAL_UNIT
+                                    |'US'|'CS'|'DS'
+                                    |'MS'|'MILLISECOND';
+SPANLENGTH:                         DEC_DIGIT+ (SPANUNIT | DAY_OR_DOUBLE);
+DECIMAL_SPANLENGTH:                 (DEC_DIGIT+)? '.' DEC_DIGIT+  SPANUNIT;
 
 NUMERIC_ID : DEC_DIGIT+ ID_LITERAL;
 
 // LITERALS AND VALUES
 //STRING_LITERAL:                     DQUOTA_STRING | SQUOTA_STRING | BQUOTA_STRING;
 fragment WEEK_SNAP_UNIT:            'W' [0-7];
-fragment TIME_SNAP_UNIT:              'S' | 'SEC' | 'SECOND'
-                                    | 'M' | 'MIN' | 'MINUTE'
-                                    | 'H' | 'HR' | 'HOUR' | 'HOURS'
-                                    | 'D' | 'DAY'
-                                    | 'W' | 'WEEK' | WEEK_SNAP_UNIT
-                                    | 'MON' | 'MONTH'
-                                    | 'Q' | 'QTR' | 'QUARTER'
-                                    | 'Y' | 'YR' | 'YEAR';
+fragment TIME_SNAP_UNIT:            COMMON_TIME_UNIT | WEEK_SNAP_UNIT | DAY_OR_DOUBLE;
 TIME_SNAP:                          AT TIME_SNAP_UNIT;
 ID:                                 ID_LITERAL;
 CLUSTER:                            CLUSTER_PREFIX_LITERAL;
 INTEGER_LITERAL:                    DEC_DIGIT+;
 DECIMAL_LITERAL:                    (DEC_DIGIT+)? '.' DEC_DIGIT+;
 FLOAT_LITERAL:                      (DEC_DIGIT+)? '.' DEC_DIGIT+ 'F';
-DOUBLE_LITERAL:                     (DEC_DIGIT+)? '.' DEC_DIGIT+ 'D';
+DOUBLE_LITERAL:                     (DEC_DIGIT+)? '.' DEC_DIGIT+ DAY_OR_DOUBLE;
 
 fragment DATE_SUFFIX:               ([\-.][*0-9]+)+;
 fragment CLUSTER_PREFIX_LITERAL:    [*A-Z]+?[*A-Z_\-0-9]* COLON;

ppl/src/main/antlr/OpenSearchPPLParser.g4

@@ -299,6 +299,8 @@ timechartParameter
 
 spanLiteral
    : SPANLENGTH
+   | DECIMAL_SPANLENGTH
+   | DOUBLE_LITERAL  // 1.5d can also represent decimal span length
    | INTEGER_LITERAL
    | DECIMAL_LITERAL
    ;

ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java

@@ -816,6 +816,13 @@ public UnresolvedExpression visitSpanLiteral(OpenSearchPPLParser.SpanLiteralCont
       return AstDSL.intLiteral(Integer.parseInt(ctx.INTEGER_LITERAL().getText()));
     } else if (ctx.DECIMAL_LITERAL() != null) {
       return AstDSL.decimalLiteral(new BigDecimal(ctx.DECIMAL_LITERAL().getText()));
+    } else if (ctx.DECIMAL_SPANLENGTH() != null || ctx.DOUBLE_LITERAL() != null) {
+      throw new IllegalArgumentException(
+          StringUtils.format(
+              "Span length [%s] is invalid: floating-point time intervals are not supported.",
+              ctx.DECIMAL_SPANLENGTH() != null
+                  ? ctx.DECIMAL_SPANLENGTH().getText()
+                  : ctx.DOUBLE_LITERAL().getText()));
     } else {
       return AstDSL.stringLiteral(ctx.getText());
     }

ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java

@@ -1493,13 +1493,19 @@ public void testReplaceCommandWithMultiplePairs() {
   public void testTimeSpanWithDecimalShouldThrow() {
     Throwable t1 =
         assertThrows(
-            SyntaxCheckException.class, () -> plan("source=t | timechart  span=1.5d count"));
-    assertTrue(t1.getMessage().contains("[1.5d] is not a valid term at this part of the query"));
+            IllegalArgumentException.class, () -> plan("source=t | timechart  span=1.5d count"));
+    assertTrue(
+        t1.getMessage()
+            .contains(
+                "Span length [1.5d] is invalid: floating-point time intervals are not supported."));
 
     Throwable t2 =
         assertThrows(
-            SyntaxCheckException.class,
+            IllegalArgumentException.class,
             () -> plan("source=t | stats count by span(@timestamp, 2.5y)"));
-    assertTrue(t2.getMessage().contains("[y] is not a valid term at this part of the query"));
+    assertTrue(
+        t2.getMessage()
+            .contains(
+                "Span length [2.5y] is invalid: floating-point time intervals are not supported."));
   }
 }