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

    https://github.com/apache/spark/pull/21106#discussion_r187353305
  
    --- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Projection.scala
 ---
    @@ -108,7 +108,31 @@ abstract class UnsafeProjection extends Projection {
       override def apply(row: InternalRow): UnsafeRow
     }
     
    -trait UnsafeProjectionCreator {
    +/**
    + * The factory object for `UnsafeProjection`.
    + */
    +object UnsafeProjection extends CodegenObjectFactory[Seq[Expression], 
UnsafeProjection] {
    --- End diff --
    
    The problem of passing `mode` into class `UnsafeProjection` is, if we 
create the `UnsafeProjection` object and then change the SQL config, it can't 
be reflected in ``UnsafeProjection` object's behavior.
    
    If we don't let the config affect this behavior on production, then it 
should be fine. Then we don't actually need this parameter at most of time, 
because we only use it in tests. If so, this parameter seems useless. We just 
need to access `CodegenObjectFactoryMode.currentMode` when `createObject` is 
called.


---

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

Reply via email to