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 diff --git a/docs/user/ppl/interfaces/endpoint.md b/docs/user/ppl/interfaces/endpoint.md index fd61c212a97..300f99331d0 100644 --- a/docs/user/ppl/interfaces/endpoint.md +++ b/docs/user/ppl/interfaces/endpoint.md @@ -247,11 +247,22 @@ 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 | — | + + ### 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)