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

    https://github.com/apache/spark/pull/21857#discussion_r204760405
  
    --- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
 ---
    @@ -182,14 +182,16 @@ case class Intersect(left: LogicalPlan, right: 
LogicalPlan) extends SetOperation
       }
     }
     
    -case class Except(left: LogicalPlan, right: LogicalPlan) extends 
SetOperation(left, right) {
    -
    +abstract class ExceptBase(left: LogicalPlan, right: LogicalPlan) extends 
SetOperation(left, right) {
       /** We don't use right.output because those rows get excluded from the 
set. */
       override def output: Seq[Attribute] = left.output
     
       override protected def validConstraints: Set[Expression] = 
leftConstraints
     }
     
    +case class Except(left: LogicalPlan, right: LogicalPlan) extends 
ExceptBase(left, right)
    +case class ExceptAll(left: LogicalPlan, right: LogicalPlan) extends 
ExceptBase(left, right)
    --- End diff --
    
    We need a logical node for `ExceptAll`? As another option, we can add a 
flag as a field value of `Except` (`case class Except(left: LogicalPlan, right: 
LogicalPlan, all: Boolean)`? That's because `Except` and `ExceptAll` is not 
different for the analyzer.


---

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

Reply via email to