Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ package org.apache.spark.sql.catalyst.expressions
import org.apache.spark.sql.catalyst.analysis.UnresolvedException
import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
import org.apache.spark.sql.catalyst.types.BooleanType

import scala.collection.immutable.HashSet

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Scala import should go first. See here for more details.


object InterpretedPredicate {
def apply(expression: Expression, inputSchema: Seq[Attribute]): (Row => Boolean) =
Expand Down Expand Up @@ -95,6 +95,22 @@ case class In(value: Expression, list: Seq[Expression]) extends Predicate {
}
}

/**
* Optimized version of In clause, when all filter values of In clause are
* static.
*/
case class InSet(value: Expression, hset: HashSet[Any], child: Seq[Expression])
extends Predicate {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should actually only be indented 2 spaces (and I'd include a blank line after). Only wrapped arguments are indented 4.

def children = child

def nullable = true // TODO: Figure out correct nullability semantics of IN.
override def toString = s"$value IN ${hset.mkString("(", ",", ")")}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd actually say InSet here as the toString is mostly for debugging / understanding how queries will execute.


override def eval(input: Row): Any = {
hset.contains(value.eval(input))
}
}

case class And(left: Expression, right: Expression) extends BinaryPredicate {
def symbol = "&&"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import org.apache.spark.sql.catalyst.plans.logical._
import org.apache.spark.sql.catalyst.rules._
import org.apache.spark.sql.catalyst.types._

import scala.collection.immutable.HashSet

object Optimizer extends RuleExecutor[LogicalPlan] {
val batches =
Batch("Combine Limits", FixedPoint(100),
Expand All @@ -38,7 +40,8 @@ object Optimizer extends RuleExecutor[LogicalPlan] {
BooleanSimplification,
SimplifyFilters,
SimplifyCasts,
SimplifyCaseConversionExpressions) ::
SimplifyCaseConversionExpressions,
OptimizedIn) ::
Batch("Filter Pushdown", FixedPoint(100),
CombineFilters,
PushPredicateThroughProject,
Expand Down Expand Up @@ -225,6 +228,24 @@ object ConstantFolding extends Rule[LogicalPlan] {
}
}

/**
* Replaces [[In (value, seq[Literal])]] with optimized version[[InSet (value, HashSet[Literal])]]
* which is much faster
*/
object OptimizedIn extends Rule[LogicalPlan] {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should probably be OptimizeIn.

def apply(plan: LogicalPlan): LogicalPlan = plan transform {
case q: LogicalPlan => q transformExpressionsDown {
case In(v, list) if list.exists {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this is correct. This will convert In expressions when any of the expressions is a literal. Here is an example failure:

$ sbt/sbt hive/console
scala> sql("SELECT * FROM src WHERE key IN (1, value)") 
res1: org.apache.spark.sql.SchemaRDD = 
SchemaRDD[1] at RDD at SchemaRDD.scala:103
== Query Plan ==
== Physical Plan ==
org.apache.spark.sql.catalyst.errors.package$TreeNodeException: No function to evaluate expression. type: AttributeReference, tree: value#5

Instead I think you want !list.exists(!_.isInstanceOf[Literal]).

case Literal(_ , _) => true
case _ => false
} => {
val hSet = list.map(e => e.eval(null))
InSet(v, HashSet() ++ hSet, v +: list)
}
}
}
}

/**
* Simplifies boolean expressions where the answer can be determined without evaluating both sides.
* Note that this rule can eliminate expressions that might otherwise have been evaluated and thus
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import org.scalatest.FunSuite

import org.apache.spark.sql.catalyst.types._

import scala.collection.immutable.HashSet

/* Implicit conversions */
import org.apache.spark.sql.catalyst.dsl.expressions._

Expand Down Expand Up @@ -136,6 +138,18 @@ class ExpressionEvaluationSuite extends FunSuite {
checkEvaluation(In(Literal(1), Seq(Literal(1), Literal(2))) && In(Literal(2), Seq(Literal(1), Literal(2))), true)
}

test("INSET") {
val hS = HashSet[Any]() + 1 + 2
val s = Seq(Literal(1), Literal(2))
val one = Literal(1)
val two = Literal(2)
val three = Literal(3)
checkEvaluation(InSet(one, hS, one +: s), true)
checkEvaluation(InSet(two, hS, two +: s), true)
checkEvaluation(InSet(three, hS, three +: s), false)
checkEvaluation(InSet(one, hS, one +: s) && InSet(two, hS, two +: s), true)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should also double check that there isn't a problem if a null is in the in list. I'm not sure if thats actually valid SQL (and it should never change the result), but we shouldn't throw an exception.

}

test("MaxOf") {
checkEvaluation(MaxOf(1, 2), 2)
checkEvaluation(MaxOf(2, 1), 2)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* 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.optimizer

import org.apache.spark.sql.catalyst.analysis.{EliminateAnalysisOperators, UnresolvedAttribute}
import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.catalyst.plans.logical.{LocalRelation, LogicalPlan}
import org.apache.spark.sql.catalyst.plans.PlanTest
import org.apache.spark.sql.catalyst.rules.RuleExecutor
import org.apache.spark.sql.catalyst.types._

// For implicit conversions
import org.apache.spark.sql.catalyst.dsl.plans._
import org.apache.spark.sql.catalyst.dsl.expressions._

import scala.collection.immutable.HashSet

class OptimizedInSuite extends PlanTest {

object Optimize extends RuleExecutor[LogicalPlan] {
val batches =
Batch("AnalysisNodes", Once,
EliminateAnalysisOperators) ::
Batch("ConstantFolding", Once,
ConstantFolding,
BooleanSimplification,
OptimizedIn) :: Nil
}

val testRelation = LocalRelation('a.int, 'b.int, 'c.int)

test("OptimizedIn test: In clause optimized to InSet") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps a test to make sure that in clauses with attributes are not corrupted.

val originalQuery =
testRelation
.where(In(UnresolvedAttribute("a"), Seq(Literal(1),Literal(2))))
.analyze

val optimized = Optimize(originalQuery.analyze)
val correctAnswer =
testRelation
.where(InSet(UnresolvedAttribute("a"), HashSet[Any]()+1+2,
UnresolvedAttribute("a") +: Seq(Literal(1),Literal(2))))
.analyze

comparePlans(optimized, correctAnswer)
}
}