HyukjinKwon commented on a change in pull request #35064:
URL: https://github.com/apache/spark/pull/35064#discussion_r776554749



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/types/DecimalType.scala
##########
@@ -76,23 +77,29 @@ case class DecimalType(precision: Int, scale: Int) extends 
FractionalType {
    * Returns whether this DecimalType is wider than `other`. If yes, it means 
`other`
    * can be casted into `this` safely without losing any precision or range.
    */
-  private[sql] def isWiderThan(other: DataType): Boolean = other match {
+  private[sql] def isWiderThan(other: DataType): Boolean = 
isWiderThanInternal(other)

Review comment:
       Otherwise:
   
   ```
   [error] 
/.../spark/sql/catalyst/src/main/scala/org/apache/spark/sql/types/DecimalType.scala:81:20:
 could not optimize @tailrec annotated method isWiderThan: it is neither 
private nor final so can be overridden
   [error]   private[sql] def isWiderThan(other: DataType): Boolean = other 
match {
   [error]                    ^
   ```

##########
File path: 
core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala
##########
@@ -751,6 +751,14 @@ private[history] class FsHistoryProvider(conf: SparkConf, 
clock: Clock)
    * Visible for testing
    */
   private[history] def doMergeApplicationListing(
+      reader: EventLogFileReader,
+      scanTime: Long,
+      enableOptimizations: Boolean,
+      lastEvaluatedForCompaction: Option[Long]): Unit = 
doMergeApplicationListingInternal(
+    reader, scanTime, enableOptimizations, lastEvaluatedForCompaction)
+
+  @scala.annotation.tailrec
+  private def doMergeApplicationListingInternal(

Review comment:
       Otherwise:
   
   ```
   [error] 
/.../spark/core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala:754:24:
 could not optimize @tailrec annotated method doMergeApplicationListing: it is 
neither private nor final so can be overridden
   [error]   private[history] def doMergeApplicationListing(
   [error]                        ^
   [error] one error found
   ```

##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/types/DecimalType.scala
##########
@@ -76,23 +77,29 @@ case class DecimalType(precision: Int, scale: Int) extends 
FractionalType {
    * Returns whether this DecimalType is wider than `other`. If yes, it means 
`other`
    * can be casted into `this` safely without losing any precision or range.
    */
-  private[sql] def isWiderThan(other: DataType): Boolean = other match {
+  private[sql] def isWiderThan(other: DataType): Boolean = 
isWiderThanInternal(other)
+
+  @tailrec
+  private def isWiderThanInternal(other: DataType): Boolean = other match {
     case dt: DecimalType =>
       (precision - scale) >= (dt.precision - dt.scale) && scale >= dt.scale
     case dt: IntegralType =>
-      isWiderThan(DecimalType.forType(dt))
+      isWiderThanInternal(DecimalType.forType(dt))
     case _ => false
   }
 
   /**
    * Returns whether this DecimalType is tighter than `other`. If yes, it 
means `this`
    * can be casted into `other` safely without losing any precision or range.
    */
-  private[sql] def isTighterThan(other: DataType): Boolean = other match {
+  private[sql] def isTighterThan(other: DataType): Boolean = 
isTighterThanInternal(other)

Review comment:
       Otherwise:
   
   ```
   [error] 
/.../spark/sql/catalyst/src/main/scala/org/apache/spark/sql/types/DecimalType.scala:94:20:
 could not optimize @tailrec annotated method isTighterThan: it is neither 
private nor final so can be overridden
   [error]   private[sql] def isTighterThan(other: DataType): Boolean = other 
match {
   [error]                    ^
   ```

##########
File path: 
core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala
##########
@@ -751,6 +751,14 @@ private[history] class FsHistoryProvider(conf: SparkConf, 
clock: Clock)
    * Visible for testing
    */
   private[history] def doMergeApplicationListing(
+      reader: EventLogFileReader,
+      scanTime: Long,
+      enableOptimizations: Boolean,
+      lastEvaluatedForCompaction: Option[Long]): Unit = 
mergeApplicationListing(
+    reader, scanTime, enableOptimizations, lastEvaluatedForCompaction)
+
+  @scala.annotation.tailrec
+  private def mergeApplicationListing(

Review comment:
       Otherwise:
   
   ```
   [error] 
/home/runner/work/spark/spark/core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala:754:24:
 could not optimize @tailrec annotated method doMergeApplicationListing: it is 
neither private nor final so can be overridden
   [error]   private[history] def doMergeApplicationListing(
   [error]                        ^
   [error] one error found
   [error] (core / Compile / compileIncremental) Compilation failed
   [error] Total time: 204 s (03:24), completed Dec 30, 2021 2:54:38 AM
   ```




-- 
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]

Reply via email to