Mailing-List: contact reviews-help@spark.apache.org; run by ezmlm
Precedence: bulk
From: hvanhovell <git@git.apache.org>
To: reviews@spark.apache.org
Reply-To: reviews@spark.apache.org
References: <git-pr-11485-spark@git.apache.org>
In-Reply-To: <git-pr-11485-spark@git.apache.org>
Subject: [GitHub] spark pull request: [SPARK-13637][SQL] use more information
 to sim...
Content-Type: text/plain
Message-Id: <20160303070348.F3812E0D58@git1-us-west.apache.org>
Date: Thu,  3 Mar 2016 07:03:48 +0000 (UTC)

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

    https://github.com/apache/spark/pull/11485#discussion_r54842640
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicOperators.scala ---
    @@ -473,41 +473,44 @@ private[sql] object Expand {
        * multiple output rows for a input row.
        *
        * @param bitmasks The bitmask set represents the grouping sets
    -   * @param groupByExprs The grouping by expressions
    +   * @param groupByAttrs The attributes of aliased group by expressions
        * @param gid Attribute of the grouping id
    -   * @param child Child operator
    +   * @param project The child project operator
        */
       def apply(
         bitmasks: Seq[Int],
    -    groupByExprs: Seq[Expression],
    +    groupByAttrs: Seq[Attribute],
         gid: Attribute,
    -    child: LogicalPlan): Expand = {
    +    project: Project): Expand = {
    +
    +    val originalOutput = project.child.output
    +    assert(project.output.length == (originalOutput ++ groupByAttrs).length)
    +    assert(project.output.zip(originalOutput ++ groupByAttrs).forall {
    +      case (attr1, attr2) => attr1 semanticEquals attr2
    +    })
    +
         // Create an array of Projections for the child projection, and replace the projections'
         // expressions which equal GroupBy expressions with Literal(null), if those expressions
         // are not set for this grouping set (according to the bit mask).
         val projections = bitmasks.map { bitmask =>
           // get the non selected grouping attributes according to the bit mask
    -      val nonSelectedGroupExprSet = buildNonSelectExprSet(bitmask, groupByExprs)
    +      val nonSelectedGroupAttrSet = buildNonSelectAttrSet(bitmask, groupByAttrs)
     
    -      (child.output :+ gid).map(expr => expr transformDown {
    -        // TODO this causes a problem when a column is used both for grouping and aggregation.
    --- End diff --
    
    It isn't anymore. We had some trouble with such columns when I placed the comment.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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