Merge the implementation of `timechart` and `chart` #4755

yuancu · 2025-11-07T08:58:44Z

Description

timechart is semantically a subset of chart , where the row-split is always fixed to @timestamp. This PR merges their implementation for easier maintenance and resolves a few existing bugs of timechart.

Related Issues

Resolves #4581, resolves #4582, resolves #4632

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
New functionality has a user manual doc added.
New PPL command checklist all confirmed.
API changes companion pull request created.
Commits are signed per the DCO using --signoff or -s.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

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

- add 2 more indicies for test purpose Signed-off-by: Yuanchun Shen <[email protected]>

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

yuancu · 2025-11-07T09:01:59Z

docs/user/ppl/cmd/timechart.rst

+Example 3: Calculate average number of packets by minute
 ================================================

-This example calculates the average CPU usage for each minute without grouping by any field.


cpu_usage does not exist in the given index, therefore replaced

yuancu · 2025-11-07T09:02:35Z

docs/user/ppl/cmd/timechart.rst

+Example 5: Count events by hour and category
 =====================================================================

-This example counts events for each second and groups them by region, showing zero values for time-region combinations with no data.


region does not exist in the given index, therefore replaced

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

LantaoJin · 2025-11-12T09:40:35Z

docs/user/ppl/cmd/timechart.rst

+    fetched rows / total rows = 8/8
+    +---------------------+---------+---------+
+    | @timestamp          | host    | count() |
+    |---------------------+---------+---------|
+    | 2023-01-01 10:00:00 | server1 | 1       |
+    | 2023-01-01 10:05:00 | server2 | 1       |
+    | 2023-01-01 10:10:00 | server1 | 1       |
+    | 2023-01-01 10:15:00 | server2 | 1       |
+    | 2023-01-01 10:20:00 | server1 | 1       |
+    | 2023-01-01 10:25:00 | server2 | 1       |
+    | 2023-01-01 10:30:00 | server1 | 1       |
+    | 2023-01-01 10:35:00 | server2 | 1       |
+    +---------------------+---------+---------+
+


Why the results before and after for the same ppl query are different? please confirm the current result is correct comparing to SPL.

The difference lies in whether we fill 0 for non-existing groups.

In SPL, timechart and chart will pivot the result table, explicitly filling 0 for count aggregation, leaving it empty for the rest aggregation functions. E.g. the result in SPL will be like:

@timestamp server1 server2

2023-01-01 10:00:00 1 0

2023-01-01 10:05:00 0 1

2023-01-01 10:10:00 1 0

2023-01-01 10:15:00 0 1

2023-01-01 10:20:00 1 0

2023-01-01 10:25:00 0 1

2023-01-01 10:30:00 1 0

2023-01-01 10:35:00 0 1

