Repository: spark Updated Branches: refs/heads/master 15c038497 -> ab0053349
[SPARK-26129][SQL] edge behavior for QueryPlanningTracker.topRulesByTime - followup patch ## What changes were proposed in this pull request? This is an addendum patch for SPARK-26129 that defines the edge case behavior for QueryPlanningTracker.topRulesByTime. ## How was this patch tested? Added unit tests for each behavior. Closes #23110 from rxin/SPARK-26129-1. Authored-by: Reynold Xin <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/spark/repo Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/ab005334 Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/ab005334 Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/ab005334 Branch: refs/heads/master Commit: ab00533490953164cb2360bf2b9adc2c9fa962db Parents: 15c0384 Author: Reynold Xin <[email protected]> Authored: Thu Nov 22 02:27:06 2018 -0800 Committer: Dongjoon Hyun <[email protected]> Committed: Thu Nov 22 02:27:06 2018 -0800 ---------------------------------------------------------------------- .../spark/sql/catalyst/QueryPlanningTracker.scala | 17 ++++++++++++----- .../sql/catalyst/QueryPlanningTrackerSuite.scala | 9 ++++++++- 2 files changed, 20 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/spark/blob/ab005334/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala ---------------------------------------------------------------------- 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 420f2a1..244081c 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) + } } } http://git-wip-us.apache.org/repos/asf/spark/blob/ab005334/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/QueryPlanningTrackerSuite.scala ---------------------------------------------------------------------- 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 f42c262..120b284 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) } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
