Skip to content

[SPARK-24402] [SQL] Optimize In expression when only one element in the collection or collection is empty #21442

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
dbtsai wants to merge 9 commits into from
Closed

[SPARK-24402] [SQL] Optimize In expression when only one element in the collection or collection is empty #21442

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7c44c70
Optimize `In`
dbtsai May 28, 2018
449613a
style
dbtsai May 28, 2018
7a354fc
Addressed feedback
dbtsai May 29, 2018
fa678f8
Merge branch 'master' into optimize-in
dbtsai Jul 16, 2018
61824f7
Merge branch 'master' into optimize-in
dbtsai Jul 16, 2018
23fedd8
Addressed feedback
dbtsai Jul 16, 2018
5079833
update
dbtsai Jul 16, 2018
c519c40
Add one more test
dbtsai Jul 16, 2018
9174a30
Remove duplicated code
dbtsai Jul 16, 2018
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
9 changes: 7 additions & 2 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,15 @@ object ReorderAssociativeOperator extends Rule[LogicalPlan] {
object OptimizeIn extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = plan transform {
case q: LogicalPlan => q transformExpressionsDown {
case In(v, list) if list.isEmpty && !v.nullable => FalseLiteral
case In(v, list) if list.isEmpty =>
// When v is not nullable, the following expression will be optimized
// to FalseLiteral which is tested in OptimizeInSuite.scala
If(IsNotNull(v), FalseLiteral, Literal(null, BooleanType))
case expr @ In(v, list) if expr.inSetConvertible =>
val newList = ExpressionSet(list).toSeq
if (newList.size > SQLConf.get.optimizerInSetConversionThreshold) {
if (newList.length == 1) {
Copy link
Member

@viirya viirya May 29, 2018

Choose a reason for hiding this comment

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

When list.length == 1, we don't need to create ExpressionSet.

Copy link
Contributor

@cloud-fan cloud-fan May 29, 2018

Choose a reason for hiding this comment

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

This is too minor, I'd like to keep the current code and not break the code flow.

Copy link
Member

@viirya viirya May 29, 2018

Choose a reason for hiding this comment

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

Sounds good.

EqualTo(v, newList.head)
Copy link
Member

@gatorsmile gatorsmile May 29, 2018

Choose a reason for hiding this comment

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

This will fail, since the schema mismatches when the data type is struct. The test cases were added a few days ago. #21425

} else if (newList.size > SQLConf.get.optimizerInSetConversionThreshold) {
Copy link
Member

@gengliangwang gengliangwang May 29, 2018

Choose a reason for hiding this comment

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

nit: size => length
because we use length in the previous if

val hSet = newList.map(e => e.eval(EmptyRow))
InSet(v, HashSet() ++ hSet)
} else if (newList.size < list.size) {
Copy link
Member

@gengliangwang gengliangwang May 29, 2018

Choose a reason for hiding this comment

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

nit: In line 235 the comment
// newList.length == list.length
can be updated as
// newList.length == list.length && newList.length > 1

Expand Down
32 changes: 32 additions & 0 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/OptimizeInSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,21 @@ class OptimizeInSuite extends PlanTest {
}
}

test("OptimizedIn test: one element in list gets transformed to EqualTo.") {
val originalQuery =
testRelation
.where(In(UnresolvedAttribute("a"), Seq(Literal(1))))
.analyze

val optimized = Optimize.execute(originalQuery)
val correctAnswer =
testRelation
.where(EqualTo(UnresolvedAttribute("a"), Literal(1)))
.analyze

comparePlans(optimized, correctAnswer)
}

test("OptimizedIn test: In empty list gets transformed to FalseLiteral " +
"when value is not nullable") {
val originalQuery =
Expand All @@ -191,4 +206,21 @@ class OptimizeInSuite extends PlanTest {

comparePlans(optimized, correctAnswer)
}

test("OptimizedIn test: In empty list gets transformed to `If` expression " +
"when value is nullable") {
val originalQuery =
testRelation
.where(In(UnresolvedAttribute("a"), Nil))
.analyze

val optimized = Optimize.execute(originalQuery)
val correctAnswer =
testRelation
.where(If(IsNotNull(UnresolvedAttribute("a")),
Literal(false), Literal.create(null, BooleanType)))
.analyze

comparePlans(optimized, correctAnswer)
}
}