Skip to content

Commit ab00533

Browse files
rxindongjoon-hyun
authored andcommitted
[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 apache#23110 from rxin/SPARK-26129-1. Authored-by: Reynold Xin <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
1 parent 15c0384 commit ab00533

File tree

2 files changed

+20
-6
lines changed

2 files changed

+20
-6
lines changed

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,19 @@ class QueryPlanningTracker {
116116

117117
def phases: Map[String, Long] = phaseToTimeNs.asScala.toMap
118118

119-
/** Returns the top k most expensive rules (as measured by time). */
119+
/**
120+
* Returns the top k most expensive rules (as measured by time). If k is larger than the rules
121+
* seen so far, return all the rules. If there is no rule seen so far or k <= 0, return empty seq.
122+
*/
120123
def topRulesByTime(k: Int): Seq[(String, RuleSummary)] = {
121-
val orderingByTime: Ordering[(String, RuleSummary)] = Ordering.by(e => e._2.totalTimeNs)
122-
val q = new BoundedPriorityQueue(k)(orderingByTime)
123-
rulesMap.asScala.foreach(q.+=)
124-
q.toSeq.sortBy(r => -r._2.totalTimeNs)
124+
if (k <= 0) {
125+
Seq.empty
126+
} else {
127+
val orderingByTime: Ordering[(String, RuleSummary)] = Ordering.by(e => e._2.totalTimeNs)
128+
val q = new BoundedPriorityQueue(k)(orderingByTime)
129+
rulesMap.asScala.foreach(q.+=)
130+
q.toSeq.sortBy(r => -r._2.totalTimeNs)
131+
}
125132
}
126133

127134
}

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/QueryPlanningTrackerSuite.scala

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,17 +62,24 @@ class QueryPlanningTrackerSuite extends SparkFunSuite {
6262

6363
test("topRulesByTime") {
6464
val t = new QueryPlanningTracker
65+
66+
// Return empty seq when k = 0
67+
assert(t.topRulesByTime(0) == Seq.empty)
68+
assert(t.topRulesByTime(1) == Seq.empty)
69+
6570
t.recordRuleInvocation("r2", 2, effective = true)
6671
t.recordRuleInvocation("r4", 4, effective = true)
6772
t.recordRuleInvocation("r1", 1, effective = false)
6873
t.recordRuleInvocation("r3", 3, effective = false)
6974

75+
// k <= total size
76+
assert(t.topRulesByTime(0) == Seq.empty)
7077
val top = t.topRulesByTime(2)
7178
assert(top.size == 2)
7279
assert(top(0)._1 == "r4")
7380
assert(top(1)._1 == "r3")
7481

75-
// Don't crash when k > total size
82+
// k > total size
7683
assert(t.topRulesByTime(10).size == 4)
7784
}
7885
}

0 commit comments

Comments
 (0)