From 6e1d4798ba0e5f6b0fe4cd4609097fec293c172a Mon Sep 17 00:00:00 2001 From: Krish Gandhi Date: Thu, 20 Aug 2026 11:47:07 -0700 Subject: [PATCH 1/4] Editing analyze rule and adding CI Signed-off-by: Krish Gandhi --- .../sql/executor/analyze/ExpensiveSortRule.java | 6 +++++- .../analyze/AnalyzeRecommendationBuilderTest.java | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/executor/analyze/ExpensiveSortRule.java b/core/src/main/java/org/opensearch/sql/executor/analyze/ExpensiveSortRule.java index 0db47504df3..fb93e3e8ff8 100644 --- a/core/src/main/java/org/opensearch/sql/executor/analyze/ExpensiveSortRule.java +++ b/core/src/main/java/org/opensearch/sql/executor/analyze/ExpensiveSortRule.java @@ -29,7 +29,11 @@ public List apply(ProfileView view) { return recommendations; } for (PlanNode node : view.planNodes()) { - if (!node.getNode().toLowerCase(Locale.ROOT).contains("sort")) { + // Match standalone sorts and bounded sorts fused with a limit. A top-level PPL sort is + // typically merged with the query-size-limit into CalciteEnumerableTopK, whose name contains + // no "sort" -- but a TopK is always a sort (it extends EnumerableLimitSort), so it qualifies. + String name = node.getNode().toLowerCase(Locale.ROOT); + if (!name.contains("sort") && !name.contains("topk")) { continue; } long rowsIn = ProfileView.rowsIn(node); diff --git a/core/src/test/java/org/opensearch/sql/executor/analyze/AnalyzeRecommendationBuilderTest.java b/core/src/test/java/org/opensearch/sql/executor/analyze/AnalyzeRecommendationBuilderTest.java index 95624ee161f..ded1b20f3e0 100644 --- a/core/src/test/java/org/opensearch/sql/executor/analyze/AnalyzeRecommendationBuilderTest.java +++ b/core/src/test/java/org/opensearch/sql/executor/analyze/AnalyzeRecommendationBuilderTest.java @@ -99,6 +99,21 @@ void expensiveSortFiresOnLargeSlowSort() { assertTrue(r.getMessage().contains("30% of execution")); } + @Test + void expensiveSortFiresForFusedTopKNode() { + // A top-level sort fuses with the query-size-limit into CalciteEnumerableTopK (no "sort" in + // the name). It is still a sort, so it must trip the rule: 30ms self-time of 100 (30% > 20%), + // 60k input rows (> 50k). + PlanNode scan = new PlanNode("EnumerableMergeJoin", 10.0, 60_000, null); + PlanNode topK = new PlanNode("CalciteEnumerableTopK", 40.0, 100, List.of(scan)); + List recs = new AnalyzeRecommendationBuilder(profile(1, 100, topK)).build(); + + Recommendation r = ruleOf(recs, "Expensive Sort").orElseThrow(); + assertEquals(RecommendationSeverityLevel.WARNING, r.getSeverity()); + assertEquals("CalciteEnumerableTopK", r.getAffected_node()); + assertTrue(r.getMessage().contains("Sorting 60000 rows")); + } + @Test void expensiveSortSilentWhenSelfTimeIsSmall() { // sort cumulative 90ms but its child took 89ms -> self-time only 1ms, not expensive From 5f0fa31338f41a4b6d902d03347186d52ca0aee5 Mon Sep 17 00:00:00 2001 From: Krish Gandhi Date: Thu, 20 Aug 2026 14:25:31 -0700 Subject: [PATCH 2/4] Updating endpoint.md Signed-off-by: Krish Gandhi --- docs/user/ppl/interfaces/endpoint.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/user/ppl/interfaces/endpoint.md b/docs/user/ppl/interfaces/endpoint.md index fd61c212a97..bfa24dea158 100644 --- a/docs/user/ppl/interfaces/endpoint.md +++ b/docs/user/ppl/interfaces/endpoint.md @@ -247,6 +247,18 @@ Expected output (trimmed): | `size` | Integer | Number of result rows returned. | +### Rules For Recommendations + | Rule | Severity | Trigger | Default threshold | Suggestion | + |------|----------|---------------|-------------------|------------| + | Ineffective Filter | WARNING | An in-memory filter operator passes through nearly all of its input rows (`rows_out / rows_in`). | ratio > 0.95 | Consider removing the filter or making it more selective. | + | Join Row Explosion | WARNING / CRITICAL | A join produces far more rows than its combined inputs (`rows_out / rows_in`). | ratio > 5 (WARNING); ≥ 20 (CRITICAL) | Add filters to the subqueries before the join to reduce rows. | + | Expensive Sort | WARNING | A sort (including a limit-fused top-N) runs in-memory over a large input and consumes a large share of execution time. | self-time > 20% of execute and input rows > 50,000 | Filter or limit rows before sorting (e.g. add `head` or a `where`). | + | Bottleneck Stage | INFO | A single operator's self-time dominates total execution time. | self-time > 75% of execute | — | + | Optimize Phase Dominates | INFO | Query planning takes longer than execution. | optimize > execute **and** optimize > 75 ms | — | + + > **Note:** `rows_in` is the sum of an operator's child row counts; self-time is an operator's own duration (`time_ms − max(child time_ms)`), since profile `time_ms` is cumulative + wall-time. + ### Notes - Analyze output is only returned when the query finishes successfully. - Analyze requires the Calcite engine to be enabled (`plugins.calcite.enabled=true`). From 4208ba4cb001e50c723b66060e2ae7df38ae1354 Mon Sep 17 00:00:00 2001 From: Krish Gandhi Date: Thu, 20 Aug 2026 14:27:41 -0700 Subject: [PATCH 3/4] Updating endpoint.md Signed-off-by: Krish Gandhi --- docs/user/ppl/interfaces/endpoint.md | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/docs/user/ppl/interfaces/endpoint.md b/docs/user/ppl/interfaces/endpoint.md index bfa24dea158..c0a9029814f 100644 --- a/docs/user/ppl/interfaces/endpoint.md +++ b/docs/user/ppl/interfaces/endpoint.md @@ -248,22 +248,21 @@ Expected output (trimmed): ### Rules For Recommendations - | Rule | Severity | Trigger | Default threshold | Suggestion | - |------|----------|---------------|-------------------|------------| - | Ineffective Filter | WARNING | An in-memory filter operator passes through nearly all of its input rows (`rows_out / rows_in`). | ratio > 0.95 | Consider removing the filter or making it more selective. | - | Join Row Explosion | WARNING / CRITICAL | A join produces far more rows than its combined inputs (`rows_out / rows_in`). | ratio > 5 (WARNING); ≥ 20 (CRITICAL) | Add filters to the subqueries before the join to reduce rows. | - | Expensive Sort | WARNING | A sort (including a limit-fused top-N) runs in-memory over a large input and consumes a large share of execution time. | self-time > 20% of execute and input rows > 50,000 | Filter or limit rows before sorting (e.g. add `head` or a `where`). | - | Bottleneck Stage | INFO | A single operator's self-time dominates total execution time. | self-time > 75% of execute | — | - | Optimize Phase Dominates | INFO | Query planning takes longer than execution. | optimize > execute **and** optimize > 75 ms | — | - - > **Note:** `rows_in` is the sum of an operator's child row counts; self-time is an operator's own duration (`time_ms − max(child time_ms)`), since profile `time_ms` is cumulative - wall-time. +| Rule | Severity | Trigger | Default threshold | Suggestion | +|------|----------|---------------|-------------------|------------| +| Ineffective Filter | WARNING | An in-memory filter operator passes through nearly all of its input rows (`rows_out / rows_in`). | ratio > 0.95 | Consider removing the filter or making it more selective. | +| Join Row Explosion | WARNING / CRITICAL | A join produces far more rows than its combined inputs (`rows_out / rows_in`). | ratio > 5 (WARNING); ≥ 20 (CRITICAL) | Add filters to the subqueries before the join to reduce rows. | +| Expensive Sort | WARNING | A sort (including a limit-fused top-N) runs in-memory over a large input and consumes a large share of execution time. | self-time > 20% of execute and input rows > 50,000 | Filter or limit rows before sorting (e.g. add `head` or a `where`). | +| Bottleneck Stage | INFO | A single operator's self-time dominates total execution time. | self-time > 75% of execute | — | +| Optimize Phase Dominates | INFO | Query planning takes longer than execution. | optimize > execute **and** optimize > 75 ms | — | + ### Notes - Analyze output is only returned when the query finishes successfully. - Analyze requires the Calcite engine to be enabled (`plugins.calcite.enabled=true`). - The `profile` section uses the same format as the `profile` endpoint. - +- In rule calculation: rows_in` is the sum of an operator's child row counts; self-time is an operator's own duration (`time_ms − max(child time_ms)`), since profile `time_ms` is cumulative + wall-time. ## Profile (Experimental) (Deprecated) From 86a3cd4ca33887a3277bb059fcde2c1ccee1159f Mon Sep 17 00:00:00 2001 From: Krish Gandhi Date: Thu, 20 Aug 2026 14:28:28 -0700 Subject: [PATCH 4/4] Updating endpoint.md Signed-off-by: Krish Gandhi --- docs/user/ppl/interfaces/endpoint.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/ppl/interfaces/endpoint.md b/docs/user/ppl/interfaces/endpoint.md index c0a9029814f..300f99331d0 100644 --- a/docs/user/ppl/interfaces/endpoint.md +++ b/docs/user/ppl/interfaces/endpoint.md @@ -261,7 +261,7 @@ Expected output (trimmed): - Analyze output is only returned when the query finishes successfully. - Analyze requires the Calcite engine to be enabled (`plugins.calcite.enabled=true`). - The `profile` section uses the same format as the `profile` endpoint. -- In rule calculation: rows_in` is the sum of an operator's child row counts; self-time is an operator's own duration (`time_ms − max(child time_ms)`), since profile `time_ms` is cumulative +- In rule calculation: `rows_in` is the sum of an operator's child row counts; self-time is an operator's own duration (`time_ms − max(child time_ms)`), since profile `time_ms` is cumulative wall-time.