add UT and IT for timechart command

ahkcs · ahkcs · commit 22426ba6ac87 · 2025-11-26T11:18:26.000-08:00
Signed-off-by: Kai Huang &lt;ahkcs@amazon.com&gt;
diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteReverseCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteReverseCommandIT.java
@@ -29,6 +29,7 @@ public void init() throws Exception {
     loadIndex(Index.BANK);
     loadIndex(Index.TIME_TEST_DATA);
     loadIndex(Index.STATE_COUNTRY);
+    loadIndex(Index.EVENTS);
   }
 
   @Test
@@ -454,4 +455,62 @@ public void testReverseWithTimestampAfterAggregation() throws IOException {
     // Categories: A=26, B=25, C=25, D=24
     verifyDataRows(result, rows(26, "A"), rows(25, "B"), rows(25, "C"), rows(24, "D"));
   }
+
+  // ==================== Timechart with Reverse tests ====================
+  // These tests verify that reverse works correctly with timechart.
+  // Timechart always adds a sort at the end of its plan (tier 1), so reverse
+  // will find the collation via metadata query and flip the sort direction.
+
+  @Test
+  public void testTimechartWithReverse() throws IOException {
+    // Timechart adds ORDER BY @timestamp ASC at the end
+    // Reverse should flip it to DESC, returning data in reverse chronological order
+    JSONObject result = executeQuery("source=events | timechart span=1m count() | reverse");
+    verifySchema(result, schema("@timestamp", "timestamp"), schema("count()", "bigint"));
+    // Events data has timestamps at 00:00, 00:01, 00:02, 00:03, 00:04
+    // Reversed order: 00:04, 00:03, 00:02, 00:01, 00:00
+    verifyDataRowsInOrder(
+        result,
+        rows("2024-07-01 00:04:00", 1),
+        rows("2024-07-01 00:03:00", 1),
+        rows("2024-07-01 00:02:00", 1),
+        rows("2024-07-01 00:01:00", 1),
+        rows("2024-07-01 00:00:00", 1));
+  }
+
+  @Test
+  public void testTimechartWithCustomTimefieldAndReverse() throws IOException {
+    // Test timechart with custom timefield (birthdate instead of @timestamp)
+    // PR #4784 allows users to specify a custom timefield in timechart
+    // The sort should be on the custom field, not @timestamp
+    JSONObject result =
+        executeQuery(
+            String.format(
+                "source=%s | timechart timefield=birthdate span=1year count() | reverse",
+                TEST_INDEX_BANK));
+    verifySchema(result, schema("birthdate", "timestamp"), schema("count()", "bigint"));
+    // Bank data has birthdates in 2017 and 2018
+    // Timechart groups by year: 2017 (2 records), 2018 (5 records)
+    // Reversed order: 2018, 2017
+    verifyDataRowsInOrder(result, rows("2018-01-01 00:00:00", 5), rows("2017-01-01 00:00:00", 2));
+  }
+
+  @Test
+  public void testTimechartWithGroupByAndReverse() throws IOException {
+    // Test timechart with group by and reverse
+    // The sort is on both @timestamp and the group by field
+    JSONObject result = executeQuery("source=events | timechart span=1h count() by host | reverse");
+    verifySchema(
+        result,
+        schema("@timestamp", "timestamp"),
+        schema("host", "string"),
+        schema("count()", "bigint"));
+    // All events are in the same hour, so only one time bucket
+    // Hosts are grouped and results are reversed
+    verifyDataRows(
+        result,
+        rows("2024-07-01 00:00:00", "db-01", 1),
+        rows("2024-07-01 00:00:00", "web-01", 2),
+        rows("2024-07-01 00:00:00", "web-02", 2));
+  }
 }
diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLStreamstatsTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLStreamstatsTest.java
@@ -229,9 +229,8 @@ public void testStreamstatsWithReverse() {
             + "  LogicalSort(sort0=[$8], dir0=[DESC])\n"
             + "    LogicalSort(sort0=[$8], dir0=[ASC])\n"
             + "      LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],"
-            + " SAL=[$5], COMM=[$6], DEPTNO=[$7], __stream_seq__=[$8], max(SAL)=[CASE(IS NOT"
-            + " NULL($7), MAX($5) OVER (PARTITION BY $7 ROWS UNBOUNDED PRECEDING), null:DECIMAL(7,"
-            + " 2))])\n"
+            + " SAL=[$5], COMM=[$6], DEPTNO=[$7], __stream_seq__=[$8], max(SAL)=[MAX($5) OVER"
+            + " (PARTITION BY $7 ROWS UNBOUNDED PRECEDING)])\n"
             + "        LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],"
             + " SAL=[$5], COMM=[$6], DEPTNO=[$7], __stream_seq__=[ROW_NUMBER() OVER ()])\n"
             + "          LogicalTableScan(table=[[scott, EMP]])\n";
@@ -240,9 +239,8 @@ public void testStreamstatsWithReverse() {
     String expectedSparkSql =
         "SELECT `EMPNO`, `ENAME`, `JOB`, `MGR`, `HIREDATE`, `SAL`, `COMM`, `DEPTNO`, `max(SAL)`\n"
             + "FROM (SELECT `EMPNO`, `ENAME`, `JOB`, `MGR`, `HIREDATE`, `SAL`, `COMM`, `DEPTNO`,"
-            + " `__stream_seq__`, CASE WHEN `DEPTNO` IS NOT NULL THEN MAX(`SAL`) OVER (PARTITION"
-            + " BY `DEPTNO` ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) ELSE NULL END"
-            + " `max(SAL)`\n"
+            + " `__stream_seq__`, MAX(`SAL`) OVER (PARTITION BY `DEPTNO` ROWS BETWEEN UNBOUNDED"
+            + " PRECEDING AND CURRENT ROW) `max(SAL)`\n"
             + "FROM (SELECT `EMPNO`, `ENAME`, `JOB`, `MGR`, `HIREDATE`, `SAL`, `COMM`, `DEPTNO`,"
             + " ROW_NUMBER() OVER () `__stream_seq__`\n"
             + "FROM `scott`.`EMP`) `t`\n"
diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java
@@ -53,13 +53,28 @@ protected Frameworks.ConfigBuilder config(CalciteAssert.SchemaSpec... schemaSpec
     ImmutableList<Object[]> rows =
         ImmutableList.of(
             new Object[] {
-              java.sql.Timestamp.valueOf("2024-07-01 00:00:00"), "web-01", "us-east", 45.2, 120
+              java.sql.Timestamp.valueOf("2024-07-01 00:00:00"),
+              java.sql.Timestamp.valueOf("2024-01-15 10:00:00"),
+              "web-01",
+              "us-east",
+              45.2,
+              120
             },
             new Object[] {
-              java.sql.Timestamp.valueOf("2024-07-01 00:01:00"), "web-02", "us-west", 38.7, 150
+              java.sql.Timestamp.valueOf("2024-07-01 00:01:00"),
+              java.sql.Timestamp.valueOf("2024-02-20 11:00:00"),
+              "web-02",
+              "us-west",
+              38.7,
+              150
             },
             new Object[] {
-              java.sql.Timestamp.valueOf("2024-07-01 00:02:00"), "web-01", "us-east", 55.3, 200
+              java.sql.Timestamp.valueOf("2024-07-01 00:02:00"),
+              java.sql.Timestamp.valueOf("2024-03-25 12:00:00"),
+              "web-01",
+              "us-east",
+              55.3,
+              200
             });
     schema.add("events", new EventsTable(rows));
     return Frameworks.newConfigBuilder()
@@ -347,6 +362,68 @@ public void testTimechartUsingZeroSpanShouldThrow() {
     verifyErrorMessageContains(t, "Zero or negative time interval not supported: 0h");
   }
 
+  // ==================== Timechart with Reverse tests ====================
+  // These tests verify that reverse works correctly with timechart.
+  // Timechart always adds a sort at the end of its plan, so reverse will
+  // find the collation via metadata query (tier 1) and flip the sort direction.
+
+  @Test
+  public void testTimechartWithReverse() {
+    // Timechart adds ORDER BY @timestamp ASC at the end
+    // Reverse should flip it to DESC
+    String ppl = "source=events | timechart count() | reverse";
+    RelNode root = getRelNode(ppl);
+    // The plan should have two sorts: original ASC from timechart, then DESC from reverse
+    String expectedLogical =
+        "LogicalSort(sort0=[$0], dir0=[DESC])\n"
+            + "  LogicalSort(sort0=[$0], dir0=[ASC])\n"
+            + "    LogicalProject(@timestamp=[$0], count()=[$1])\n"
+            + "      LogicalAggregate(group=[{0}], count()=[COUNT()])\n"
+            + "        LogicalProject(@timestamp0=[SPAN($0, 1, 'm')])\n"
+            + "          LogicalFilter(condition=[IS NOT NULL($0)])\n"
+            + "            LogicalTableScan(table=[[scott, events]])\n";
+    verifyLogical(root, expectedLogical);
+
+    String expectedSparkSql =
+        "SELECT *\n"
+            + "FROM (SELECT SPAN(`@timestamp`, 1, 'm') `@timestamp`, COUNT(*) `count()`\n"
+            + "FROM `scott`.`events`\n"
+            + "WHERE `@timestamp` IS NOT NULL\n"
+            + "GROUP BY SPAN(`@timestamp`, 1, 'm')\n"
+            + "ORDER BY 1 NULLS LAST) `t3`\n"
+            + "ORDER BY `@timestamp` DESC NULLS FIRST";
+    verifyPPLToSparkSQL(root, expectedSparkSql);
+  }
+
+  @Test
+  public void testTimechartWithCustomTimefieldAndReverse() {
+    // Timechart with custom timefield should also work with reverse
+    // The sort is on created_at (the custom field), not @timestamp
+    String ppl = "source=events | timechart timefield=created_at span=1month count() | reverse";
+    RelNode root = getRelNode(ppl);
+
+    // Verify the logical plan shows two sorts: ASC from timechart, DESC from reverse
+    String expectedLogical =
+        "LogicalSort(sort0=[$0], dir0=[DESC])\n"
+            + "  LogicalSort(sort0=[$0], dir0=[ASC])\n"
+            + "    LogicalProject(created_at=[$0], count()=[$1])\n"
+            + "      LogicalAggregate(group=[{0}], count()=[COUNT()])\n"
+            + "        LogicalProject(created_at0=[SPAN($1, 1, 'M')])\n"
+            + "          LogicalFilter(condition=[IS NOT NULL($1)])\n"
+            + "            LogicalTableScan(table=[[scott, events]])\n";
+    verifyLogical(root, expectedLogical);
+
+    String expectedSparkSql =
+        "SELECT *\n"
+            + "FROM (SELECT SPAN(`created_at`, 1, 'M') `created_at`, COUNT(*) `count()`\n"
+            + "FROM `scott`.`events`\n"
+            + "WHERE `created_at` IS NOT NULL\n"
+            + "GROUP BY SPAN(`created_at`, 1, 'M')\n"
+            + "ORDER BY 1 NULLS LAST) `t3`\n"
+            + "ORDER BY `created_at` DESC NULLS FIRST";
+    verifyPPLToSparkSQL(root, expectedSparkSql);
+  }
+
   private UnresolvedPlan parsePPL(String query) {
     PPLSyntaxParser parser = new PPLSyntaxParser();
     AstBuilder astBuilder = new AstBuilder(query);
@@ -363,6 +440,8 @@ public static class EventsTable implements ScannableTable {
                 .builder()
                 .add("@timestamp", SqlTypeName.TIMESTAMP)
                 .nullable(true)
+                .add("created_at", SqlTypeName.TIMESTAMP)
+                .nullable(true)
                 .add("host", SqlTypeName.VARCHAR)
                 .nullable(true)
                 .add("region", SqlTypeName.VARCHAR)
