From 683630ac3fbf054534e2589258793c9baaebfbf5 Mon Sep 17 00:00:00 2001 From: Reynold Xin Date: Wed, 21 Nov 2018 23:25:09 +0100 Subject: [PATCH] [SPARK-26129] --- .../sql/catalyst/QueryPlanningTracker.scala | 17 ++++++++++++----- .../catalyst/QueryPlanningTrackerSuite.scala | 9 ++++++++- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala index 420f2a1f20997..244081cd160b6 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala @@ -116,12 +116,19 @@ class QueryPlanningTracker { def phases: Map[String, Long] = phaseToTimeNs.asScala.toMap - /** Returns the top k most expensive rules (as measured by time). */ + /** + * Returns the top k most expensive rules (as measured by time). If k is larger than the rules + * seen so far, return all the rules. If there is no rule seen so far or k <= 0, return empty seq. + */ def topRulesByTime(k: Int): Seq[(String, RuleSummary)] = { - val orderingByTime: Ordering[(String, RuleSummary)] = Ordering.by(e => e._2.totalTimeNs) - val q = new BoundedPriorityQueue(k)(orderingByTime) - rulesMap.asScala.foreach(q.+=) - q.toSeq.sortBy(r => -r._2.totalTimeNs) + if (k <= 0) { + Seq.empty + } else { + val orderingByTime: Ordering[(String, RuleSummary)] = Ordering.by(e => e._2.totalTimeNs) + val q = new BoundedPriorityQueue(k)(orderingByTime) + rulesMap.asScala.foreach(q.+=) + q.toSeq.sortBy(r => -r._2.totalTimeNs) + } } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/QueryPlanningTrackerSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/QueryPlanningTrackerSuite.scala index f42c262dfbdd8..120b284a77854 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/QueryPlanningTrackerSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/QueryPlanningTrackerSuite.scala @@ -62,17 +62,24 @@ class QueryPlanningTrackerSuite extends SparkFunSuite { test("topRulesByTime") { val t = new QueryPlanningTracker + + // Return empty seq when k = 0 + assert(t.topRulesByTime(0) == Seq.empty) + assert(t.topRulesByTime(1) == Seq.empty) + t.recordRuleInvocation("r2", 2, effective = true) t.recordRuleInvocation("r4", 4, effective = true) t.recordRuleInvocation("r1", 1, effective = false) t.recordRuleInvocation("r3", 3, effective = false) + // k <= total size + assert(t.topRulesByTime(0) == Seq.empty) val top = t.topRulesByTime(2) assert(top.size == 2) assert(top(0)._1 == "r4") assert(top(1)._1 == "r3") - // Don't crash when k > total size + // k > total size assert(t.topRulesByTime(10).size == 4) } }