Skip to content

[SPARK-24879][SQL] Fix NPE in Hive partition pruning filter pushdown #21832

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
PenguinToast wants to merge 3 commits into from
Closed

[SPARK-24879][SQL] Fix NPE in Hive partition pruning filter pushdown #21832

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
388caa9
Filter out `null` values for partition pruning predicates
PenguinToast Jul 20, 2018
34d74e3
Prevent partition pruning filters with `null` from being pushed down
PenguinToast Jul 20, 2018
ce86fbe
Clearly document the nuances of this change
PenguinToast Jul 20, 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
11 changes: 10 additions & 1 deletion sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {

object ExtractableLiteral {
def unapply(expr: Expression): Option[String] = expr match {
case Literal(null, _) => None // `null`s can be cast as other types; we want to avoid NPEs.
case Literal(value, _: IntegralType) => Some(value.toString)
case Literal(value, _: StringType) => Some(quoteStringLiteral(value.toString))
case _ => None
Expand All @@ -606,7 +607,15 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {

object ExtractableLiterals {
def unapply(exprs: Seq[Expression]): Option[Seq[String]] = {
val extractables = exprs.map(ExtractableLiteral.unapply)
// SPARK-24879: The Hive filter parser does not support "null", but we still want to push

@gatorsmile gatorsmile Jul 20, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

-> Hive metastore filter parser

// down as many predicates as we can while still maintaining correctness. "x in (a, b,
// null)" can be rewritten as "x in (a, b)" for the purposes of partition pruning, so we

@gatorsmile gatorsmile Jul 20, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we should write down the rules here.
1 in (2, NULL) -> NULL
1 in (1, NULL) -> true
1 in (2) -> false

NULL is not equal to FALSE. Since all the pushed down predicates are NULL intolerant and connected by AND or OR, NULL can be treated as FALSE.

// filter out "null" values here to achieve this.
val extractables = exprs
.filter {
case Literal(null, _) => false
case _ => true
}.map(ExtractableLiteral.unapply)
if (extractables.nonEmpty && extractables.forall(_.isDefined)) {
Some(extractables.map(_.get))
} else {
Expand Down
8 changes: 8 additions & 0 deletions sql/hive/src/test/scala/org/apache/spark/sql/hive/client/FiltersSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ class FiltersSuite extends SparkFunSuite with Logging with PlanTest {
(Literal("p2\" and q=\"q2") === a("stringcol", StringType)) :: Nil,
"""stringcol = 'p1" and q="q1' and 'p2" and q="q2' = stringcol""")

filterTest("SPARK-24879 null literals should be ignored for IN constructs",
(a("intcol", IntegerType) in (Literal(1), Literal(null))) :: Nil,
"(intcol = 1)")

filterTest("typecast null literals should not be pushed down in simple predicates",
(a("intcol", IntegerType) === Literal(null, IntegerType)) :: Nil,
"")

private def filterTest(name: String, filters: Seq[Expression], result: String) = {
test(name) {
withSQLConf(SQLConf.ADVANCED_PARTITION_PREDICATE_PUSHDOWN.key -> "true") {
Expand Down