Skip to content

[SPARK-21247][SQL] Type comparison should respect case-sensitive SQL conf #18460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed

[SPARK-21247][SQL] Type comparison should respect case-sensitive SQL conf #18460

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
037120b
[SPARK-21247][SQL] Allow case-insensitive type equality in Set operation
dongjoon-hyun Jun 28, 2017
cded719
Add comments.
dongjoon-hyun Jul 7, 2017
180e1b3
Address comments.
dongjoon-hyun Jul 8, 2017
2343d97
Add test case.
dongjoon-hyun Jul 8, 2017
8fa884e
Move to findTightestCommonType.
dongjoon-hyun Jul 30, 2017
b2d1338
Move testcase, too.
dongjoon-hyun Jul 30, 2017
c72aa18
Add test cases.
dongjoon-hyun Oct 6, 2017
67a037c
Update test cases.
dongjoon-hyun Oct 6, 2017
52d19d3
Address comments.
dongjoon-hyun Oct 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package org.apache.spark.sql.catalyst.analysis

import java.util.Locale
import javax.annotation.Nullable

import scala.annotation.tailrec
Expand Down Expand Up @@ -105,11 +104,10 @@ object TypeCoercion {
Some(StructType(fields1.zip(fields2).map { case (f1, f2) =>
// Since `t1.sameType(t2)` is true, two StructTypes have the same DataType
// except `name` (in case of `spark.sql.caseSensitive=false`) and `nullable`.
// - Different names: use a lower case name because findTightestCommonType is commutative.
// - Different names: use f1.name
// - Different nullabilities: `nullable` is true iff one of them is nullable.
val name = if (f1.name == f2.name) f1.name else f1.name.toLowerCase(Locale.ROOT)
val dataType = findTightestCommonType(f1.dataType, f2.dataType).get
StructField(name, dataType, nullable = f1.nullable || f2.nullable)
StructField(f1.name, dataType, nullable = f1.nullable || f2.nullable)
}))

case _ => None
Expand Down
17 changes: 11 additions & 6 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercionSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,17 @@ class TypeCoercionSuite extends AnalysisTest {
widenFunc: (DataType, DataType) => Option[DataType],
t1: DataType,
t2: DataType,
expected: Option[DataType]): Unit = {
expected: Option[DataType],
isSymmetric: Boolean = true): Unit = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gatorsmile . I extended this function for using non-symmetric tests and addressed your comments.

var found = widenFunc(t1, t2)
assert(found == expected,
s"Expected $expected as wider common type for $t1 and $t2, found $found")
// Test both directions to make sure the widening is symmetric.
found = widenFunc(t2, t1)
assert(found == expected,
s"Expected $expected as wider common type for $t2 and $t1, found $found")
if (isSymmetric) {
found = widenFunc(t2, t1)
assert(found == expected,
s"Expected $expected as wider common type for $t2 and $t1, found $found")
}
}

test("implicit type cast - ByteType") {
Expand Down Expand Up @@ -419,10 +422,12 @@ class TypeCoercionSuite extends AnalysisTest {
None)
}
withSQLConf(SQLConf.CASE_SENSITIVE.key -> "false") {
widenTest(
checkWidenType(
TypeCoercion.findTightestCommonType,
StructType(Seq(StructField("a", IntegerType), StructField("B", IntegerType))),
StructType(Seq(StructField("A", IntegerType), StructField("b", IntegerType))),
Some(StructType(Seq(StructField("a", IntegerType), StructField("b", IntegerType)))))
Some(StructType(Seq(StructField("a", IntegerType), StructField("B", IntegerType)))),
isSymmetric = false)
}
}

Expand Down