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

    https://github.com/apache/spark/pull/15484#discussion_r84577307
  
    --- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
 ---
    @@ -518,59 +517,58 @@ case class Window(
     
     object Expand {
       /**
    -   * Extract attribute set according to the grouping id.
    +   * Build bit mask from attributes of selected grouping set.
        *
    -   * @param bitmask bitmask to represent the selected of the attribute 
sequence
    -   * @param attrs the attributes in sequence
    -   * @return the attributes of non selected specified via bitmask (with 
the bit set to 1)
    +   * @param groupingSetAttrs The attributes of selected grouping set
    +   * @param attrMap Mapping group by attributes to its index in attributes 
sequence
    +   * @return The bitmask which represents the selected attributes out of 
group by attributes.
    +   *         A bit in the bitmask is corresponding to an attribute in 
group by attributes sequence,
    +   *         the selected attribute has corresponding bit set to 0 and 
otherwise set to 1.
        */
    -  private def buildNonSelectAttrSet(
    -      bitmask: Int,
    -      attrs: Seq[Attribute]): AttributeSet = {
    -    val nonSelect = new ArrayBuffer[Attribute]()
    -
    -    var bit = attrs.length - 1
    -    while (bit >= 0) {
    -      if (((bitmask >> bit) & 1) == 1) nonSelect += attrs(attrs.length - 
bit - 1)
    -      bit -= 1
    -    }
    -
    -    AttributeSet(nonSelect)
    +  private def buildBitmask(
    +    groupingSetAttrs: Seq[Attribute],
    +    attrMap: Map[Attribute, Int]): Int = {
    +    val numAttributes = attrMap.size
    +    val mask = (1 << numAttributes) - 1
    +    (mask +: groupingSetAttrs.map(attrMap).map(index =>
    --- End diff --
    
    Could you add a little bit of documentation on the mask? It is non-trivial 
to understand.
    
    It might also be a good idea to split this into two separate statements. 
One to calculate the the attribute masks and one to reduce them.


---
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 [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to