In this thread and offline discussions, we decided to not fill 0 for non-existing groups to simplify implementations and keep consistence with docs (see #4632), leaving the pivoting and zero-filling to frontend if necessary.

LantaoJin · 2025-11-12T09:45:31Z

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

+                "source=%s | chart usenull=false count() over gender by age "
+                    + " span=10 nullstr='not_shown'",


why the location of option nullstr changed?

@penghuo commented that it would be better if the argument positions are more flexible in #4579 (comment)

Here, I altered the location of argument nullstr to exemplify this point

LantaoJin · 2025-11-12T09:46:24Z

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

-
    JSONObject result =
-        executeQuery("source=events_null | timechart span=1d limit=2 count() by host");
+        executeQuery("source=events_null | timechart count() by host span=1d limit=2");


ditto: cannot timechart span=1d limit=2 count() by host work now? can it work in spl?

It works. The locations of these arguments are scattered in different locations across the ITs.

opensearch-trigger-bot · 2025-11-12T10:42:37Z

The backport to 2.19-dev failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/sql/backport-2.19-dev 2.19-dev
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4755-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 daf1795a0672c9f6ce1d1bd74fc58415963d099a
# Push it to GitHub
git push --set-upstream origin backport/backport-4755-to-2.19-dev
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.19-dev

Then, create a pull request where the base branch is 2.19-dev and the compare/head branch is backport/backport-4755-to-2.19-dev.

…ct#4755) * Remove visitTimechart Signed-off-by: Yuanchun Shen <[email protected]> * Migrate per functions to Chart Signed-off-by: Yuanchun Shen <[email protected]> * Update CalcitePPLTimechartTest Signed-off-by: Yuanchun Shen <[email protected]> * Migrate TimecharTest to use Chart Signed-off-by: Yuanchun Shen <[email protected]> * Fix AST relevant tests Signed-off-by: Yuanchun Shen <[email protected]> * Remove Timechart AST object in favor of Chart Signed-off-by: Yuanchun Shen <[email protected]> * Update expected plans for timechart Signed-off-by: Yuanchun Shen <[email protected]> * Update doctest for timechart - add 2 more indicies for test purpose Signed-off-by: Yuanchun Shen <[email protected]> * Add yaml tests for 4581, 4582, and 4632 Signed-off-by: Yuanchun Shen <[email protected]> * Allow flexible parameter positions for chart and timechart Signed-off-by: Yuanchun Shen <[email protected]> * Simplify CalciteTimechartCommandIT Signed-off-by: Yuanchun Shen <[email protected]> --------- Signed-off-by: Yuanchun Shen <[email protected]> (cherry picked from commit daf1795)

* Remove visitTimechart * Migrate per functions to Chart * Update CalcitePPLTimechartTest * Migrate TimecharTest to use Chart * Fix AST relevant tests * Remove Timechart AST object in favor of Chart * Update expected plans for timechart * Update doctest for timechart - add 2 more indicies for test purpose * Add yaml tests for 4581, 4582, and 4632 * Allow flexible parameter positions for chart and timechart * Simplify CalciteTimechartCommandIT --------- (cherry picked from commit daf1795) Signed-off-by: Yuanchun Shen <[email protected]>

yuancu added 10 commits November 7, 2025 16:53

Remove visitTimechart

74427a5

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

Migrate per functions to Chart

c577973

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

Update CalcitePPLTimechartTest

69f518f

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

Migrate TimecharTest to use Chart

5c288c8

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

Fix AST relevant tests

75bbb84

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

Remove Timechart AST object in favor of Chart

2c958b3

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

Update expected plans for timechart

b6c69fe

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

Update doctest for timechart

2315560

- add 2 more indicies for test purpose Signed-off-by: Yuanchun Shen <[email protected]>

Add yaml tests for 4581, 4582, and 4632

8155c84

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

Allow flexible parameter positions for chart and timechart

5c12898

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

yuancu commented Nov 7, 2025

View reviewed changes

yuancu added bug Something isn't working backport 2.19-dev enhancement New feature or request labels Nov 7, 2025

yuancu marked this pull request as ready for review November 7, 2025 10:08

yuancu requested review from MaxKsyunz, Swiddis, YANG-DB, Yury-Fridlyand, anirudha, dai-chen, derek-ho, forestmvey, joshuali925, kavithacm, mengweieric, penghuo, ps48, seankao-az and vamsimanohar as code owners November 7, 2025 10:08

yuancu requested review from GumpacG, LantaoJin, RyanL1997, acarbonetto, noCharger, qianheng-aws and ykmr1224 as code owners November 7, 2025 10:08

Simplify CalciteTimechartCommandIT

54d0d04

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

This was referenced Nov 7, 2025

Fix eval on grouped fields after timechart #4758

Merged

Support chart command in PPL #4579

Merged

qianheng-aws approved these changes Nov 11, 2025

View reviewed changes

yuancu mentioned this pull request Nov 12, 2025

Specify timestamp field with timefield in timechart command #4784

Merged

8 tasks

LantaoJin reviewed Nov 12, 2025

View reviewed changes

LantaoJin approved these changes Nov 12, 2025

View reviewed changes

LantaoJin merged commit daf1795 into opensearch-project:main Nov 12, 2025
35 checks passed

opensearch-trigger-bot bot added the backport-failed label Nov 12, 2025

yuancu deleted the issues/4581 branch November 12, 2025 12:45

yuancu mentioned this pull request Nov 13, 2025

[Backport 2.19-dev] Merge the implementation of timechart and chart (#4755) #4796

Merged

8 tasks

LantaoJin added the backport-manually Filed a PR to backport manually. label Nov 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Merge the implementation of `timechart` and `chart` #4755

Merge the implementation of `timechart` and `chart` #4755

Uh oh!

yuancu commented Nov 7, 2025 •

edited

Loading

Uh oh!

yuancu Nov 7, 2025

Uh oh!

yuancu Nov 7, 2025

Uh oh!

LantaoJin Nov 12, 2025

Uh oh!

yuancu Nov 12, 2025 •

edited

Loading

Uh oh!

LantaoJin Nov 12, 2025

Uh oh!

yuancu Nov 12, 2025

Uh oh!

LantaoJin Nov 12, 2025

Uh oh!

yuancu Nov 12, 2025 •

edited

Loading

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`@timestamp`	`server1`	`server2`
2023-01-01 10:00:00	1	0
2023-01-01 10:05:00	0	1
2023-01-01 10:10:00	1	0
2023-01-01 10:15:00	0	1
2023-01-01 10:20:00	1	0
2023-01-01 10:25:00	0	1
2023-01-01 10:30:00	1	0
2023-01-01 10:35:00	0	1

		"source=%s \| chart usenull=false count() over gender by age "
		+ " span=10 nullstr='not_shown'",

Merge the implementation of timechart and chart #4755

Merge the implementation of timechart and chart #4755

Uh oh!

Conversation

yuancu commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

yuancu Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

yuancu Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

LantaoJin Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

yuancu Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LantaoJin Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

yuancu Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

LantaoJin Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

yuancu Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Merge the implementation of `timechart` and `chart` #4755

Merge the implementation of `timechart` and `chart` #4755

yuancu commented Nov 7, 2025 •

edited

Loading

yuancu Nov 12, 2025 •

edited

Loading

yuancu Nov 12, 2025 •

edited

Loading