Skip to content

Commit

Permalink
[SPARK-6463][SQL] AttributeSet.equal should compare size
Browse files Browse the repository at this point in the history
Previously this could result in sets compare equals when in fact the right was a subset of the left.

Based on #5133 by sisihj

Author: sisihj <jun.hejun@huawei.com>
Author: Michael Armbrust <michael@databricks.com>

Closes #5194 from marmbrus/pr/5133 and squashes the following commits:

5ed4615 [Michael Armbrust] fix imports
d4cbbc0 [Michael Armbrust] Add test cases
0a0834f [sisihj]  AttributeSet.equal should compare size
  • Loading branch information
marmbrus committed Mar 26, 2015
1 parent e87bf37 commit 276ef1c
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ class AttributeSet private (val baseSet: Set[AttributeEquals])

/** Returns true if the members of this AttributeSet and other are the same. */
override def equals(other: Any): Boolean = other match {
case otherSet: AttributeSet => baseSet.map(_.a).forall(otherSet.contains)
case otherSet: AttributeSet =>
otherSet.size == baseSet.size && baseSet.map(_.a).forall(otherSet.contains)
case _ => false
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.spark.sql.catalyst.expressions

import org.scalatest.FunSuite

import org.apache.spark.sql.types.IntegerType

class AttributeSetSuite extends FunSuite {

val aUpper = AttributeReference("A", IntegerType)(exprId = ExprId(1))
val aLower = AttributeReference("a", IntegerType)(exprId = ExprId(1))
val fakeA = AttributeReference("a", IntegerType)(exprId = ExprId(3))
val aSet = AttributeSet(aLower :: Nil)

val bUpper = AttributeReference("B", IntegerType)(exprId = ExprId(2))
val bLower = AttributeReference("b", IntegerType)(exprId = ExprId(2))
val bSet = AttributeSet(bUpper :: Nil)

val aAndBSet = AttributeSet(aUpper :: bUpper :: Nil)

test("sanity check") {
assert(aUpper != aLower)
assert(bUpper != bLower)
}

test("checks by id not name") {
assert(aSet.contains(aUpper) === true)
assert(aSet.contains(aLower) === true)
assert(aSet.contains(fakeA) === false)

assert(aSet.contains(bUpper) === false)
assert(aSet.contains(bLower) === false)
}

test("++ preserves AttributeSet") {
assert((aSet ++ bSet).contains(aUpper) === true)
assert((aSet ++ bSet).contains(aLower) === true)
}

test("extracts all references references") {
val addSet = AttributeSet(Add(aUpper, Alias(bUpper, "test")()):: Nil)
assert(addSet.contains(aUpper))
assert(addSet.contains(aLower))
assert(addSet.contains(bUpper))
assert(addSet.contains(bLower))
}

test("dedups attributes") {
assert(AttributeSet(aUpper :: aLower :: Nil).size === 1)
}

test("subset") {
assert(aSet.subsetOf(aAndBSet) === true)
assert(aAndBSet.subsetOf(aSet) === false)
}

test("equality") {
assert(aSet != aAndBSet)
assert(aAndBSet != aSet)
assert(aSet != bSet)
assert(bSet != aSet)

assert(aSet == aSet)
assert(aSet == AttributeSet(aUpper :: Nil))
}
}

0 comments on commit 276ef1c

Please sign in to comment.