juliuszsompolski commented on code in PR #41748:
URL: https://github.com/apache/spark/pull/41748#discussion_r1246905668
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala:
##########
@@ -19,16 +19,16 @@ package org.apache.spark.sql.catalyst
import scala.collection.JavaConverters._
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
import org.apache.spark.util.BoundedPriorityQueue
-
/**
* A simple utility for tracking runtime and associated stats in query
planning.
*
* There are two separate concepts we track:
*
- * 1. Phases: These are broad scope phases in query planning, as listed below,
i.e. analysis,
- * optimization and physical planning (just planning).
+ * 1. Phases: These are broad scope phases in query planning, as listed
below, i.e. analysis,
+ * optimization and physical planning (just planning).
Review Comment:
revert unrelated reformat
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala:
##########
@@ -43,13 +43,17 @@ object QueryPlanningTracker {
/**
* Summary for a rule.
- * @param totalTimeNs total amount of time, in nanosecs, spent in this rule.
- * @param numInvocations number of times the rule has been invoked.
- * @param numEffectiveInvocations number of times the rule has been invoked
and
- * resulted in a plan change.
+ * @param totalTimeNs
+ * total amount of time, in nanosecs, spent in this rule.
+ * @param numInvocations
+ * number of times the rule has been invoked.
+ * @param numEffectiveInvocations
+ * number of times the rule has been invoked and resulted in a plan change.
*/
class RuleSummary(
- var totalTimeNs: Long, var numInvocations: Long, var
numEffectiveInvocations: Long) {
+ var totalTimeNs: Long,
+ var numInvocations: Long,
+ var numEffectiveInvocations: Long) {
Review Comment:
revert unrelated reformat
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala:
##########
@@ -85,12 +89,17 @@ object QueryPlanningTracker {
def withTracker[T](tracker: QueryPlanningTracker)(f: => T): T = {
val originalTracker = localTracker.get()
localTracker.set(tracker)
- try f finally { localTracker.set(originalTracker) }
+ try f
+ finally { localTracker.set(originalTracker) }
Review Comment:
revert unrelated reformat
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala:
##########
@@ -120,12 +131,29 @@ class QueryPlanningTracker {
ret
}
+ /**
+ * Set when the query has been analysed and is ready for execution. This is
after analysis for
+ * eager commands and after planning for other queries. see @link
+ * org.apache.spark.sql.execution.CommandExecutionMode When called multiple
times, ignores
+ * subsequent call
+ */
+ def setReadyForExecution(analyzedPlan: LogicalPlan): Unit = {
+ if (readyForExecution) {
+ return
+ }
+ readyForExecution = true
+ readyForExecutionCallback(this, analyzedPlan)
Review Comment:
```suggestion
if (!readyForExecution) {
readyForExecution = true
readyForExecutionCallback(this, analyzedPlan)
}
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala:
##########
@@ -120,12 +131,29 @@ class QueryPlanningTracker {
ret
}
+ /**
+ * Set when the query has been analysed and is ready for execution. This is
after analysis for
+ * eager commands and after planning for other queries. see @link
+ * org.apache.spark.sql.execution.CommandExecutionMode When called multiple
times, ignores
+ * subsequent call
Review Comment:
nit: would be more readable with adjusted spacing
```suggestion
* Set when the query has been analysed and is ready for execution. This
is after analysis for
* eager commands and after planning for other queries.
* see @link org.apache.spark.sql.execution.CommandExecutionMode
* When called multiple times, ignores subsequent call.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala:
##########
@@ -120,12 +131,29 @@ class QueryPlanningTracker {
ret
}
+ /**
+ * Set when the query has been analysed and is ready for execution. This is
after analysis for
+ * eager commands and after planning for other queries. see @link
+ * org.apache.spark.sql.execution.CommandExecutionMode When called multiple
times, ignores
+ * subsequent call
+ */
+ def setReadyForExecution(analyzedPlan: LogicalPlan): Unit = {
+ if (readyForExecution) {
+ return
+ }
+ readyForExecution = true
+ readyForExecutionCallback(this, analyzedPlan)
+ }
+
/**
* Record a specific invocation of a rule.
*
- * @param rule name of the rule
- * @param timeNs time taken to run this invocation
- * @param effective whether the invocation has resulted in a plan change
+ * @param rule
+ * name of the rule
+ * @param timeNs
+ * time taken to run this invocation
+ * @param effective
+ * whether the invocation has resulted in a plan change
Review Comment:
revert unrelated spacing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
