[Backport 2.19-dev] Support chart command in PPL (#4579)#4756
Merged
LantaoJin merged 4 commits intoopensearch-project:2.19-devfrom Nov 10, 2025
Merged
Conversation
* WIP: Make poc implementation for chart command Signed-off-by: Yuanchun Shen <[email protected]> * Support param useother and otherstr Signed-off-by: Yuanchun Shen <[email protected]> * Support usenull and nullstr (when both row split and col split present) Signed-off-by: Yuanchun Shen <[email protected]> * Append a final aggregation to merge OTHER categories Signed-off-by: Yuanchun Shen <[email protected]> * Handle common agg functions for OTHER category for timechart Signed-off-by: Yuanchun Shen <[email protected]> * Fix timechart IT Signed-off-by: Yuanchun Shen <[email protected]> * Sort earliest results with asc order Signed-off-by: Yuanchun Shen <[email protected]> * Support non-string fields as column split Signed-off-by: Yuanchun Shen <[email protected]> * Fix min/earliest order & fix non-accumulative agg for chart Signed-off-by: Yuanchun Shen <[email protected]> * Hint non-null in aggregateWithTrimming Signed-off-by: Yuanchun Shen <[email protected]> * Add integration tests for chart command Signed-off-by: Yuanchun Shen <[email protected]> * Add unit tests Signed-off-by: Yuanchun Shen <[email protected]> * Add doc for chart command Signed-off-by: Yuanchun Shen <[email protected]> * Prompt users that multiple agg is not supported Signed-off-by: Yuanchun Shen <[email protected]> * Add explain ITs Signed-off-by: Yuanchun Shen <[email protected]> * Remove unimplemented support for multiple aggregations in chart command Signed-off-by: Yuanchun Shen <[email protected]> * Add unit tests for chart command Signed-off-by: Yuanchun Shen <[email protected]> * Remove irrelevant yaml test Signed-off-by: Yuanchun Shen <[email protected]> * Tweak chart.rst Signed-off-by: Yuanchun Shen <[email protected]> * Swap the order of chart output to ensure metrics come last Signed-off-by: Yuanchun Shen <[email protected]> * Filter rows without col split when calculate grand total Signed-off-by: Yuanchun Shen <[email protected]> * Chores: tweak code order Signed-off-by: Yuanchun Shen <[email protected]> * Add anonymize test to chart command Signed-off-by: Yuanchun Shen <[email protected]> * Change grammart from limit=top 10 to limit=top10 Signed-off-by: Yuanchun Shen <[email protected]> * Update chart doc Signed-off-by: Yuanchun Shen <[email protected]> * Rename __row_number__ for chart to _row_number_chart_ Signed-off-by: Yuanchun Shen <[email protected]> * Sort by row and col splits on top of chart results Signed-off-by: Yuanchun Shen <[email protected]> * Ignore rows without a row split in chart command Signed-off-by: Yuanchun Shen <[email protected]> * Keep categories with max summed values when top k is set Signed-off-by: Yuanchun Shen <[email protected]> * Simplify toAddHintsOnAggregate condition Signed-off-by: Yuanchun Shen <[email protected]> * Chores: eliminate unnecessary variables Signed-off-by: Yuanchun Shen <[email protected]> * Apply a non-null filter on fields referred by aggregations Signed-off-by: Yuanchun Shen <[email protected]> * Fix chart plans Signed-off-by: Yuanchun Shen <[email protected]> * Get rid of record class Signed-off-by: Yuanchun Shen <[email protected]> * Move ranking by column split to a helper function Signed-off-by: Yuanchun Shen <[email protected]> --------- Signed-off-by: Yuanchun Shen <[email protected]> (cherry picked from commit 5523932)
Signed-off-by: Yuanchun Shen <[email protected]>
Signed-off-by: Yuanchun Shen <[email protected]>
yuancu
requested review from
GumpacG,
LantaoJin,
MaxKsyunz,
Swiddis,
YANG-DB,
Yury-Fridlyand,
acarbonetto,
anirudha,
dai-chen,
derek-ho,
forestmvey,
joshuali925,
kavithacm,
mengweieric,
noCharger,
penghuo,
ps48,
qianheng-aws,
seankao-az and
ykmr1224
as code owners
Signed-off-by: Yuanchun Shen <[email protected]>
yuancu
force-pushed
the
backport/backport-4579-to-2.19-dev
branch
from
3e14b2f to
cb5fe97
Compare
qianheng-aws
approved these changes
Nov 10, 2025
LantaoJin
approved these changes
Nov 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Backport #4579 to 2.19-dev
Commit Message
WIP: Make poc implementation for chart command
Support param useother and otherstr
Support usenull and nullstr (when both row split and col split present)
Append a final aggregation to merge OTHER categories
Handle common agg functions for OTHER category for timechart
Fix timechart IT
Sort earliest results with asc order
Support non-string fields as column split
Fix min/earliest order & fix non-accumulative agg for chart
Hint non-null in aggregateWithTrimming
Add integration tests for chart command
Add unit tests
Add doc for chart command
Prompt users that multiple agg is not supported
Add explain ITs
Remove unimplemented support for multiple aggregations in chart command
Add unit tests for chart command
Remove irrelevant yaml test
Tweak chart.rst
Swap the order of chart output to ensure metrics come last
Filter rows without col split when calculate grand total
Chores: tweak code order
Add anonymize test to chart command
Change grammart from limit=top 10 to limit=top10
Update chart doc
Rename row_number for chart to row_number_chart
Sort by row and col splits on top of chart results
Ignore rows without a row split in chart command
Keep categories with max summed values when top k is set
Simplify toAddHintsOnAggregate condition
Chores: eliminate unnecessary variables
Apply a non-null filter on fields referred by aggregations
Fix chart plans
Get rid of record class
Move ranking by column split to a helper function
(cherry picked from commit 5523932)
Related Issues
Resolves #399
Check List
--signoffor-s.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.