Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/23062#discussion_r234846910
  
    --- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/ScalaReflectionSuite.scala
 ---
    @@ -109,6 +109,35 @@ object TestingUDT {
       }
     }
     
    +/** An example derived from Twitter/Scrooge codegen for thrift  */
    +object ScroogeLikeExample {
    +  def apply(x: Int): ScroogeLikeExample = new Immutable(x)
    +
    +  def unapply(_item: ScroogeLikeExample): Option[Int] = Some(_item.x)
    +
    +  class Immutable(val x: Int) extends ScroogeLikeExample
    +}
    +
    +trait ScroogeLikeExample extends Product1[Int] with Serializable {
    +  import ScroogeLikeExample._
    +
    +  def x: Int
    +
    +  override def _1: Int = x
    +
    +  def copy(x: Int = this.x): ScroogeLikeExample = new Immutable(x)
    +
    +  override def canEqual(other: Any): Boolean = 
other.isInstanceOf[ScroogeLikeExample]
    +
    +  private def _equals(x: ScroogeLikeExample, y: ScroogeLikeExample): 
Boolean =
    +      x.productArity == y.productArity &&
    --- End diff --
    
    Sure, but you've already established it's a `ScroogeLikeExample` here. Why 
must it be Product1 just to check whether it's also Product1? seems like it's 
not adding anything. In fact, why not just compare the one element that this 
trait knows about? to the extent it can implement equals() meaningfully, that's 
all it is doing already.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to